Re: RFR: 8236971: [macos] Gestures handled incorrectly due to missing events
On Thu, 2 Apr 2020 14:55:35 GMT, Kevin Rushforth 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. looks good to me. - Marked as reviewed by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/156
Re: RFR: 8236971: [macos] Gestures handled incorrectly due to missing events
On Mon, 6 Apr 2020 12:28:21 GMT, Ambarish Rapte 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
Re: RFR: 8236971: [macos] Gestures handled incorrectly due to missing events
On Thu, 2 Apr 2020 14:55:35 GMT, Kevin Rushforth 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 888:
> 887: }
> 888: }
> 889: }
In continuation to the change in `maybeBeginGestureWithEvent`, we would need to
remove the inner `if` condition change
here,
if ((gesturesBeganMask & theMask) != 0) {
gesturesBeganMask &= ~theMask;
[self sendJavaGestureEndEvent:theEvent];
}
-
PR: https://git.openjdk.java.net/jfx/pull/156
Re: RFR: 8236971: [macos] Gestures handled incorrectly due to missing events
On Thu, 2 Apr 2020 14:55:35 GMT, Kevin Rushforth 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]
}
-
PR: https://git.openjdk.java.net/jfx/pull/156
Re: RFR: 8236971: [macos] Gestures handled incorrectly due to missing events
On Thu, 2 Apr 2020 14:55:35 GMT, Kevin Rushforth 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. Marked as reviewed by mpaus (Author). - PR: https://git.openjdk.java.net/jfx/pull/156
Re: RFR: 8236971: [macos] Gestures handled incorrectly due to missing events
On Fri, 3 Apr 2020 14:14:36 GMT, Michael Paus wrote: >> These are all separate issues that could be explored with follow-on bugs. >> I'm not sure what the right behavior is for >> these or whether there is enough information from the OS to do anything >> about them (e.g., I suspect there is no way to >> get the touch count for these trackpad-generated gestures on macOS, nor do I >> really think it would be useful). You >> might file a follow-on issue for 1 and 3. >> Would you be able to review this? If not I'll ask @johanvos or >> @pankaj-bansal to be the second reviewer. > > I think 1. is more a point for discussion and I am also not sure what is > right or wrong but 3. is IMHO a bug, > especially because the documentation is so explicit about this behaviour. > I'll file a new issue for that. > What would I have to do to formally review this? I haven't done that before. I agree about 1 being a point of discussion. Please do file a bug for 3. As for formally reviewing it, you would press the `Files changed` tab near the top of the PR, add any inline comments you might have, and then press the `Review changes` pull down. You would add any overall review comments that you have, then select one of `Comment` (to add comments or questions before approving), `Approve` (this means you have reviewed it and are OK with the changes; you will be listed as a reviewer), or `Request changes` (to indicate a bug or other serious problem), and then press `Submit Review`. - PR: https://git.openjdk.java.net/jfx/pull/156
Re: RFR: 8236971: [macos] Gestures handled incorrectly due to missing events
On Fri, 3 Apr 2020 12:35:35 GMT, Kevin Rushforth wrote: >> I've built a fresh JFX, with your changes applied to master, this morning. I >> can confirm that the primary bug seems to >> be fixed with these changes but I have observed some behaviour where I am >> not sure whether it is correct or whether my >> expectation is just wrong. 1. The total delta values are now accumulated >> correctly for the regualar scroll events but >> not for the following generated inertia events. They are still equal to the >> delta values. Is that according to the >> specification? From a practical point of view a programmer who relies on the >> total delta values is probably much >> surprised if the total delta is reset by the inertia events. 2. Is the >> touch-count field only valid for touch-events >> or why is it always zero? 3. The mouse-wheel behaviour is still wrong >> because the total-deltas for mouse-wheel >> generated scroll-events is still not 0 but this has probably another root >> cause. > > These are all separate issues that could be explored with follow-on bugs. I'm > not sure what the right behavior is for > these or whether there is enough information from the OS to do anything about > them (e.g., I suspect there is no way to > get the touch count for these trackpad-generated gestures on macOS, nor do I > really think it would be useful). You > might file a follow-on issue for 1 and 3. > Would you be able to review this? If not I'll ask @johanvos or @pankaj-bansal > to be the second reviewer. I think 1. is more a point for discussion and I am also not sure what is right or wrong but 3. is IMHO a bug, especially because the documentation is so explicit about this behaviour. I'll file a new issue for that. What would I have to do to formally review this? I haven't done that before. - PR: https://git.openjdk.java.net/jfx/pull/156
Re: RFR: 8236971: [macos] Gestures handled incorrectly due to missing events
On Fri, 3 Apr 2020 11:32:12 GMT, Michael Paus wrote: >> @arapte Can you be one of the reviewers? >> >> @mipastgt I ran it against your test application, but if you have other >> tests you could run, that would be helpful. > > I've built a fresh JFX, with your changes applied to master, this morning. I > can confirm that the primary bug seems to > be fixed with these changes but I have observed some behaviour where I am not > sure whether it is correct or whether my > expectation is just wrong. 1. The total delta values are now accumulated > correctly for the regualar scroll events but > not for the following generated inertia events. They are still equal to the > delta values. Is that according to the > specification? From a practical point of view a programmer who relies on the > total delta values is probably much > surprised if the total delta is reset by the inertia events. 2. Is the > touch-count field only valid for touch-events > or why is it always zero? 3. The mouse-wheel behaviour is still wrong > because the total-deltas for mouse-wheel > generated scroll-events is still not 0 but this has probably another root > cause. These are all separate issues that could be explored with follow-on bugs. I'm not sure what the right behavior is for these or whether there is enough information from the OS to do anything about them (e.g., I suspect there is no way to get the touch count for these trackpad-generated gestures on macOS, nor do I really think it would be useful). You might file a follow-on issue for 1 and 3. Would you be able to review this? If not I'll ask @johanvos or @pankaj-bansal to be the second reviewer. - PR: https://git.openjdk.java.net/jfx/pull/156
Re: RFR: 8236971: [macos] Gestures handled incorrectly due to missing events
On Thu, 2 Apr 2020 22:04:55 GMT, Kevin Rushforth 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. > > @arapte Can you be one of the reviewers? > > @mipastgt I ran it against your test application, but if you have other tests > you could run, that would be helpful. I've built a fresh JFX, with your changes applied to master, this morning. I can confirm that the primary bug seems to be fixed with these changes but I have observed some behaviour where I am not sure whether it is correct or whether my expectation is just wrong. 1. The total delta values are now accumulated correctly for the regualar scroll events but not for the following generated inertia events. They are still equal to the delta values. Is that according to the specification? From a practical point of view a programmer who relies on the total delta values is probably much surprised if the total delta is reset by the inertia events. 2. Is the touch-count field only valid for touch-events or why is it always zero? 3. The mouse-wheel behaviour is still wrong because the total-deltas for mouse-wheel generated scroll-events is still not 0 but this has probably another root cause. - PR: https://git.openjdk.java.net/jfx/pull/156
Re: RFR: 8236971: [macos] Gestures handled incorrectly due to missing events
On Thu, 2 Apr 2020 14:55:35 GMT, Kevin Rushforth 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. @arapte Can you be one of the reviewers? @mipastgt I ran it against your test application, but if you have other tests you could run, that would be helpful. - PR: https://git.openjdk.java.net/jfx/pull/156
