GNOME Bugzilla – Bug 435188
Fixing tooltips coordinate translation to be relative to widget->window
Last modified: 2007-05-23 12:33:07 UTC
Currently the documentation advertises the coordinates you receive with the query-tooltip signal to be relative to the widget's allocation. This is kind of vague and should really be relative to widget->window. I checked the current implementation and in two special cases the coordinates provided were not relative to widget->window. These two cases are: 1. Some no-window widgets do have their own input-only window (GtkCheckButton comes to mind). By default motion events are received with coordinates relative to widget->window, but after a button-press the events will start being delivered from the input-only window. Consequently the coordinates are now relative to that input-only window and need to be corrected. 2. For complex widget with their own windows not all events are directly delivered to widget->window, but to one of the subwindows. When we call window_to_alloc() in the tooltips code (please see the patch for more context), the coordinates we pass in are expected to be relative to widget->window, so we have to do subwindow to widget->window translation here. I will attach a patch which fixes these issues. Comments appreciated.
Created attachment 87388 [details] [review] patch corrected the coordinates for both special cases
Comment on attachment 87388 [details] [review] patch corrected the coordinates for both special cases >Index: gtktooltip.c >=================================================================== >--- gtktooltip.c (revision 17628) >+++ gtktooltip.c (working copy) >@@ -463,6 +463,35 @@ find_widget_under_pointer (GdkWindow *wi > child_loc.y = *y; > > gdk_window_get_user_data (window, (void **)&event_widget); >+ event_widget can be NULL here (e.g. if a widget is in destruction phase), please check for that before accessing the widget pointer (if it is non-NULL, event_widget can reasonably be assumed to point to a valid GtkWidget* within gtk programs). >+ /* Some no window widgets do have an event window, >+ * correct the coorindates for that here. >+ */ >+ if (GTK_WIDGET_NO_WINDOW (event_widget) >+ && window != event_widget->window >+ && gdk_window_get_parent (window) == event_widget->window) >+ { >+ gint px, py; >+ >+ gdk_window_get_position (window, &px, &py); >+ child_loc.x += px + event_widget->allocation.x; >+ child_loc.y += py + event_widget->allocation.y; i don't quite get why oyu do this special casing for NO_WINDOW widgets which nest subwindows with depth 1. the below loop should take care of this case as well. >+ } >+ else >+ { >+ /* Translate the coordinates to be relative to widget->window */ >+ while (window && event_widget->window != window) >+ { >+ gint px, py; >+ >+ gdk_window_get_position (window, &px, &py); >+ child_loc.x += px; >+ child_loc.y += py; >+ >+ window = gdk_window_get_parent (window); >+ } note that this loop won't succeed for all widgets. a handle box has subwindows which don't have widget->window in their ancestry. >+ } >+ > if (GTK_IS_CONTAINER (event_widget)) > { > window_to_alloc (event_widget,
(In reply to comment #2) > event_widget can be NULL here (e.g. if a widget is in destruction phase), > please check for that before accessing the widget pointer (if it is non-NULL, > event_widget can reasonably be assumed to point to a valid GtkWidget* within > gtk programs). Ah, yes, of course :) > >+ /* Some no window widgets do have an event window, > >+ * correct the coorindates for that here. > >+ */ > >+ if (GTK_WIDGET_NO_WINDOW (event_widget) > >+ && window != event_widget->window > >+ && gdk_window_get_parent (window) == event_widget->window) > >+ { > >+ gint px, py; > >+ > >+ gdk_window_get_position (window, &px, &py); > >+ child_loc.x += px + event_widget->allocation.x; > >+ child_loc.y += py + event_widget->allocation.y; > > i don't quite get why oyu do this special casing for NO_WINDOW widgets > which nest subwindows with depth 1. the below loop should take care of > this case as well. The depth is kind of arbitrary, I wonder if this also occurs in cases with a higher depth value. Note that the coordinate translation here is different than the one below. This case has been specially designed for the case were X starts to deliver the motion events from a (usually input-only) subwindow which is not equal to widget->window (which for the check button case is a GdkWindow from a parent). Before the motion events were delivered on the widget->window; this switch of event window occurs after for example a button press. In this case we also need to add the allocation.[xy] values in order to get correct coordinates relative to widget->window. > >+ } > >+ else > >+ { > >+ /* Translate the coordinates to be relative to widget->window */ > >+ while (window && event_widget->window != window) > >+ { > >+ gint px, py; > >+ > >+ gdk_window_get_position (window, &px, &py); > >+ child_loc.x += px; > >+ child_loc.y += py; > >+ > >+ window = gdk_window_get_parent (window); > >+ } > > note that this loop won't succeed for all widgets. a handle box > has subwindows which don't have widget->window in their ancestry. Good to know. Is there any recommended way for handling this?
(In reply to comment #3) > (In reply to comment #2) > > i don't quite get why oyu do this special casing for NO_WINDOW widgets > > which nest subwindows with depth 1. the below loop should take care of > > this case as well. > > The depth is kind of arbitrary, I wonder if this also occurs in cases with a > higher depth value. Note that the coordinate translation here is different > than the one below. This case has been specially designed for the case were X > starts to deliver the motion events from a (usually input-only) subwindow which > is not equal to widget->window (which for the check button case is a GdkWindow > from a parent). Before the motion events were delivered on the widget->window; > this switch of event window occurs after for example a button press. In this > case we also need to add the allocation.[xy] values in order to get correct > coordinates relative to widget->window. i still don't think that needs special casing. regardless of what kind a subwindow is that we get coords for, the following logic should work: w = event_window while w and w != widget->window: p = w->parent; translate_coords (w, &x, &y, p); if NO_WINDOW (widget): x += widget->allocation.x y += widget->allocation.y if not w: ignore_tooltip > > note that this loop won't succeed for all widgets. a handle box > > has subwindows which don't have widget->window in their ancestry. > > Good to know. Is there any recommended way for handling this? i don't have a better answer than the above "ignore_tooltip" for this case. that is, if you can't find any widget's widget->window by walking window parents, simply ignore that event for tooltips (or if you propagate ::query_tooltips up the widget ancestry and fail to translate the coordinates at some point). that means we won't support ::query_tooltip on handle boxes for when they are detached. that shouldn't be too much of a problem though, we can add a comment about this to the handlebox/tooltips docs, and users can simply use ::query_tooltip on handle_box->child, which will be a hbox/vbox often enough anyway.
Created attachment 87803 [details] [review] patch based on code snippet in last comment Came up with this patch -- seems to work completely fine!
Comment on attachment 87803 [details] [review] patch based on code snippet in last comment (In reply to comment #5) > Created an attachment (id=87803) [edit] > patch based on code snippet in last comment > > Came up with this patch -- seems to work completely fine! great ;) >+ if (GTK_WIDGET_NO_WINDOW (event_widget)) >+ { >+ child_loc.x += event_widget->allocation.x; >+ child_loc.y += event_widget->allocation.y; >+ } >+ >+ if (!window) >+ return NULL; please add a comment around return that describes why we bail out here (since that'll be non-obvious for most readers). e.g. /* failing to find widget->window can happen for e.g. a detached handle box; * chaining ::query-tooltip up to its parent probably makes little sense, * and users better implement tooltips on handle_box->child. * so we simply ignore the event for tooltips here. */ >+ > if (GTK_IS_CONTAINER (event_widget)) > { > window_to_alloc (event_widget,
r17896.