[8] Review request for 7171045 [macosx] There are no enter or exit events reported against 8b39 for MouseEventsDuringDrag. (original) (raw)
Anthony Petrov anthony.petrov at oracle.com
Tue Aug 28 06:06:43 PDT 2012
- Previous message: [8] Review request for 7171045 [macosx] There are no enter or exit events reported against 8b39 for MouseEventsDuringDrag.
- Next message: [8] Review request for 7171045 [macosx] There are no enter or exit events reported against 8b39 for MouseEventsDuringDrag.
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi Alexander,
Thanks for the clarifications. The code now looks clearer. I'm fine with the fix.
-- best regards, Anthony
On 8/28/2012 3:17 PM, Alexander Scherbatiy wrote:
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/7171045/webrev.05/ The comments are below: - NSTrackingCursorUpdate option is removed from the rolloverTrackingArea On 8/24/2012 7:00 PM, Anthony Petrov wrote: src/macosx/classes/sun/lwawt/macosx/CPlatformWindow.java
66 public static native CPlatformWindow nativeGetTopmostPlatformWindowUnderMouse();
This method must probably be private. Fixed.
Still, I don't see why getTopmostPlatformWindowUnderMouse() must return null when it's invoked on an embedded frame, and why the implementation should even be different at all. The mouse is a global system entity. There's either some window under the mouse, or there's not. I would still think of LWToolkit.getPlatformWindowUnderMouse(), and its implementation in LWCToolkit. Dmitry, who is a responsible person for the CPlatformEmbeddedFrame, said that the implementation of the getPlatformWindowUnderMouse() method could be different for applets. So the getPlatformWindowUnderMouse() method implementation will be filled as a separated issue and fixed by Dmitry. Also, the logic in LWWindowPeer lines 722-743 seems to be overly complicated (as well as the whole dispatchMouseEvent() method). E.g., if the LWWindowPeer manages an embedded frame, we get topmostWindowPeer == this at line 730, and thus always go through the 'true' branch of if() at line 733, even though actually the mouse pointer can be located over some other window at the moment (e.g. over a popup window opened by an applet). I updated the topmostWindowPeer variable creation so it now can have only the topmost window under mouse value or null and added a comment that the (topmostWindowPeer == null ) condition should be removed after the getPlatformWindowUnderMouse() method implementation in the CPlatformEmbeddedFrame class. For now the (topmostWindowPeer == null ) condition allows to track the mouse enter/exit components events as it was before the fix. Overall, it's really difficult to understand what is going on there. I've spent half an hour reading the code and am still not sure if I get it right. Why does LWWindowPeer even care about the EXITED/ENTERED events for components? Shouldn't this code belong to LWComponentPeer? Or even the shared code? How do Swing components in regular Swing apps get the ENTERED/EXITED events then? Why can't we use the same approach for LWAWT? Swing controls have Container in a parent class hierarchy. The Container class has the LightweightDispatcher dispatcher which allows to track mouse moving from one component to another and generate necessary mouse enter/exit events. AWT controls have Component class as a parent and do not have the dispatcher. So moving/dragging a mouse from one AWT control to another does not generate necessary mouse events. The aim of the fix is not redesigning current architecture. It just adds checking a case where a mouse is dragged from one window to another and so the first window which gets the mouse drag events is not the real window for which component enter/exit events should be generated. Thanks, Alexandr. -- best regards, Anthony On 8/24/2012 5:53 PM, Alexander Scherbatiy wrote:
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/7171045/webrev.04/ The comments are below: On 8/23/2012 4:51 PM, Anthony Petrov wrote: Hi Alexandr,
1. In synthesizeMouseEnteredExitedEventsForAllWindows, can we iterate through app's windows only once and send both Exited and Entered events where needed? Now, when we do not care about the order of the mouse enter/exit events generation it is possible to do. I have updated the code. 2. Also, it looks like nativeSynthesizeMouseEnteredExitedEvents no longer requires a window pointer as an argument. Fixed. 3. Here's a major concern: the LWWindowPeer (and other LW* classes) should never import C* classes. Instead you should've added a new method to the PlatformWindow interface, and used it from LWWindowPeer. The CPlatformWindow should've implemented this method, and called an appropriate native method from there. In this case, however, you actually need a static method, so it'd better be part of the LWToolkit interface, and implemented in the LWCToolkit class instead. I see. It seems that the getTopmostWindowUnderMouse method implementation is different for the CPlatformWindow and for the CPlatformEmbeddedFrame. So I added the getTopmostPlatformWindowUnderMouse method to the PlatformWindow interface. Thanks, Alexandr.
-- best regards, Anthony On 08/23/12 15:40, Alexander Scherbatiy wrote: Could someone review the fix? Thanks, Alexandr. On 8/10/2012 6:00 PM, Alexander Scherbatiy wrote:
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/7171045/webrev.02/ The comments are below: On 8/7/2012 6:47 PM, Mike Swingler wrote: I noticed that, NSWindow's windowNumber is NSInteger, which is 32 bits wide on 32-bit and 64 bits wide on 64-bit. Chopping that to a simple 32 bit int is probably not what you intended to do. fixed. Also, have you considered simply calling -[NSWindow setAcceptsMouseMovedEvents:YES] in -[AWTView initWithRect:], and then removing the calls that flip it on and off in -[AWTView mouseEntered:] and -[AWTView mouseExited"]? This would also require a change in JavasunlwawtmacosxCPlatformWindownativeSetNSWindowBounds(), which has a comment that states it only turns on mouse moved events if the window is currently under the mouse. I tried it. The result that I got is that a dragging mouse from one window to another does not generate mouse enter/exit events on the second window in case when tracking rect is used and the acceptsMouseMovedEvents flag is always set to YES.
I would think that AppKit should be more than happy to send you mouse-over/enter/exited events as long as you say you want them. Was this approach tried and didn't work for some reason? Hi, Alexander. Probably all this stuff can be simplified? Can you try to change TrackingRect to TrackingArea with appropriate flags like NSTrackingEnabledDuringMouseDrag etc. I changed the TrackingRect to TrackingArea in the updated fix. It now generates the mouse enter/exit events on the second window during drag. So it is not necessary to generates such events from our side. The getTopmostWindowUnderMouse is necessary to handle mouse enter/exit events for component in case when a mouse is dragged from one window to another. Where the second window contains it's own components and the only way to know about tracking over them are drag events which receives the first window. I think that the LWWindowPeer class code can be simplified if we assume that window mouse enter/exit events are always come to the current window. So the component mouse enter/exit events can be tracked only in the current or topmost window. For this case I separated the global lastCommonMouseEventPeer which is necessary for the cursor manager and the local lastMouseEventPeer. According to the specification: The proper order of mouse-entered and mouse-exited events received by tracking-area objects in an application cannot be guaranteed. https://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/EventOverview/TrackingAreaObjects/TrackingAreaObjects.html
So it seems it is not possible to have one lastMouseEventPeer instead of two: lastCommonMouseEventPeer and lastMouseEventPeer. And there are possible situations that we can receive mouse drag event between enter and exit. To handle this the isMouseOver flag is added to the LWWindowPeer class. So the component mouse enter/exit events are not generated if the window mouse exited event is received and window mouse entered is not. There is the test: test/java/awt/Mouse/EnterExitEvents/ResizingFrameTest.java It shows that even using the TrackingArea some mouse enter/exit events are not generateed in the following cases: - window is created under the mouse - window changes it's bounds so it becomes under the mouse For this cases it still necessary to generate mouse enter/exit events from our side. I just moved the related code to one method synthesizeMouseEnteredExitedEventsForAllWindows. Thanks, Alexandr. 07.08.2012 15:17, Alexander Scherbatiy wrote: bug: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7171045 webrev: http://cr.openjdk.java.net/~alexsch/7171045/webrev.01/ This is a regression after the fix 7154048 [macosx] At least drag twice, the toolbar can be dragged to the left side. In the case where a toolbar is created under a mouse and it is dragged over the initial window the mouse enter/exit events should not be generated for the components which are under the toolbar. The current fix is supposed to solve this regression and also to fix generation of mouse enter/exit events for all cases where a mouse is dragged from one window to another or a new window is created under a mouse (like it is implemented for toolbar). Let's see the following cases 1) Mouse is dragged from a window and back to the same window. The Mac OS X system generates a mouse exit event only the first time when a mouse is dragged from a window and does not generate mouse enter/exit events next times during one drag trace. 2) Mouse is dragged from one window to another. The Mac OS X system does not generate mouse enter/exit events for the second window. 3) Mouse is dragged from one window to another and released. In this case the Mac OS X system generates mouse enter event for the second window only after releasing the mouse. 4) Clicking on window creates a new window after that the new window is dragged (toolbar dragging). However the Java system generates mouse enter/exit events each time when a mouse is dragged over any window. To fix this the following methods are introduced: - Get topmost window under mouse - Synthesize mouse enter/exit events for all windows
The dispatchMouseEvent method from the LWWindowPeer class is handled previous and next components and is able to generate mouse enter/exit events for them. However the the AWTView native class contains isMouseOver flag which value becomes inconsistent if we do not updated it. Because of this the fix generates the mouse enter/exit window events on the native side. Generating mouse enter/exit events always should be in order where mouse exit events are generated before the mouse enter events. The synthesizeMouseEnteredExitedEventsForAllWindows method tries to generate mouse enter/exit events for all windows during mouse drag or window creation/window bounds changing. However only those windows which isMouseOver flag is differ from the isTopMostWindowUnderMouse state are affected. This method also tries to generate both mouse enter and exit event for the same window but only one of them can be applicable because the isTopMostWindowUnderMouse state is not changed for these calls. So the window enter/exit events now are always generated from the native system and component enter/exit events are generated in the LWWindowPeer class. LWWindowPeer class now uses three links to components: current, last and topmostUnderMouse. All of them are necessary because in case when a mouse is dragged from one window to another the current component receives drag events and last and topmostUnderMouse links handle components mouse exit/enter events on the second window. Thanks, Alexandr.
-- Best regards, Sergey.
- Previous message: [8] Review request for 7171045 [macosx] There are no enter or exit events reported against 8b39 for MouseEventsDuringDrag.
- Next message: [8] Review request for 7171045 [macosx] There are no enter or exit events reported against 8b39 for MouseEventsDuringDrag.
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]