ActiveEvent/Object>>currentEvent broken by Morphic-mt.1283?

After loading my development scratch code into the rc1 image I got a really weird dNU where an instvar of PaintCanvas is nil a few bytecodes after it is set to definitely-not-nil. The code is pretty simple -
!PaintCanvas methodsFor: 'event handling' stamp: 'tpr 8/19/2016 18:30'!
selectRectangleStartingAt: aPoint
        "Set selectionRect to a rectangular area starting at the given canvas point."

        | p |
        selectionRect := aPoint extent: 0@0.
        showSelection := true.
        World activeHand showTemporaryCursor: nil.
        Cursor crossHair showWhile: [
                [self currentEvent anyButtonPressed] whileTrue: [
                        p := self cursorPoint.
                        self autoScroll: p.
                        self canvasChanged: selectionRect.
                        selectionRect := aPoint rect: (self screenToCanvas: p).
                        selectionRect := selectionRect intersect: canvasForm boundingBox.
                        self canvasChanged: selectionRect.
                        World doOneCycle]].
        showSelection := false.
        self changed.
        World displayWorld.

        selectionRect area = 0 ifTrue: [selectionRect := nil].
        ^ selectionRect


In PaintCanvas>>selectRectangleStartingAt: we set selectionRect to a rectangle in the first line. On line 8
`self canvasChanged: selectionRect.`
it appears selectionRect is nil. In the debugger it is indeed nil. I can’t see any path that should make it nil on an initial go-round, so something is wrong in the loop that follows, which spins on `self currentEvent anyButtonPressed`
After sticking some counting code in the loop to see if the problem was happening during the first pass I was startled to see it was more usually occurring after 160-180 loops! And to make life even more interesting with some checking code added before each use of the ‘selectionRect’ ivar it appeared to be getting nilled between the bottom of the loop and the immediately following top. This got me worrying about the vm doing something bad as you might imagine.

However, trying this out in an update #15314 image caused no problems, which at least seems to clear the vm of guilt. After much diffing  of recent updates to Morphic I *think* there must be some interaction between Object>>currentEvent (which uses the ActiveEvent global) and Morphic-mt.1283 changes in HandMorph - purely since they alter the usage of Active Event. The symptom seems to centre around the event not being updated properly.

If I revert the HandMorph>>sendFocusEvent:to:clear: & sendEvent:focus:clear: methods to the versions prior to Morphic-mt.1283 then the PaintCanvas code works as before. The bit that really confuses me is that it seems that more event processing must be going on that invokes PaintCanvas>>mouseDown: *within* the loop above - that’s the only current path I can find to nil the selectionRect ivar. Weird.

There is also some possible confusion in the system between Morph>>cursorPoint, which simply gets the currentHand’s lastEvent cursorPoint, and Object>>currentEvent which gives one either the ActiveEvent global or the currentHand’s lastEvent. Seems to me one or the other is giving wrong info.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
CChheecckk yyoouurr dduupplleexx sswwiittcchh..

tim Rowledge wrote

After loading my development scratch code into the rc1 image I got a really weird dNU where an instvar of PaintCanvas is nil a few bytecodes after it is set to definitely-not-nil. The code is pretty simple -
!PaintCanvas methodsFor: 'event handling' stamp: 'tpr 8/19/2016 18:30'!
selectRectangleStartingAt: aPoint
        "Set selectionRect to a rectangular area starting at the given canvas point."

        | p |
        selectionRect := aPoint extent: 0@0.
        showSelection := true.
        World activeHand showTemporaryCursor: nil.
        Cursor crossHair showWhile: [
                [self currentEvent anyButtonPressed] whileTrue: [
                        p := self cursorPoint.
                        self autoScroll: p.
                        self canvasChanged: selectionRect.
                        selectionRect := aPoint rect: (self screenToCanvas: p).
                        selectionRect := selectionRect intersect: canvasForm boundingBox.
                        self canvasChanged: selectionRect.
                        World doOneCycle]].
        showSelection := false.
        self changed.
        World displayWorld.

        selectionRect area = 0 ifTrue: [selectionRect := nil].
        ^ selectionRect


In PaintCanvas>>selectRectangleStartingAt: we set selectionRect to a rectangle in the first line. On line 8
`self canvasChanged: selectionRect.`
it appears selectionRect is nil. In the debugger it is indeed nil. I can’t see any path that should make it nil on an initial go-round, so something is wrong in the loop that follows, which spins on `self currentEvent anyButtonPressed`
After sticking some counting code in the loop to see if the problem was happening during the first pass I was startled to see it was more usually occurring after 160-180 loops! And to make life even more interesting with some checking code added before each use of the ‘selectionRect’ ivar it appeared to be getting nilled between the bottom of the loop and the immediately following top. This got me worrying about the vm doing something bad as you might imagine.

However, trying this out in an update #15314 image caused no problems, which at least seems to clear the vm of guilt. After much diffing  of recent updates to Morphic I *think* there must be some interaction between Object>>currentEvent (which uses the ActiveEvent global) and Morphic-mt.1283 changes in HandMorph - purely since they alter the usage of Active Event. The symptom seems to centre around the event not being updated properly.

If I revert the HandMorph>>sendFocusEvent:to:clear: & sendEvent:focus:clear: methods to the versions prior to Morphic-mt.1283 then the PaintCanvas code works as before. The bit that really confuses me is that it seems that more event processing must be going on that invokes PaintCanvas>>mouseDown: *within* the loop above - that’s the only current path I can find to nil the selectionRect ivar. Weird.

There is also some possible confusion in the system between Morph>>cursorPoint, which simply gets the currentHand’s lastEvent cursorPoint, and Object>>currentEvent which gives one either the ActiveEvent global or the currentHand’s lastEvent. Seems to me one or the other is giving wrong info.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
CChheecckk yyoouurr dduupplleexx sswwiittcchh..

Hi Tim,

that code is tricky in the sense that it is exactly like MVC's "[Sensor anyButtonPressed]" loops but you should favor Morphic's event dispatching style.

Yes, it should still work this way. In the course of fixing strange behavior in Morphic tests, we make set and clear of ActiveWorld, ActiveHand, and ActiveEvent more robust. Nothing what would interfere with this here.

Here is a simpler version:

[self currentEvent hand anyButtonPressed]
        whileFalse: [
                Transcript showln: self currentEvent hand position.
                World doOneCycle].

This works in the following cases:
- Do-it with keyboard shortcut, e.g., in Workspace
- Do-it with mouse/pop-up menu, e.g., in Workspace
- Invoking it in response to any event dispatching, e.g., a button click

This, however, will not work, if you invoke it outside of any event dispatching such as "Project addDeferredUIMessage: [...]".

I would at least add this "hand" call to the event. You cannot be sure to always get a mouse event here.

---------------

So, what could be the issue then? :-/

I noticed that the selection tool in the Paint Editor does not work at all. And the nil-debugger appears *only* if I try to draw a rectangle after trying to select something, which did not work.

So, does the selection tool work for you? I would fix that first.

My tip would be to do something in the paint editor and then hit CMD+. and see whether the stack looks as expected. It seems not and hence selectionRect gets nil'ed by code in PaintCanvas.

My guess is that Scratch can be fixed to work again. It seems to do very strange strings. Especially when having a system window *above* the paint editor, the mouse cursor keeps changing to a cross. :-D

Best,
Marcel

marcel.taeumel wrote

tim Rowledge wrote

After loading my development scratch code into the rc1 image I got a really weird dNU where an instvar of PaintCanvas is nil a few bytecodes after it is set to definitely-not-nil. The code is pretty simple -
!PaintCanvas methodsFor: 'event handling' stamp: 'tpr 8/19/2016 18:30'!
selectRectangleStartingAt: aPoint
        "Set selectionRect to a rectangular area starting at the given canvas point."

        | p |
        selectionRect := aPoint extent: 0@0.
        showSelection := true.
        World activeHand showTemporaryCursor: nil.
        Cursor crossHair showWhile: [
                [self currentEvent anyButtonPressed] whileTrue: [
                        p := self cursorPoint.
                        self autoScroll: p.
                        self canvasChanged: selectionRect.
                        selectionRect := aPoint rect: (self screenToCanvas: p).
                        selectionRect := selectionRect intersect: canvasForm boundingBox.
                        self canvasChanged: selectionRect.
                        World doOneCycle]].
        showSelection := false.
        self changed.
        World displayWorld.

        selectionRect area = 0 ifTrue: [selectionRect := nil].
        ^ selectionRect


In PaintCanvas>>selectRectangleStartingAt: we set selectionRect to a rectangle in the first line. On line 8
`self canvasChanged: selectionRect.`
it appears selectionRect is nil. In the debugger it is indeed nil. I can’t see any path that should make it nil on an initial go-round, so something is wrong in the loop that follows, which spins on `self currentEvent anyButtonPressed`
After sticking some counting code in the loop to see if the problem was happening during the first pass I was startled to see it was more usually occurring after 160-180 loops! And to make life even more interesting with some checking code added before each use of the ‘selectionRect’ ivar it appeared to be getting nilled between the bottom of the loop and the immediately following top. This got me worrying about the vm doing something bad as you might imagine.

However, trying this out in an update #15314 image caused no problems, which at least seems to clear the vm of guilt. After much diffing  of recent updates to Morphic I *think* there must be some interaction between Object>>currentEvent (which uses the ActiveEvent global) and Morphic-mt.1283 changes in HandMorph - purely since they alter the usage of Active Event. The symptom seems to centre around the event not being updated properly.

If I revert the HandMorph>>sendFocusEvent:to:clear: & sendEvent:focus:clear: methods to the versions prior to Morphic-mt.1283 then the PaintCanvas code works as before. The bit that really confuses me is that it seems that more event processing must be going on that invokes PaintCanvas>>mouseDown: *within* the loop above - that’s the only current path I can find to nil the selectionRect ivar. Weird.

There is also some possible confusion in the system between Morph>>cursorPoint, which simply gets the currentHand’s lastEvent cursorPoint, and Object>>currentEvent which gives one either the ActiveEvent global or the currentHand’s lastEvent. Seems to me one or the other is giving wrong info.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
CChheecckk yyoouurr dduupplleexx sswwiittcchh..

Hi Tim,

that code is tricky in the sense that it is exactly like MVC's "[Sensor anyButtonPressed]" loops but you should favor Morphic's event dispatching style.

Yes, it should still work this way. In the course of fixing strange behavior in Morphic tests, we make set and clear of ActiveWorld, ActiveHand, and ActiveEvent more robust. Nothing what would interfere with this here.

Here is a simpler version:

[self currentEvent hand anyButtonPressed]
        whileFalse: [
                Transcript showln: self currentEvent hand position.
                World doOneCycle].

This works in the following cases:
- Do-it with keyboard shortcut, e.g., in Workspace
- Do-it with mouse/pop-up menu, e.g., in Workspace
- Invoking it in response to any event dispatching, e.g., a button click

This, however, will not work, if you invoke it outside of any event dispatching such as "Project addDeferredUIMessage: [...]".

I would at least add this "hand" call to the event. You cannot be sure to always get a mouse event here.

---------------

So, what could be the issue then? :-/

I noticed that the selection tool in the Paint Editor does not work at all. And the nil-debugger appears *only* if I try to draw a rectangle after trying to select something, which did not work.

So, does the selection tool work for you? I would fix that first.

My tip would be to do something in the paint editor and then hit CMD+. and see whether the stack looks as expected. It seems not and hence selectionRect gets nil'ed by code in PaintCanvas.

My guess is that Scratch can be fixed to work again. It seems to do very strange strings. Especially when having a system window *above* the paint editor, the mouse cursor keeps changing to a cross. :-D

Best,
Marcel

Hey Tim,

using "currentHand" instead of "currentEvent" in both places fixes it. Somewhat. Dragging the selection does not work then.

At this point in the code, "self currentEvent" would always be the same event object in the middle of event dispatching. So will "currentHand". Hence, this whileTrue will never end if you ask the same (immutable) event object over and over again. You nil-bug occures if you ivoke another event inside this loop, which will then call some code in PaintCanvas that makes selectionRect nil.

I know that this observable behavior of "currentEvent" changed recently in the course of fixing Morphic some tests. But taking local state for global state is not, in my opinion, the intention in Morphic. If you want to do function-based spinning, use Sensor like MVC does. If you want to do it state-based, use Morph >> #mouseMove: etc. like Morphic does and manage your state. SystemWindow's corner grip morphs do this, for example, when fast-dragging is disabled. Any mixture of these styles, as here in this piece of scratch code, is really tricky to maintain in the long term.

...aaand we do want to make "worlds in worlds" work again, right? ^___^

Best,
Marcel

All good points Marcel -

> On 20-08-2016, at 3:28 AM, marcel.taeumel <[hidden email]> wrote:
>
>>
>> that code is tricky in the sense that it is exactly like MVC's "[Sensor
>> anyButtonPressed]" loops but you should favor Morphic's event dispatching
>> style.

Absolutely; I’d love to get around to rewriting all that stuff but it isn’t going to happen soon. There’s 24 usages of currentEvent in Scratch code. Oh, and at least 10 in the base image code, so I hope you’ve been able to convince yourself they won’t have similar problems!

I used currentEvent on Bert’s advice a couple of years ago and understood that it really should be the current event. If that is no longer true then I see a good chance of quite a few bits of code going wrong and it needs to be trapped to alert people  properly. I’m fairly sure that my original puzzling situation was being caused by new events getting read as part of the doOneCycle, some of them causing changes in the PaintCanvas state and thus running clearMoveState  which nils my selectionRect and thus, Boom. But your changes leave the currentEvent returning the same event (I think) all the time or at least nearly so.

At a wild guess the entire edifice around ActiveEvent and #currentEvent etc are relics of converting to events and ought to be removed in order to force us all to use proper event handling!



>>
>> I noticed that the selection tool in the Paint Editor does not work at
>> all. And the nil-debugger appears *only* if I try to draw a rectangle
>> after trying to select something, which did not work.

Correct. Reverting the two #sendFocus… methods lets it all work.

>>
>> So, does the selection tool work for you? I would fix that first.

It only works when reverting etc.

>> My guess is that Scratch can be fixed to work again.


I certainly hope so ;-) You’ll have a large number of kids chasing you to the ends of the earth if not!


> At this point in the code, "self currentEvent" would always be the same
> event object in the middle of event dispatching. So will "currentHand".
> Hence, this whileTrue will never end if you ask the same (immutable) event
> object over and over again. You nil-bug occures if you ivoke another event
> inside this loop, which will then call some code in PaintCanvas that makes
> selectionRect nil.

Yeah, I think this where the old rule - self current will always be the latest event to have been processed - has been broken.

But yes, the best real answer is to rewrite a couple of dozen complex UI methods. Sigh.


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Don't diddle code to make it faster; find a better algorithm.

Hey Tim,

the use of #currentEvent is, basically, fine in Scratch and in Squeak. It only gets tricky if you do it in a loop like this:

[self currentEvent anyButtonPressed] whileTrue: [
   ...
   self currentWorld doOneCycle].

After doing another cycle, another event dispatching got finished and #currentEvent is again the object it was when entering the method were this loop is running. If you really want the "last mouse event", you should call "self currentHand lastEvent" -- which only keeps track of the last mouse event (no other kinds of events) compared to #currentEvent, which can also have keyboard events or drop events.

In general, using #currentEvent without spawning another world cycle in a loop is fine:

"self currentEvent shiftPressed" (BookMorph >> #showMoreControls)
If you cannot pass the current event as an argument from call to call but want to reuse some piece of code that just accesses that current event.

"menu popUpEvent: self currentEvent in: self world." (DialogWindow >> #offerDialogMenu)
Same here. Note that there *is* a world cycle below but the conditions are quite different: "[menu isInWorld] whileTrue: [self world doOneSubCycle]."

"aMenu popUpEvent: self currentEvent in: self world" (FlapTab >> #setEdgeToAdhereTo:)
Same here. No loop with do-one-cycle.

"...self currentEvent isMouse  and: [self currentEvent isMouseUp]..." (MorphicToolBuilder >> #open:)
Single query, no loop. Fine.

******************

Scratch uses #currentEvent in 22 places. Only these 2 places bother me:

PaintCanvas >> #selectRectangleStartingAt:
ScriptableScratchMorph class >> #fromUser

Because they do event dispatching again in a loop and assume that their own current event changes. You should use "self currentHand lastEvent" here.

There are also 4 step methods:

ScratchNoteSelector >> #step
LibraryItemMorph >> #step
MediaItemMorph >> #step
PaintCanvas >> #step

Which get called outside any event dispatching but separately. These will automatically fall-back to "self currentHand lastEvent" because ActiveEvent will be nil then.

I hope this helps.

Best,
Marcel