Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

output: fix cursor surface position handling #1074

Closed

Conversation

emersion
Copy link
Member

@emersion emersion commented Jun 20, 2018

This is not how it works. s{x,y} are supposed to be added to the current position on commit.

@Ongy
Copy link

Ongy commented Jun 22, 2018

https://github.com/nobled/wayland/blob/master/protocol/wayland.xml#L790 I think we need to move the hotspot instead of moving the actual cursor.

@acrisci
Copy link

acrisci commented Jun 24, 2018

Can you provide a test case to demonstrate what you are trying to fix?

@emersion
Copy link
Member Author

Here's a test case: https://github.com/emersion/wleird/blob/master/cursor.c

wlroots seems broken in very weird ways when setting a buffer position. It seems the old cursor surface doesn't get cleared.

On commit, the surface position is a delta in surface-local
coordinates.
@emersion emersion force-pushed the remove-output-cursor-broken-position branch from 515c36d to 9feed67 Compare June 25, 2018 21:13
@acrisci
Copy link

acrisci commented Jun 25, 2018

It doesn't change the result, but I think you aren't damaging the surface right in your example.

[ 75142.073] wl_pointer@8.button(69, 30508257, 272, 0)
[ 75142.107]  -> wl_surface@11.attach(wl_buffer@13, -50, -50)
[ 75142.125]  -> wl_surface@11.damage(0, 0, 50, 50)
[ 75142.143]  -> wl_surface@11.commit()
[ 75142.779] wl_buffer@13.release()

@emersion
Copy link
Member Author

Hmm. You're probably right, I was assuming damage and damage_buffer were relative to the buffer, not to the surface.

@acrisci
Copy link

acrisci commented Jun 25, 2018

I don't know now. weston-dnd does this for drag icons:

[2079853.632]  -> wl_surface@39.attach(wl_buffer@44, -27, -29)
[2079853.678]  -> wl_surface@39.damage(0, 0, 64, 64)
[2079853.703]  -> wl_surface@39.commit()

@emersion
Copy link
Member Author

Yeah, after discussing on #wayland, the surface damage is relative to the top-left corner of the buffer, after applying the buffer translation.

07:25 <mceier> emersion: x and y in attach are relative to the current buffer's upper left corner, so even if there's non-zero offset (-27, -29) in attach, it still might be (0,0) offset for damage. Though weston always damages whole surface, so it might mean damage_buffer(27, 29, ...) in roundabout way ;)
08:30 <pq> ongy, I think the only libwayland patch by me is https://patchwork.freedesktop.org/patch/204915/ - review would be appreciated since it cannot land without, but it is quite trivial so expect a similar'ish review in return. ;-)
08:33 <pq> mceier, what do you mean by weston always damages whole surface? Do you refer to the lack of EGL buffer_age extension? If so, use the pixman renderer to see actual damage being painted.
08:33 <pq> emersion, the offset from attach gets applied first, then the damage coordinates get evaluated, which means that damaging in negative coordinates never makes sense.
08:34 <pq> emersion, the documentation of wl_surface.commit tries to explain this.
08:47 <mceier> pq: in case of dnd, whole drag_surface is damaged - in dnd.c line 574, wl_surface_damage(dnd_drag->drag_surface, 0, 0, dnd_drag->width, dnd_drag->height);
08:48 <pq> oh that, not the compositor
08:48 <mceier> yeah
08:49 <emersion> hmm
08:50 <pq> if you move the surface (as opposed to adding a new strip of content to the top or left), then it makes sense to damage the whole surface, because no area on the surface will retain its old content
08:51 <emersion> yeah
08:53 <emersion> so damage() damages in surface units, but relative to the buffer, not the surface
08:54 <emersion> well, okay, that just makes sense since attach *moves* the surface relative to its old position
08:54 <pq> well, it depends, are you referring to the old or the new position of the surface
08:55 <emersion> to the new one
08:56 <pq> the idea as spec'd for wl_surface.commit is that the attach with dx,dy gets processed first, and everything else is relative to the new surface position
08:57 <pq> there's also some confusion between damage to the screen and damage to the surface (i.e. partial texture update)
08:58 <emersion> yeah

@emersion
Copy link
Member Author

Closing in favor of #1076

@emersion emersion closed this Jun 30, 2018
@emersion emersion deleted the remove-output-cursor-broken-position branch July 23, 2018 15:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants