Questions about wl_pointer.set_cursor

[Pekka Paalanen]

On Wed, 18 Nov 2020 13:04:04 +0200
Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:

> On Wed, Nov 18, 2020 at 12:10:45PM +0200, Pekka Paalanen wrote:
> > On Tue, 17 Nov 2020 17:38:32 +0200
> > Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:
> >   
> > > Hi everyone!  
> > 
> > Hi Alf
> >   
> 
> Hi Pekka,
> 
> > > I have some questions about the expected behavior of
> > > wl_pointer.set_cursor:
> > > 

...

> > > 3. When, after an enter event, we set a cursor surface that has a
> > >    attached buffer from a previous enter, is there any requirement to
> > >    recommit the buffer? It doesn't seem this is the case from the spec,
> > >    but Weston doesn't like it if we don't recommit. This is likely a bug,
> > >    but just making sure.  
> > 
> > This is an interesting question, and it involves a race. Let's assume a
> > client does this:
> > 
> > 1. wl_pointer.set_cursor(surface)
> > 2. wl_surface.attach(buffer)
> > 3. wl_surface.commit
> > 
> > If the surface already has content before step 1, but that content is
> > incorrect, then if the compositor happens to repaint between steps 1
> > and 3, the cursor will show incorrect for a moment. A glitch.
> > 
> > That glitch does not happen if the order is reversed:
> > 
> > 1. wl_surface.attach(buffer)
> > 2. wl_surface.commit
> > 3. wl_pointer.set_cursor(surface)
> > 
> > However, your question says that this order does not work with Weston.
> > I'm not surprised, it seems a little off nowadays to give content to a
> > surface before giving it a role. xdg-shell outright forbids that, for
> > instance.  
> 
> A clarification: both the first and second sequences you mention work
> with weston, assuming no enter/leave in between. What doesn't work is
> the following:

Yes, what I wrote applies specifically after a (wl_pointer.leave,)
wl_pointer.enter.

There is no reason to issue wl_pointer.set_cursor more than once after
each wl_pointer.enter. If wl_pointer.set_cursor has already been issued
once after a wl_pointer.enter, then the two sequences above are
equivalent and both have a redundant wl_pointer.set_cursor.

> 
> 1. Enter
> 2. wl_surface.attach, wl_surface.commit, wl_pointer.set_cursor (order
> doesn't matter for this scenario, glitches aside)

If the order doesn't matter here, then how is it that it matters at
step 5?

This implies that it's actually some Weston internal issue here rather
than how it handles the protocol. Maybe Weston repaints at least once
while your cursor wl_surface is not the cursor, releasing its buffer
since it's not mapped, or something. It may be as complicated as
needing the surface to first be part of a repaint to have the "buffer
attach flushed to renderer", and then another repaint without the
surface in the scenegraph.

Or not... since releasing the buffer should still leave the GL texture
in the compositor, so while Weston can't use the KMS cursor plane, it
should still GL-composite the cursor.

> 3. Leave
> 4. Enter
> 5. wl_pointer.set_cursor(surface_with_already_attached_buffer)
> 
> After (5) nothing is displayed for the cursor, where I would expect that
> the cursor would use the buffer already attached during (2).

Right.

> I have tried sway and gnome with an test client and they both exhibit
> the expected behavior. I looked a bit more into Weston and have a
> potential fix for this, if this behavior is indeed a bug.

I guess it's ok if the client knows the old content on the surface is
the right cursor image, and if it is not the right cursor image then
attach+commit *must* happen before set_cursor.

There is more than one way to avoid the race, but compositors and
clients need to agree on how it happens. It's also made worse by the
fact that it is probably near-impossible to lose the race because
libwayland batches requests, so there is no opportunity for a
compositor to repaint at the wrong time. Unfortunately, near-impossible
is not impossible.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20201118/7a2b5cfc/attachment.sig>