Re: long standing dnd bug fixes
- From: Owen Taylor <otaylor redhat com>
- To: Tim Janik <timj gtk org>
- Cc: Gtk+ Developers <gtk-devel-list gnome org>
- Subject: Re: long standing dnd bug fixes
- Date: 04 Jan 2002 13:11:33 -0500
Tim Janik <timj gtk org> writes:
> hi owen,
>
> i got annoyed enough by my dnd-bug workaround code in beast to actually
> sat down and fix the mroe serious bugs in gtkdnd.c.
> the patch to gtk_drag_find_widget() is appended, it adresses:
> 1) dnd code messing up if the widget tree changes during motion (happens
> if a target-indicator widget is added/removed under the drag icon)
This part looks reasonable. (Thought I remembered doing this,
but if I did, it's long gone.)
> 2) wrong drop position calculations within scrolled windows that have
> non-0 scroll offsets (and probably other cases where parent->window !=
> gtk_widget_get_parent_window (child)).
I'm really confused by this:
- My code looks correct
- I don't understand your code.
- A simple test case (sticking testdnd in a scrolled window)
works fine with the old code.
There is a small bug in the current code if there is a window
widget with it's window not at allocation->x,y (back when
buttons were window widets, they did this when there was
a border width, may be other widgets do the same.)
> + GdkWindow *pwindow = gtk_widget_get_parent_window (widget);
> + /* translate offset relative to widget's parent window */
> while (window != widget->parent->window)
> {
> - gint tx, ty, twidth, theight;
> - gdk_window_get_size (window, &twidth, &theight);
> + wlist = g_slist_prepend (wlist, window);
> + window = gdk_window_get_parent (window);
> + }
> + for (slist = wlist; slist; slist = slist->next)
> + {
> + window = slist->data;
> + gdk_window_get_position (window, &tx, &ty);
> + x_offset += tx;
> + y_offset += ty;
> + }
So, x_offset/y_offset are the total offset between
widget->parent->window and ->window.
> + g_slist_free (wlist);
> + wlist = NULL;
What's the point of the list - gdk_window_get_position()
isn't going to change the heirarchy, and order doesn't
matter for addition....
At this point, coordinates of new_allocation are relative
to pwindow.
+ window = widget->window;
> + while (window != pwindow)
> + {
> + wlist = g_slist_prepend (wlist, window);
> + window = gdk_window_get_parent (window);
> + }
> + for (slist = wlist; slist; slist = slist->next)
> + {
> + window = slist->data;
This is a bit of a silly list. pwindow is either widget->window
or gdk_window_get_parent (window)... So, you could just
write if (window != pwindow) {}
> + gdk_window_get_position (window, &tx, &ty);
> + gdk_window_get_size (window, &twidth, &theight);
> + x_offset += tx;
> + y_offset += ty;
Wait, we've already added in this position in the previous
loop...
> + if (tx > new_allocation.x)
> {
> - new_allocation.width += new_allocation.x;
> + new_allocation.width -= tx - new_allocation.x;
> new_allocation.x = 0;
> + new_allocation.width = MIN (new_allocation.width, twidth);
> + }
> + else
> + {
> + new_allocation.x -= tx;
> + new_allocation.width = MIN (new_allocation.width, twidth - new_allocation.x);
> }
> - if (new_allocation.y < 0)
> + if (ty > new_allocation.y)
> {
> - new_allocation.height += new_allocation.y;
> + new_allocation.height -= ty - new_allocation.y;
> new_allocation.y = 0;
> + new_allocation.height = MIN (new_allocation.height, theight);
> }
> + else
> + {
> + new_allocation.y -= ty;
> + new_allocation.height = MIN (new_allocation.height, theight - new_allocation.y);
> + }
OK, so this part seems to translate correctly from coordinates
relative to pwindow to coordinates relative to window.
But, we've lost the clipping to windows between widget->parent->window and
pwindow, which means you may be able to drop on child widgets which are not
visible.
>
> + new_data = *data;
> + new_data.x -= x_offset;
> + new_data.y -= y_offset;
> + new_data.found = FALSE;
> + new_data.toplevel = FALSE;
> +
> if (data->toplevel ||
> - ((data->x >= new_allocation.x) && (data->y >= new_allocation.y) &&
> - (data->x < new_allocation.x + new_allocation.width) &&
> - (data->y < new_allocation.y + new_allocation.height)))
> + ((new_data.x >= new_allocation.x) && (new_data.y >= new_allocation.y) &&
> + (new_data.x < new_allocation.x + new_allocation.width) &&
> + (new_data.y < new_allocation.y + new_allocation.height)))
In the old code we were comparing coordinates relative to
widget->parent->window, here we are comparing coordinates relative to
widget->window. It doesn't matter one way or the other.
> @@ -1306,16 +1353,14 @@ gtk_drag_find_widget (GtkWidget *w
> {
> data->found = data->callback (widget,
> data->context,
> - data->x - new_allocation.x,
> - data->y - new_allocation.y,
> + new_data.x - new_allocation.x,
> + new_data.y - new_allocation.y,
> data->time);
Again, here the difference is just in what coordinate system we
are working in.
If you could come up with a modification to testdnd that shows
the coordinate problems, I'd appreciate it. I'll check in
some fixes now that add some comments and fixes the problem
with window widgets with borders.
> btw, i suspect my new (like your old) code to still be broken for
> handleboxes, since the position calculation relies on parent->window
> and widget->window to form a chain via gdk_window_get_parent().
I'll do a quick fix to fix the infinite loop. I think the long term
fix is to make GtkHandleBox gtk_widget_reparent() it's child widget
into a GtkWindow. Anything else just violates all sorts of GTK+
invariants.
Regards,
Owen
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]