Re: Tooltips patch [take 1]

On Fri, 19 Jan 2007, Kristian Rietveld wrote:

Hi Tim,

On Tue, Jan 09, 2007 at 02:51:56PM +0100, Tim Janik wrote:

erm, this paints a box at the size the window would like to have, not
necessarily the size it has. size requisition is always a bad figure
to use for painint, people should always only use the actual allocation
which may be gotten via widget->allocation or gdk_window_get_size.

Fully agree, but actually this code was taken from the old tooltips
implementation.  Was there a reason to do it like this there?

not sure, maybe tooltips used to resize and show the toplevel window directly when this code was written.

+static GtkWidget *
+find_widget_under_pointer (GdkWindow *window,
+			   gint      *x,
+			   gint      *y)
+  GtkWidget *event_widget;
+  struct ChildLocation child_loc = { NULL, NULL, 0, 0 };
+  child_loc.x = *x;
+  child_loc.y = *y;
+  gdk_window_get_user_data (window, (void **)&event_widget);
+  if (GTK_IS_CONTAINER (event_widget))
+    {

huh? event->window may as well belong to a non-container widget,
so i think you need to use GTK_IS_WIDGET instead here.

Then we would call gtk_container_foreach() with a non-container

that's an implementation detail, child_location_foreach() has the
correct GTK_IS_CONTAINER check to avoid recursion on non-containers.
the logic of child_location_foreach() still needs to be executed
for the event_widget here, regardless of whether it's a container
or not. you can get that behaviour by calling child_location_foreach()
directly initially, instead of indirectly through gtk_container_foreach.

+  else
+    {
+      guint cursor_size;
+      gdk_window_get_pointer (gdk_screen_get_root_window (gtk_widget_get_screen (tooltip_widget)), &x, &y, NULL);

as pointed out in an above note, we did 'get_pointer' already.
each _get_pointer is a server round trip, so we should reuse the coords
from above.
also, the coords we really want to use are in the event structure, to
avoid bugs where a slow application/connections causes the tooltip to be
positioned completely off-target because the mouse moved since the event
was originally queued (happens regularly here with v1.2.1 for instance).

that was meant to say "gaim v1.2.1" btw

but well, for the moment we don't have those here...

I changed the code to cache the coordinates from the last event and
eliminated all three roundtrips in this function.


thanks for the new patch. all in all, the code looks fairly complete already.
there're some logic issues that still need to be worked out, but
that should be done in SVN. please check this in as soon as possible,
if you want to keep the history, even before fixing my above comments.

Cool, I would say we check in the current patch with the fresh changes
on my disk and look at the last coordinate translation issue mentioned
above in svn.  If there are no objections I hope to check this in
(together with doc comments :) at the end of next weekend or probably in
the weekend following that week.

ok, check it in already! ;)




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]