RFR: 8236971: [macos] Gestures handled incorrectly due to missing events

[Kevin Rushforth]

On Mon, 6 Apr 2020 12:28:21 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

>> As noted in the JBS issue, this bug is a result of a deliberate change by Apple that affects applications (in this case
>> the JDK) linked against MacOSX 10.11 SDK or later -- they no longer call two methods that we are relying on,
>> `beginGestureWithEvent` and `endGestureWithEvent`. There is no deprecation warning or any other indication at compile
>> time or runtime that these methods are ineffective. They just stopped calling them. It is documented in their developer
>> guide:
>> [developer.apple.com/documentation/appkit/nsresponder/1526368-begingesturewithevent](https://developer.apple.com/documentation/appkit/nsresponder/1526368-begingesturewithevent?language=objc)
>> Note that it's the version of the MacOSX SDK that the JDK is linked with that matters. JavaFX gesture notification
>> works when run on a JDK that was linked against the 10.10 SDK or earlier (regardless of what MacOSX SDK was used to
>> link the JavaFX libraries). JavaFX gesture notification fails when run on a JDK that was linked against the 10.11 SDK
>> or later.   The solution, as indicated in the Apple documentation referred to above, is to use the phase information
>> from gesture events to track when to call begin / end gesture.  The fix does the following things: 1. Removes the
>> `beginGestureWithEvent` and `endGestureWithEvent` responder methods in `GlassView2D` and `GlassView3D` 2. Calls new
>> local methods from each gesture to track when to call the associated `GlassViewDelegate` notification methods,
>> `sendJavaGestureBeginEvent` and `sendJavaGestureEndEvent`  I've tested this on macOS 10.13.6 and 10.15.3 (Catalina) and
>> the attached program now runs correctly. I also tested the GestureEvent sample in Ensemble8 and it reproduces the
>> problem before the fix and works correctly after the fix.  I instrumented the code with print statements (which I have
>> since reverted) and verified that the stream of events is as expected, and matches JDK 8u241 with bundled FX.
>
> modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.m line 868:
> 
>> 867:         }
>> 868:         gesturesBeganMask |= theMask;
>> 869:     }
> 
> Rotate and Magnify gestures can be started and performed together.
> If Rotate is already in progress then `gesturesBeganMask` would be set to `4`.
> and If we start a Magnify gesture while the Rotate is not ended then `sendJavaGestureBeginEvent` will not be executed
> for Magnify gesture. However the application currently receives the synthetic `ZOOM_STARTED` event, which is generated
> as a result of call to `sendJavaGestureEvent` and not an event generated from here,
> A change in inner `if` statement as below can fix the issue,
> 
> if ((gesturesBeganMask & theMask) == 0) {
>     gesturesBeganMask |= theMask;
>     [self sendJavaGestureBeginEvent:theEvent]
> }

This is a good thought, and one I had initially considered, but didn't do for two related reasons:
1. The macOS NSEvent code that sends `beginGestureWithEvent` and `endGestureWithEvent` when the JDK is compiled with an
older SDK doesn't send a begin call when starting a rotate gesture if a zoom gesture is already active, or vice versa.
One goal of this change is to match the previous behavior to minimize risk. 2. More importantly, the existing logic is
not expecting nested `sendJavaGestureBeginEvent` / `sendJavaGestureEndEvent` calls, and will not track the state
correctly if they are nested. I did a quick test implementing your suggestion and confirmed that it misbehaves.

Worth noting, the logic that synthesizes the begin and end gesture notifications that are sent by the Java gesture code
already takes care of the case of switching from zoom to rotate and sends the correct sequence to the event listener.

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

PR: https://git.openjdk.java.net/jfx/pull/156