GNOME Bugzilla – Bug 762711
GDK W32: Mouse cursor reverts to default during resize operation
Last modified: 2016-03-03 07:20:50 UTC
When one grabs the edge of a CSD window and drags to resize, the cursor might revert from the appropriate resize cursor to the default arrow pointer. As far as i can tell, this is related to grabs. To maintain a non-default cursor while mouse is moving around, potentially outside of the window being resized, we have to make a grab and specify a non-default cursor. Right now GDK explicitly ungrabs (releasing implicit grab that happens on left button being pressed) on resize and lets native W32 WM resize mechanism to do its thing. After bug 761629 is fixed, GDK will no longer ungrab the implicit grab. Implicit grab means that the window correctly gets the mouse messages, which is what 761629 needs to operate, but implicit grab does not do anything to mouse cursor. Therefore, once the mouse leaves the resize grip area, it reverts to default. Non-CSD windows are unaffected, because W32 WM ensures that the cursor is kept in appropriate state, because cursor changes are made on mouse being over window frame, which is completely controlled by W32 WM. For CSD windows native W32 WM resizer is still at work, but it doesn't do anything with the window frame anymore, apparently. I've tried switching from implicit to explicit grab, that didn't work well so far. Might be related to bug 445284.
Some questions i have: GDK calls update_cursor() , which calls: impl_class->set_device_cursor (toplevel, device, cursor); , which is: gdk_win32_window_set_device_cursor() , which calls: GDK_DEVICE_GET_CLASS (device)->set_window_cursor (device, window, cursor); , which is: gdk_device_virtual_set_window_cursor() , which is the actual w32 implementation of the gdk pointer device (not sure why the "other" win32 implementation isn't used, and why it's so incomplete) I've compared gdk_device_virtual_set_window_cursor(), and noted that some of the other implementations don't seem to be using the "window" parameter much, whereas gdk_device_virtual_set_window_cursor() only sets the cursor if mouse pointer is currently over the "window". So even if i do a correct grab (i think i do) and give it a cursor, the implementation won't set that cursor. Is this how cursor setting should *actually* work? If so, how are other platforms doing it?
I don't know how win32 cursors work, but gdk is patterned after X11, and the way it works is: Every (native) window can have a custom cursor set, which the xserver automatically switches to when moving into the window. If its not unset, the nearest parent that has one set gets used. However, if there is an explicit grab, then a cursor can be specified, and while the grab is active a different cursor it used. If a custom cursor is specified, then that will always be used, independent on where the cursor are. But if the icon is not specified, then the "normal" (i.e. as above) icon is used for the grabbed window or its subwindows, but for other windows the cursor from the grabbed window is used. In practice though, with current gdk we always set the custom cursor on the toplevel window, and we always grab on the toplevel, so in terms of the backend, all it has to do is during a grab, always use the specified cursor, or if that is NULL, use the current cursor for the grab window. Implicit grabs are supposed work just like an explicit grab with and undefined cursor, so they should set the cursor to whatever is the current cursor for the toplevel its in.
Trying to add a bit more after alex's accurate coverage. (In reply to LRN from comment #1) > Some questions i have: > GDK calls update_cursor() > , which calls: > impl_class->set_device_cursor (toplevel, device, cursor); gdk_window_set_device_cursor() is the per-pointer way to define the cursor that will be used when it's over that window. Note that with update_cursor() we hold the real action until the conditions meet for a pointer cursor to be updated. > , which is: > gdk_win32_window_set_device_cursor() > , which calls: > GDK_DEVICE_GET_CLASS (device)->set_window_cursor (device, window, cursor); And this vmethod is the one actually interfacing with the windowing. In X11 the window argument is needed for XIDefineCursor, the window argument is granted as right for eg. cases like implicit grabs otherwise. On other windowing systems the pointer cursor is a global resource, so the window argument shouldn't be needed at all. > , which is: > gdk_device_virtual_set_window_cursor() > , which is the actual w32 implementation of the gdk pointer device (not sure > why the "other" win32 implementation isn't used, and why it's so incomplete) > > I've compared gdk_device_virtual_set_window_cursor(), and noted that some of > the other implementations don't seem to be using the "window" parameter > much, whereas gdk_device_virtual_set_window_cursor() only sets the cursor if > mouse pointer is currently over the "window". Knowing nothing about win32 specifics here, this seems wrong under that light. This should be the do-it call, the check for the window belongs a few layers above, in the update_cursor() callers, the (implicit) grab machinery, etc... > > So even if i do a correct grab (i think i do) and give it a cursor, the > implementation won't set that cursor. Is this how cursor setting should > *actually* work? If so, how are other platforms doing it? Hope the above was helpful.
Okay, i've changed virtual device implementation to just set the cursor (it was really weird anyway), and things seem to work...except for one little thing - mouse reentry screws up the cursor. For example, you press on the west resize grip, cursor is "w-resize". Now move the mouse so that it is now over the north-west resize grip, cursor is still "w-resize". Now quickly move mouse to the left. If custom resize code is used (it is, in my case), the change in actual window size is delayed until next redraw, i.e. it doesn't catch up with the location of the mouse right away. If that gap happens to be large enough (you have to be super-fast, but it's doable), mouse actually ends up *outside* of the window. Window gets GDK_NOTIFY_LEAVE. Then the window catches up, and gets GDK_NOTIFY_ENTER. At which point update_cursor() is called. update_cursor() checks which window is under cursor on re-entry, then checks whether that window is a child of the grab (if any). Because we do grab, and do grab on the toplevel, anything inside the window will naturally be a child of toplevel. And because of that GDK uses cursor of the window-under-pointer-at-the-moment-of-reentry instead of grab cursor, even if grab cursor is set. Now, remember that our mouse was over the nw-resize grip area. Because we moved left, when the window catches up, our mouse ends up in nw-resize grip area again. And this time mouse cursor is set to "nw-resize", because that's what is under cursor. Even though the w-resize op is still in progress. It becomes even weirder if you move left-right quickly enough to end up not at the nw-resize grip, but actually *inside* the window proper, in a place like a GtkEntry. GtkEntry has the beam cursor, and that is what you get. Maybe i could screw up with the generation of crossing events to prevent update_cursor() from being called, but i'm not sure what else notify-enter/leave are responsible for, so it doesn't seem like a good idea...
Created attachment 322506 [details] [review] GDK W32: Force correct mouse cursor for custom resize/move This is what i came up with. (BTW, nacho, "I told you so": these fields in the context were useful after all). This applies on top of custom resize patch from bug 761629.
Review of attachment 322506 [details] [review]: Patch looks fine to me but see the comments. ::: gdk/win32/gdkdevice-virtual.c @@ +109,3 @@ GdkWindow *curr_window = gdk_window_get_device_position (window, device, NULL, NULL, NULL); + if (TRUE || curr_window == window || why this TRUE? ::: gdk/win32/gdkwindow-win32.c @@ +2702,3 @@ _gdk_win32_get_window_rect (window, &rect); + switch (op) I would say factor out a method to get the cursor name
(In reply to Ignacio Casal Quinteiro (nacho) from comment #6) > ::: gdk/win32/gdkdevice-virtual.c > @@ +109,3 @@ > GdkWindow *curr_window = gdk_window_get_device_position (window, > device, NULL, NULL, NULL); > > + if (TRUE || curr_window == window || > > why this TRUE? Simpler and neater than commenting out all that else {} code.
(In reply to LRN from comment #7) > (In reply to Ignacio Casal Quinteiro (nacho) from comment #6) > > ::: gdk/win32/gdkdevice-virtual.c > > @@ +109,3 @@ > > GdkWindow *curr_window = gdk_window_get_device_position (window, > > device, NULL, NULL, NULL); > > > > + if (TRUE || curr_window == window || > > > > why this TRUE? > > Simpler and neater than commenting out all that else {} code. With this I guess you mean the patch it is not ready since I would expect a cleanup on this case :)
Created attachment 322716 [details] [review] GDK W32: Force correct mouse cursor for custom resize/move v2: * Axed the old virtual cursor setter code instead of papering it over with if (TRUE) * Put cursor name-choosing code into separate function as suggested. * Handle all cases when chooing cursor name * Some other tweaks and cleanups
Review of attachment 322716 [details] [review]: See the comments. ::: gdk/win32/gdkevents-win32.c @@ +2497,3 @@ + if (impl->drag_move_resize_context.op != GDK_WIN32_DRAGOP_NONE && + impl->drag_move_resize_context.button == button) + gdk_win32_window_end_move_resize_drag (window); this seems unrelated to this patch but more about the drag and resize one. ::: gdk/win32/gdkwindow-win32.c @@ +2685,3 @@ +static const gchar * +get_cursor_name_from_op (GdkW32WindowDragOp op, GdkWindowEdge edge) split params @@ +2715,3 @@ + */ + case GDK_WIN32_DRAGOP_COUNT: + abort (); use g_assert_not_reached @@ +2721,3 @@ + } + + abort (); g_assert_not_reached, and add it to the default case so it is more consistent with other parts of the code @@ +2723,3 @@ + abort (); + + return "should crash by the time it reaches this point"; just add a return NULL
(In reply to Ignacio Casal Quinteiro (nacho) from comment #10) > Review of attachment 322716 [details] [review] [review]: > > See the comments. > > ::: gdk/win32/gdkevents-win32.c > @@ +2497,3 @@ > + if (impl->drag_move_resize_context.op != GDK_WIN32_DRAGOP_NONE && > + impl->drag_move_resize_context.button == button) > + gdk_win32_window_end_move_resize_drag (window); > > this seems unrelated to this patch but more about the drag and resize one. This patch introduces explicit grabs. gdk_win32_window_end_move_resize_drag() calls gdk_device_ungrab(), which is why it's added here. > @@ +2721,3 @@ > + } > + > + abort (); > > g_assert_not_reached, OK > and add it to the default case so it is more > consistent with other parts of the code I deliberately avoided default: case to ensure that gcc warns about unhandled enum values.
Created attachment 322773 [details] [review] GDK W32: Force correct mouse cursor for custom resize/move v3: * Fixed nitpicks * Preserve pointer_grab->native_window before calling ReleaseCapture(), as it might cause WM_CAPTURECHANGED, which will cause an ungrab, which will invalidate pointer_grab. Testing on Windows 10 produced infrequent crashes during drag ops. Gdb says that GTK crashes during crossing event synth, when it tries to find parents of the "src" window. Debugging showed that pointer_grab->native_window gets b0rked during the call to ReleaseCapture(), most likely because pointer_grab gets freed. After caching native_window i was unable to make gtk3-demo crash (hopefully, because i've fixed the problem and not because the crash is that infequent).
Review of attachment 322773 [details] [review]: Let's get this in too.
Attachment 322773 [details] pushed as 84ddc6a - GDK W32: Force correct mouse cursor for custom resize/move