Opened 3 months ago

Closed 13 days ago

#431 closed bug (fixed)

System freeze when dragging the address field in Odyssey 1.23

Reported by: Gregor Owned by:
Priority: normal Milestone: 5.0-2020R1
Component: muimaster.library Version: 5.0-2019R4
Severity: minor Keywords:
Cc: Roman Kargin, Andy Broad OS Platform: AmigaOS4
Blocked By: Blocking:
Release Notes:

Description

Summary

System freeze when dragging accidentally the address field in Odyssey 1.23. This can happen when trying to copy and paste text from/to the address field. The problem is also present in Odyssey 1.23-R5 beta01, 02, 03 and 04.

Steps to reproduce

  1. Move the mouse pointer to the leftmost end of the address field in Odyssey, so that the pointer still stays as a text pointer (vertical bar).
  2. Press the left mouse button and keep it down.
  3. Move the mouse toward right, until the pointer freezes.

Freeze is 100% reproducible in my system.

Expected results

Dragging the address field should not even be possible.

Actual results

The address field is dragged, after which the system freezes (mouse/keyboard do not react any more).

Regression

Hardware reset is needed.

Notes

Tested in AmigaOne X5000/20 with AmigaOS 4.1 Prerelease (resail version, no beta), max RAM amount.

Attachments (10)

muimaster_locking.lha (1.1 MB) - added by Thore Böckelmann 3 months ago.
muimaster.library with extended locking selection
muimaster_20200618.zip (1.2 MB) - added by Thore Böckelmann 3 weeks ago.
nightly build of muimaster.library including debug symbols
cap2 (93.8 KB) - added by Andy Broad 2 weeks ago.
Log without lockup but muidebug set to POP
muidebug_20200624.zip (1.2 MB) - added by Thore Böckelmann 2 weeks ago.
muimaster.library with extended locking selection
muilock.cap (7.5 KB) - added by Andy Broad 2 weeks ago.
Log with setenv mui/screenlock intuition
muidebug_20200625.zip (1.2 MB) - added by Thore Böckelmann 2 weeks ago.
muimaster.library with extended locking selection
muilock2.cap (120.8 KB) - added by Andy Broad 2 weeks ago.
Log with setenv mui/screenlock intuition
muidebug_20200626.zip (1.3 MB) - added by Thore Böckelmann 2 weeks ago.
muimaster.library with extended locking selection
mui_20200626_01.cap (2.8 KB) - added by Andy Broad 13 days ago.
Capture with mui/screenlock set to intuition, off, then layers
mui_20200626_02.cap (1.6 KB) - added by Andy Broad 13 days ago.
Capture with mui/screenlock deleted

Change History (56)

comment:1 Changed 3 months ago by Thore Böckelmann

Cc: Roman Kargin added

I just tried the latest beta 04 of Odyssey 1.23 but I cannot reproduce this issue on my fs-uae based OS4 emulation. At least dragging the address bar with the default "about:" URL works perfectly for me.

Roman, what about you? Can you reproduce this issue?

comment:2 Changed 3 months ago by Roman Kargin

@Thore
Gregor wrote to me before as well, and I wasn't able to reproduce the issue. I tried again now and still can't. I can move a mouse pointer to the beginning of the address bar, pressing on the left mouse button and move drag bar everywhere I wish without lockups.

Maybe Gregor has some specific MUI-settings… Or maybe something with AISS installation, because when you drag to the areas you can't pointer images changes, maybe if AISS has broken something there…

@Gregor
Can you make a video of a bug? Also, can you try first to go to MUI-settings from Odyssey window, clicking on MUI's settings button and choice "preset: AmigaOS4", so it will use default MUI settings, and try to reproduce your issue.

Also why you say that "Dragging the address field should not even be possible." That done so you can move it to the hot-links and drop it here, to add a current address from the address bar to hot links.

But we need a video so we can see exactly where you move and on which point it freezes. Maybe it crashes when you move it off the screen… Video surely will help.

Last edited 3 months ago by Roman Kargin (previous) (diff)

comment:3 Changed 3 months ago by Thore Böckelmann

@Gregor does drag'n'drop work in general for you? Can you drag any of the background/color/frame settings in MUI prefs?

Last edited 3 months ago by Thore Böckelmann (previous) (diff)

comment:4 Changed 3 months ago by Gregor

@Thore @Roman
If I have in Odyssey "Settings/Mui…/DRag&Dro/Autostart" 'on' (ticked), I can drag the "quick link" buttons, and the objects in MUI prefs, and all these cause the same type of freezing. Also, trying to drag in YAM 2.7 an attachment file to a disk shows this issue.

But I think I found the primary source of this problem! There seems to be a conflict with the latest version (53.29) of "Clock" from the A-Eon's Enhancer package. I use it constantly on my WB screen, in the analog form (with arms).

If I a) close the clock or b) change the form from analog to digital or c) use the previous version (53.20) without changing any settings, I cannot anymore reproduce any of those freezings. The original Hyperion Clock (53.1) does not cause any problems, either.

If you have the Enhancer Software, could you please open the v. 53.29 Clock in analog form and see whether you can then reproduce the issue?

comment:5 Changed 3 months ago by Thore Böckelmann

I neither do own the enhancer package nor will I buy it, hence I cannot check that version of the Clock tool.

I suppose you have compositing activated on the screens you are using MUI applications on. In that case MUI should already detect that no screen locking is required for drag'n'drop. This locking formerly was required to pause screen updates of other applications while MUI draws the dragged images anywhere on the screen. But with compositing MUI can use a semi-transparent layer and locking the screen is no longer necessary.

However, you might want to try to set ENV:mui/nolock to any content. This will force MUI to skip any kind of screen locking and . If this hack causes drag'n'drop to work even with the new Clock version then both application try to do the same thing in parallel and hence cause a deadlock. Additionally it might happen that MUI does not restore the the screen imagery correctly when dragging the object accros the Clock window. You have been warned.

comment:6 Changed 3 months ago by Gregor

Where do you activate the compositing for WB screen? I use all my Mui programs on WB.

The trick with 'nolock' works! By using it there is no freezing with the new Clock version.

But what is the next step to take? Is this a 'bug' in the Clock or Mui or both? Should I report this to A-Eon and ask them to fix something in the Clock…?

comment:7 Changed 3 months ago by Thore Böckelmann

The question is: why does the Clock tool need a global screen lock to update its clock face? MUI does require it because it might draw the drag image anywhere on the screen during drag'n'drop actions, at least outside its own window. The Clock tool should draw within its own window only, at least from my understanding of how to draw a clock…

From my point of view this should be reported to A-Eon. At least former versions of Clock did not require this lock. So why does the current version require it?

comment:8 in reply to:  6 Changed 3 months ago by Thore Böckelmann

Replying to Gregor:

Where do you activate the compositing for WB screen? I use all my Mui programs on WB.

That should be GUI prefs → Effects → Visual effects

comment:9 Changed 3 months ago by Gregor

Activating the compositing works also - no freezes!

I reported this issue in an Enhancer bug thread in Amigans.net, with a link to this ticket.

Thank you very much for your help !-)

comment:10 Changed 3 months ago by Andy Broad

Hi I am investigating this from the A-Eon end so to speak.

I can confirm the bug occurs when clock is running on a non compositing screen and a drag of the clipboard frame in MUI settings is started. It does not occur when the X-Dock clock.dockapp is running instead (uses same clock.gadget for rendering the clock)

What do you mean by a "global screen lock" ? I would guess at one of

LockScreen()
LockScreenGI()
Locklayers()
LockIBase()

but Clock does none of these. Nor as far as I can see does clock.gadget

comment:11 Changed 3 months ago by Thore Böckelmann

Locking the screen either happens via layers/LockLayers() or via intuition/LockScreen(), depending on what is required in certain situations. For drag'n'drop operations LockLayers() should be used on non-composited screens.

comment:12 Changed 3 months ago by Andy Broad

OK so which is MUI using?

Clock is not calling any of the above and not doing any direct rendering.

But as I say the crash only occurs whilts it's running. I've disabled the timer loop that updates the hands so no rendering should be taking place and the crash still occurs!

comment:13 Changed 3 months ago by Andy Broad

Actually I only disabled the rendering trigger by the timer, if I get rid of the timer itself, then the lockup goes. Investigating further…

comment:14 Changed 3 months ago by Thore Böckelmann

I just checked again. For drag'n'drop actions LockLayers() is used on non-composited screens, when using Numeric buttons LockScreen() will be used. I don't remember why different locking mechanisms are used, but I think it made a difference back then.

comment:15 Changed 3 months ago by Andy Broad

OK that's interesting. LockLayers() in the case I'm testing then I would guess.

I have now tracked down the exact line of code that 'locks up' at the Clock end. And it's a peculiar one:

SetGadgetAttrs ((struct Gadget *)OBJ(OBJ_CLOCK), window,NULL,GA_HintInfo,buffer_HintInfo, TAG_END);

This updates the date and time info which is displayed using the gadget help tooltip.

Will check the clock.gadget to see if it's doing anythin odd with that tag.

Last edited 3 months ago by Andy Broad (previous) (diff)

comment:16 Changed 3 months ago by Andy Broad

OK finding s so far at my end:

Clock runs a Timer loop that updates both the display and the hintinfo once a second.

The display is updated with a SetAttrs() RefreshGList() combination (don't ask me why I didn't write this) this doesn't lockup, the HintInfo is updated with a SetGadgetAttrs() call. This is the point that locks up.

Debugging the clock.gadget I see that afer the call to SetGadgetAttrs() the OM_SET method procedes up to a GM_RENDER call and locks up at this point. I replaced that with a more modern DoRender() call but still get the lock up.

I haven't debugged the GM_RENDER to see if there is anything odd in that yet.

By comparison if I drag an icon on workbench Clock pauses at the SetGadgetAttrs() point and the OM_SET method is not called till after the mouse is released and no lockup occurs.

At this stage I can work arround the problem by changing the SetGadgetAttrs() to a SetAttrs() thus avoiding the embedded GM_RENDER call. But this is only a work arround IMHO

comment:17 Changed 3 months ago by Thore Böckelmann

Does GM_RENDER any screen locking internally? If yes, how? Via LockLayer() or via LockScreen()?

comment:18 Changed 3 months ago by Thore Böckelmann

And if GM_RENDER causes any screen locking, the question is: why?

MUI does require the lock, because it will render the drag image anywhere on the screen during drag'n'drop operations, at least it might render it outside the application's own window. That is the only reason for the lock: to not interfere with graphics updates of other applications which correctly render inside their own windows.

comment:19 Changed 3 months ago by Thore Böckelmann

In 6550:

  • masterpop.c: extended the forced screen locking mechanism to choose the kind of locking. This refs #431.

Changed 3 months ago by Thore Böckelmann

Attachment: muimaster_locking.lha added

muimaster.library with extended locking selection

comment:20 Changed 3 months ago by Thore Böckelmann

You might want to try the version of muimaster.library with extended locking selection.

Set ENV:muiscreenlock to "intuition" to force locking via LockScreen().
Set ENV:muiscreenlock to "layers" to force locking via LockLayers().
Set ENV:muiscreenlock to "off" to force no locking at all.

comment:21 Changed 3 months ago by Andy Broad

The render method is very simple, just blit from a backing store into the provided rastport, no locking of any kind.

However, further debugging shows that GM_RENDER is never actually called, the systems locks at the call to DoRender() or if I replace the old style code at the call to ObtainGIRPort() (so proably locking at the equiavalent call to ObtainGIRPort() inside DoRender() I suppose).

Thanks for the updated MUImaster. Testing with this I get lockups with both 'layers' and 'intuition' but naturally not with 'off'.

comment:22 Changed 3 months ago by Thore Böckelmann

Testing with this I get lockups with both 'layers' and 'intuition' but naturally not with 'off'.

That's really bad. LockScreen() is documented to be the "intuition friendly" way.

comment:23 Changed 3 months ago by Andy Broad

ObtainGIRPort() has a call to LockLayerRom() so I'm guessing this is where it hangs at my end. It will wait there till MUI releases it's ScreenLock if I understand it, so it could be useful to work out where MUI is hanging.

There is nothing on serial to suggest a crash as such.

LockScreen() is supposed to intuition friendly, but you are still limited on intuition calls IIRC, don't know exactly which are safe and which not though.

Workbench uses LockScreen() these days for icon dragging and causes no lockup.

WRT to the Clock program, I will implement the "workarround" anyway as it saves a gratuitous redraw of the gadget.

comment:24 Changed 3 months ago by Andy Broad

I have the feeling that any application that displays a gadget update independantly of users input, eg via timer or notification of some kind will potentially cause this lockup (except perhaps MUI based apps).

I tested with Tunenet and if you play a song so that the VUs or the song progress bar are updating then initiate a MUI drag it will lockup.

If you open USBInspector go to the functionailty tab start a MUI drag and drop then insert or remove a USB stick it will also lockup.

comment:25 Changed 3 weeks ago by Thore Böckelmann

In 6567:

  • masterpop.c: added some debug output during screen locking/unlocking. This refs #431.

comment:26 Changed 3 weeks ago by Thore Böckelmann

Please try to reproduce the lock using the attached version of muimaster.library. Please set ENV:muidebug to "POP" and capture the log using a serial terminal.

comment:27 Changed 3 weeks ago by Thore Böckelmann

In 6568:

  • masterpop.c: added more debug output. This refs #431.

Changed 3 weeks ago by Thore Böckelmann

Attachment: muimaster_20200618.zip added

nightly build of muimaster.library including debug symbols

comment:28 Changed 2 weeks ago by Thore Böckelmann

Cc: Andy Broad added

I just checked the complete AmigaOS4 SVN repository for calls of LockScreen() and LockScreenGI(). The disappointing fact is that there is exactly ONE LockScreen() call, and this happens in workbench.library. All other system components seem to use a different approach or their source code is not/no longer available in the AmigaOS4 repository.

To sum it up: from my point of view MUI is not doing anything wrong and the problem must be caused somewhere else. Does anybody have a better explanation?

Changed 2 weeks ago by Andy Broad

Attachment: cap2 added

Log without lockup but muidebug set to POP

comment:29 Changed 2 weeks ago by Andy Broad

I just tried the above muimaster.library and got no lockups when dragging an element. I tried both the TuneNet case and the USB onsertion cases mentioned above. However I noticed that the VU gadets and USBInspector windowwere updating during the test. That doesn't sound like a 'Locked' screen to me. Did the call to the LockScreen() function get removed when adding debug?

[edot] typos

Last edited 2 weeks ago by Andy Broad (previous) (diff)

comment:30 Changed 2 weeks ago by Andy Broad

I just did a quick compare with Workbench and when dragging icons the TuneNet VUs don't update until the icon is released. (even though it doesn't lockup rendering is prevented).

comment:31 Changed 2 weeks ago by Andy Broad

As an aside that test version of muimaster.library locks up if I touch the mouse wheel whilst a MUI window is active! Had to revert to post these as the habbit of using the mouse wheel to scroll is so strong! Seperate bug I know but could indicate a build error in the test version?

comment:32 Changed 2 weeks ago by Andy Broad

BTW if it still applies I had muiscreenlock set to intuition whilst testing

comment:33 Changed 2 weeks ago by Thore Böckelmann

That log does not reveal anything useful, because no screen locking was necessary due to active compositing. In that case even forcing a specific locking approach will do nothing. You can see that just in the very first line. A transparent layer is created with compositing only. A transparent layer does not require any locking because no destructive drawing performed. Only the layer is moved across the screen.

Without compositing the log will contain something like "created bitmap mode $xxxxxxxx" and further references to the chosen locking mode.

Can you please try again with disabled compositing?

Regarding the lockup caused by mouse wheel actions I must have a closer look tomorrow.

comment:34 Changed 2 weeks ago by Andy Broad

Facepam: sorry forgot that the bug only occured with compositing off.

However when I retested with compositing disabled I still didn't see any locking. Here is a fragment from the log

masterpop.c BuildDragImage : created bitmap node 58044330
masterpop.c LIB_MUIP_HandleLo: do not lock screen
masterpop.c LIB_MUIP_HandleLo: start locked stuff
masterpop.c LIB_MUIP_HandleLo: wait for input
masterpop.c LIB_MUIP_HandleLo: handle input
masterpop.c LIB_MUIP_HandleLo: handle input
masterpop.c LIB_MUIP_HandleLo: wait for input

Is the locki mode still controlled via the variable muiscreenlock ?

I had

New Shell process 11
11.NGFSBoot:> getenv muiscreenlock
intuition

comment:35 Changed 2 weeks ago by Thore Böckelmann

ENV:muiscreenlock is still supported. Try to delete ENV:mui/nolock if it exists. If that file exists, then any locking will be disabled without any further feedback.

I really need to add more debug output to these artificial checks and tweaks.

comment:36 Changed 2 weeks ago by Thore Böckelmann

In 6571:

  • masterpop.c: reworked the screen locking mechanism and its enforcement. This should make things lots easier. This refs #431.

Changed 2 weeks ago by Thore Böckelmann

Attachment: muidebug_20200624.zip added

muimaster.library with extended locking selection

comment:37 Changed 2 weeks ago by Thore Böckelmann

Please try again with the latest debug version installed on top of the current nightly build.

This version no longer makes any use of ENV:mui/nolog, only ENV:mui/screenlock (or ENV:muiscreenlock as a fallback) is used.
Before there was no locking active at all in the debug build. This is fixed now.

Changed 2 weeks ago by Andy Broad

Attachment: muilock.cap added

Log with setenv mui/screenlock intuition

comment:38 Changed 2 weeks ago by Andy Broad

Atached is a serial log muilock.cap which logs me draging the "clipboard" box at the bottom of the MUI settings windows, and removing USB mouse whilat USBInspector is open. This causes a hard lockup.

comment:39 Changed 2 weeks ago by Andy Broad

I notice the debug says lock via LockLayers() but I have

12.NGFSBoot:> getenv mui/screenlock
intuition

To be sure of versions I tested with

12.NGFSBoot:> version muimaster.library full
muimaster.library 21.196 (24/06/2020)
Copyright © 2006-2020 by Thore Boeckelmann, Jens Maus [AmigaOS4/PPC] [svn r6570] [private]

comment:40 in reply to:  39 Changed 2 weeks ago by Thore Böckelmann

Replying to Andy Broad:

I notice the debug says lock via LockLayers() but I have

12.NGFSBoot:> getenv mui/screenlock
intuition

Sorry, my fault. The buffer was a bytes too small. Hence the data read from ENV:mui/screenlock was shortened and of course the comparison with "intuition" could never be equal.

I will build a fixed version.

Changed 2 weeks ago by Thore Böckelmann

Attachment: muidebug_20200625.zip added

muimaster.library with extended locking selection

Changed 2 weeks ago by Andy Broad

Attachment: muilock2.cap added

Log with setenv mui/screenlock intuition

comment:41 Changed 2 weeks ago by Andy Broad

So repeated with the new build this time the log refers to UnlockScreen() so I presume it's now correctly using intuition locking. I did the same test with USB Inspector and also a test with TuneNets VU meters displaying. No lockup occured!

comment:42 Changed 2 weeks ago by Thore Böckelmann

Perfect! Now try again with ENV:mui/screenlock unset or being empty. Then intuition mode should be selected automatically.

comment:43 Changed 2 weeks ago by Thore Böckelmann

In 6580:

  • masterpop.c, Numericbutton.c: reworked the screen locking mechanism to be as friendly as possible and to actually perform a screen lock only if it is really required. This refs #431.

Changed 2 weeks ago by Thore Böckelmann

Attachment: muidebug_20200626.zip added

muimaster.library with extended locking selection

comment:44 Changed 2 weeks ago by Thore Böckelmann

Yet another version to test. Please try to produce a lockup with disabled compositing and ENV:mui/screenlock set to "intuition", "layers", "off" and not set at all.

I guess only "layers" will cause a lockup, but that cannot be avoided.

Please note that Numericbutton.mui now also chooses the locking mode depending on whether the active knob is inside or outside the window. Just in case you want to try this class as well.

Changed 13 days ago by Andy Broad

Attachment: mui_20200626_01.cap added

Capture with mui/screenlock set to intuition, off, then layers

Changed 13 days ago by Andy Broad

Attachment: mui_20200626_02.cap added

Capture with mui/screenlock deleted

comment:45 Changed 13 days ago by Andy Broad

I performed tests with mui/screenlock set to

intuition
off
layers

in turn and got a lock in the last case. (mui_20200626_01.cap)

the I deleted the variable and captured log mui_20200626_02.cap this time with no lockup.

The default case does indeed appear to be using intuition when no variable is set.

I didn't test the numericabutton gadget as yet as I don't know where this gadget might be used. MUI Apps wise I have only Odyssey on my X1000 where I'm testing

comment:46 Changed 13 days ago by Thore Böckelmann

Component: undefinedmuimaster.library
Milestone: future release5.0-2020R1
Priority: undecidednormal
Resolution: fixed
Status: newclosed

Intuition based locking is the automaric default for AmigaOS4 now. So this should be fixed now.

Note: See TracTickets for help on using tickets.