[PATCH xwayland] xwayland: Always update the wl_pointer cursor on pointer focus

[Jonas Ådahl]

On Fri, Sep 25, 2015 at 12:13:13PM -0500, Keith Packard wrote:
> Jonas Ådahl <jadahl at gmail.com> writes:
> 
> > The reason I didn't do this was that we'd send duplicate set_cursor
> > requests when the cursor actually did change. What the patch does is
> > make it so that the path that actually does change the cursor when it
> > changes according to the X server always does it when needed in
> > Xwayland. I assume we still need to call CheckMotion() on enter to
> > actually keep the state up to date anyway.
> 
> Ok, slightly annoying, but I assume it will work (at least for now).
> 
> > This is what this patch does by introducing the "invalidated" state
> > causing ChangeToCursor to ignore that the cursor didn't change.
> > mipoiner.c has its own didn't-change-checking which is why there is the
> > same type of code added there. Or do you have a suggestion on how to do
> > that without adding API (in both events.c and mipointer.c)?
> 
> Now you made me go read the code. I think we can safely assume at this
> point that essentially every DDX uses mipointer. That means the
> redundant checks for cursor changes in events.c are mostly a waste of
> time.

Sure, if relying on mipointer doing this is fine then why not.

> 
> How about we change DIX to always call pScreen->DisplayCursor on cursor
> changes (and fix the obvious ref count bug that would introduce), and
> then you can happily wrap the screen DisplayCursor function to do
> whatever you like in terms of getting the cursor displayed.

I'd still need to add the 'invalidate' state to mipointer.c here though
(see below).

> 
> One suggestion -- call the wrapped function with a NULL cursor and then
> with the real cursor when you need to get the cursor displayed for the
> entered window. Your DDX would then only set the cursor the second time
> through.

I assume you mean to replace the 'invalidate' state with just setting to
NULL, but I don't think that will work because NULL is a valid cursor
(mipointer setting NULL will cause xwayland-cursor.c to unset the Wayland
cursor) which means we'd get an incorrect cursor state transition. If we
really don't want to fix mipointer and friends to deal with this ("this"
being the ability to deal with incorrect idea of the actual cursor sprite
state), then the other option I see is to add work-arounds to xwayland
(meaning we'd duplicate cursor sprite tracking in xwayland, and then
"force update" whenever we need).


Jonas