[PATCH] Made the dnd client actually work

[Kristian Høgsberg]

On Sat, Nov 27, 2010 at 7:26 PM, Joel Teichroeb <joel at teichroeb.net> wrote:
> This patch makes the dnd client work a lot better. There is one small issue.
> If you drag a flower to a place where it is not accepted, it will just be
> gone. I can not find a work around for this because it does not seem that
> Wayland has a way to tell a client if their drag was rejected.
>
> I also fixed a bug where when the window was moved and a shell configure
> event was sent, the new x and y locations where not saved it the window was
> not resized.

Hi Joel,

That's a great patch, it definitely shows off the drag and drop
feature better when you can actually move the flowers around.  There's
a couple of minor problems with patch though:

First, configure git name and email address so the patch has the right
author info.  See here for details:

  http://book.git-scm.com/2_setup_and_initialization.html

Then please split the window.c patch into a separate commit (see my
other email about wayland patches).  Also, the patch mostly follows
the indentation style, but theres a couple of places where it's breaks
(dnd_add_item, missing newline between functions and between variables
and code, and the opening '{' of the functions should be on its own
line. The last return in a function always has an empty line above it,
etc.  Look around in the code and just copy that style).  Also, since
we're now sending a wayland dnd specific blob around, we should change
the offered mime-type to application/x-wayland-dnd-flower or such.
Also, in drag_finish, you can just make the message a stack variable
instead of malloc-ing it, and I'd prefer to send the random values
(petal_count, r1, r2, u, v) instead of the random seed, but that's
really just nitpicking.  Where do the magic numbers (26 and 66) come
from in drop_io_func?.  Last nit-pick: don't mix declarations and code
(int i; in dnd_button_handler).  (And a meta-comment: the above is a
good example of how it's hard to review and comment on a patch when
it's attached - git send-email makes this much easier),

On a more serious note, this part of the patch looks wrong:

@@ -305,8 +353,6 @@ drag_offer_pointer_focus(void *data,
         * allocated. */
        if (!surface) {
                fprintf(stderr, "pointer focus NULL, session over\n");
-               wl_array_release(&dnd_offer->types);
-               free(dnd_offer);
                wl_drag_offer_destroy(offer);
                return;
        }

We need to free the dnd_offer when the dnd session is over, which is
when the drag pointer leaves the surface or when we receive the drop.
It looks like I missed that in the drop_io_func (which you fixed,
maybe this should be a separate patch too), but we still need it in
the case where the pointer leaves the window without dropping.

Anyway thanks, I appreciate it, the overall direction of the patch is
exactly right.  I had a few ideas for future improvements, if you're
interested:

 - We'll need to update the protocol to separate the pointer image
from the dragged item.  This will make it easier for applications to
attach the surface and update the pointer, and it allows the
compositor to animate the "snap-back" to drag origin when the drag is
rejected.

 - Avoid losing flowers when the target rejects the drop.  This means
adding a new 'reject' method to the drag_offer interface and a similar
event to the drag interface.  Or make fd = -1 mean reject and update
the protocol code to handle that.

 - Make a rejected drag "snap" back to the origin (has to be done in
the compositor).  This requires the two above steps so the compositor
can reliably detect a reject drop and so that it has the dragged icon
without the cursor image overlaid.

 - Make the root window send x-wayland/dnd-delete mime type accept
events if the drag source advertises that type and delete the flower
if we drop it there.

 - Advertise image/svg and render the flower to svg using cairos svg
surface, accept image/svg in the image client.  This is a good way to
demonstrate the multi-format dnd, but a bit of work, of course.

Kristian