On Sat, Nov 27, 2010 at 7:26 PM, Joel Teichroeb <[email protected]> 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 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
