[weston] dnd: reset the mouse cursor after adding a new item

Submitted by Derek Foreman on Nov. 18, 2014, 10:24 p.m.

Details

Message ID 1416349467-24442-1-git-send-email-derekf@osg.samsung.com
State Superseded
Headers show

Not browsing as part of any series.

Commit Message

Derek Foreman Nov. 18, 2014, 10:24 p.m.
When ending a drag in the window the cursor will be wrong until the mouse
is moved again.  This is because the item being dragged isn't added
until after the enter event.

This patch stores the input device that last entered the window, and
sets its mouse cursor as new items are dropped.

Closes one of the issues in bug 56298
https://bugs.freedesktop.org/show_bug.cgi?id=56298

Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
---
 clients/dnd.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Patch hide | download patch | download mbox

diff --git a/clients/dnd.c b/clients/dnd.c
index 956c306..332d137 100644
--- a/clients/dnd.c
+++ b/clients/dnd.c
@@ -50,6 +50,7 @@  struct dnd {
 	struct item *items[16];
 	int self_only;
 	struct dnd_drag *current_drag;
+	struct input *last_input;
 };
 
 struct dnd_drag {
@@ -508,10 +509,20 @@  dnd_enter_handler(struct widget *widget,
 	struct dnd *dnd = data;
 
 	dnd->current_drag = NULL;
+	dnd->last_input = input;
 
 	return lookup_cursor(dnd, x, y);
 }
 
+static void
+dnd_leave_handler(struct widget *widget,
+		  struct input *input, void *data)
+{
+	struct dnd *dnd = data;
+
+	dnd->last_input = NULL;
+}
+
 static int
 dnd_motion_handler(struct widget *widget,
 		   struct input *input, uint32_t time,
@@ -564,6 +575,13 @@  dnd_receive_func(void *data, size_t len, int32_t x, int32_t y, void *user_data)
 			   message->seed);
 
 	dnd_add_item(dnd, item);
+	if (dnd->last_input) {
+		int32_t x, y;
+
+		input_get_position(dnd->last_input, &x, &y);
+		input_set_pointer_image(dnd->last_input,
+					lookup_cursor(dnd, x, y));
+	}
 	window_schedule_redraw(dnd->window);
 }
 
@@ -627,6 +645,7 @@  dnd_create(struct display *display)
 
 	widget_set_redraw_handler(dnd->widget, dnd_redraw_handler);
 	widget_set_enter_handler(dnd->widget, dnd_enter_handler);
+	widget_set_leave_handler(dnd->widget, dnd_leave_handler);
 	widget_set_motion_handler(dnd->widget, dnd_motion_handler);
 	widget_set_button_handler(dnd->widget, dnd_button_handler);
 	widget_set_touch_down_handler(dnd->widget, dnd_touch_down_handler);

Comments

I think the dnd client needs to handle normal mouse enter events and set 
the cursor. If the server is not sending an enter right after the drop 
then this is a bug, right?

On 11/18/2014 02:24 PM, Derek Foreman wrote:
> When ending a drag in the window the cursor will be wrong until the mouse
> is moved again.  This is because the item being dragged isn't added
> until after the enter event.
>
> This patch stores the input device that last entered the window, and
> sets its mouse cursor as new items are dropped.

> +		input_get_position(dnd->last_input, &x, &y);
> +		input_set_pointer_image(dnd->last_input,
> +					lookup_cursor(dnd, x, y));

Can't you just use the xy from the dnd event here?
On 18/11/14 05:08 PM, Bill Spitzak wrote:
> I think the dnd client needs to handle normal mouse enter events and set
> the cursor. If the server is not sending an enter right after the drop
> then this is a bug, right?

When dnd_drop_handler triggers (which could be before the enter event
every time - I haven't checked) it calls input_receive_drag_data() with
a callback function that will add the item.

So even if the drop and the enter always occur in a convenient order,
the drag data arrives a little later when it's too late for the enter
handler to set the cursor.

Unless I'm misunderstanding all of this of course.  ;)

So... I don't think it's a bug in the server.

> On 11/18/2014 02:24 PM, Derek Foreman wrote:
>> When ending a drag in the window the cursor will be wrong until the mouse
>> is moved again.  This is because the item being dragged isn't added
>> until after the enter event.
>>
>> This patch stores the input device that last entered the window, and
>> sets its mouse cursor as new items are dropped.
> 
>> +        input_get_position(dnd->last_input, &x, &y);
>> +        input_set_pointer_image(dnd->last_input,
>> +                    lookup_cursor(dnd, x, y));
> 
> Can't you just use the xy from the dnd event here?

I'm pretty sure the mouse may have already moved - exactly the same
reason as above, this handler is getting the drop co-ords but time has
passed since the drop.

So in theory I could have moved the mouse just far enough to not be on
an item and stopped moving before we hit that code, and I'll end up
pointing in space with the hand cursor.

I'm not sure I'm capable of doing that stupid-mouse-trick, but I think
it can happen...
On 11/19/2014 07:16 AM, Derek Foreman wrote:

> I'm pretty sure the mouse may have already moved - exactly the same
> reason as above, this handler is getting the drop co-ords but time has
> passed since the drop.

You are right about that. I'm not clear on how input_get_position is 
implemented, did the client record the position of each input, or is 
this some sort of round-trip to the server? If the latter it probably 
should be done differently such as recording it in the motion handler.

I think you need to change all input cursors when dnd_add_item is 
executed for any reason. Any of them may have changed from a pointer to 
a hand. Imagine if there are two pointers and one is used to drop the 
data at the same location the other is pointing at. So there is no need 
to record the last input, you have to change them all.

You have to not change the mouse cursor on move and enter events while 
waiting for the data. You must not lose the dragged flower until the 
actual flower appears.

If there are multiple pointers, it actually looks like it is best if you 
don't change any of them while waiting for the drop data. This will 
reduce the number of transitions they go through. It also appears there 
is a need for stacking order of pointers, the drop one should be on the 
bottom?

To really remove any possibility of flicker the client also needs to 
enter pointer-lock mode (assuming it works as I want, which is to make 
no changes but to stop moving the cursor except in response to 
positioning commands from the client). This will stop the drop cursor 
image from moving away from the drop location if the user moves the 
mouse before the drop finishes. This may need some server support so the 
cursor stops moving at the time of the drop.
Hi,

On 19 November 2014 22:41, Bill Spitzak <spitzak@gmail.com> wrote:

> On 11/19/2014 07:16 AM, Derek Foreman wrote:
>
>> I'm pretty sure the mouse may have already moved - exactly the same
>> reason as above, this handler is getting the drop co-ords but time has
>> passed since the drop.
>>
>
> You are right about that. I'm not clear on how input_get_position is
> implemented, did the client record the position of each input, or is this
> some sort of round-trip to the server? If the latter it probably should be
> done differently such as recording it in the motion handler.
>

The client records the position. There are no round-trips in the Wayland
protocol.

Cheers,
Dan
On 19/11/14 04:41 PM, Bill Spitzak wrote:
> On 11/19/2014 07:16 AM, Derek Foreman wrote:
> 
>> I'm pretty sure the mouse may have already moved - exactly the same
>> reason as above, this handler is getting the drop co-ords but time has
>> passed since the drop.
> 
> You are right about that. I'm not clear on how input_get_position is
> implemented, did the client record the position of each input, or is
> this some sort of round-trip to the server? If the latter it probably
> should be done differently such as recording it in the motion handler.
> 
> I think you need to change all input cursors when dnd_add_item is
> executed for any reason. Any of them may have changed from a pointer to
> a hand. Imagine if there are two pointers and one is used to drop the
> data at the same location the other is pointing at. So there is no need
> to record the last input, you have to change them all.

Multi pointer.  Sweet.  Guess what, it's not just the cursor that's
messed up - the translucency of the dragged item may be wrong if an item
gets dropped beneath it, or picked up from beneath it.

> You have to not change the mouse cursor on move and enter events while
> waiting for the data. You must not lose the dragged flower until the
> actual flower appears.

I don't understand this requirement - if I don't change the mouse cursor
on enter events while waiting then the cursor will be wrong if a flower
is right at the edge of the window.

I need to make sure I don't change the mouse cursor of any mouse that's
actually dragging an object, I think...  But they should receive a leave
event pretty much immediately at drag start.  Except they don't - if
you're moving the mouse and pick up an item on the fly the cursor is wrong.

I guess because the drag start involves creating the surface to be
dragged, so motion can occur between the click and that actually
happening in the server.

> If there are multiple pointers, it actually looks like it is best if you
> don't change any of them while waiting for the drop data. This will
> reduce the number of transitions they go through. It also appears there
> is a need for stacking order of pointers, the drop one should be on the
> bottom?

I think pointer stacking order is probably a different issue and won't
be dealt with in the dnd example client?

Not sure at all how that should be handled, if at all.  Who's on top:
Last click?  Last enter?  Last motion?  And, ultimately, does anyone
care enough to define what "correct" pointer stacking order should be? :)

> To really remove any possibility of flicker the client also needs to
> enter pointer-lock mode (assuming it works as I want, which is to make
> no changes but to stop moving the cursor except in response to
> positioning commands from the client). This will stop the drop cursor
> image from moving away from the drop location if the user moves the
> mouse before the drop finishes. This may need some server support so the
> cursor stops moving at the time of the drop.

The drop cursor image is just the four way arrow, I don't think it's a
really big deal if it diverges a bit from where the dropped item is
about to appear?  The cursor sticking in place would probably be more
annoying...