New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

surface: redesign state #1076

Merged
merged 14 commits into from Jul 5, 2018

Conversation

Projects
None yet
5 participants
@emersion
Copy link
Member

emersion commented Jun 20, 2018

  • Rename invalid to committed - better ideas welcome!
  • Move subsurface_position in wlr_subsurface
  • Fix sx/sy handling
  • Don't monkey-patch surface_damage and buffer_damage - add a new buffer damage region instead
  • Make current and pending embedded structs
  • Add a previous state
  • Update cursor hotspot with buffer position

Test plan:

  • Make sure damage tracking works on surfaces and subsurfaces (try gnome-calculator)
  • Try with scaled and transformed outputs
  • Try wleird's cursor example

See #1065

@emersion emersion changed the title surface: redesign state [WIP] surface: redesign state Jun 20, 2018

@ammen99

This comment has been minimized.

Copy link
Member

ammen99 commented Jun 21, 2018

Remove subsurface_position

Is there another way to get the subsurface position relative to its parent?

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jun 21, 2018

Is there another way to get the subsurface position relative to its parent?

Yes, this will be moved in wlr_subsurface.

@emersion emersion force-pushed the emersion:redesign-surface-state branch from ca7df31 to b90842a Jun 21, 2018

enum wlr_surface_state_field {
WLR_SURFACE_STATE_BUFFER = 1,
WLR_SURFACE_STATE_SURFACE_DAMAGE = 2,
WLR_SURFACE_STATE_BUFFER_DAMAGE = 2,

This comment has been minimized.

@synaptiko

synaptiko Jun 22, 2018

Should it be 4 here?

This comment has been minimized.

@emersion

emersion Jun 22, 2018

Member

Right, thanks!

@emersion emersion force-pushed the emersion:redesign-surface-state branch from 9de257f to 1fec7cf Jun 25, 2018

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jun 25, 2018

I'm not really happy with the current wlr_surface_state struct. It contains both things from the client (fields from buffer to frame_callback_list) accumulated before commit and things computed on commit (width, height, s{x,y}, and I wanted to add the wlr_buffer?).

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jun 27, 2018

Still not sure how damage should be defined.

  • What should be the (0, 0) point in the damage region? The top-left corner of the buffer or the top-left corner of the surface?
  • Should it be in buffer-local coords or in surface-local coords? We loose precision if we do surface-local coords, and it's doesn't add a lot of complexity in the damage code - just one … / surface->current.scale somewhere, and one wlr_transform_compose(…, surface->current.transform) somewhere else.
@ammen99

This comment has been minimized.

Copy link
Member

ammen99 commented Jun 28, 2018

@emersion, as we're doing the damage for just convenience, I think it should be in surface local coordinates, because that's
what a compositor typically uses. In this case it'll also make sense to make (0,0) the top left corner of the surface. I'm however not sure how to do precision issues, maybe wlr_region_expand?

@ddevault

This comment has been minimized.

Copy link
Member

ddevault commented Jun 28, 2018

It should be buffer local.

@ammen99

This comment has been minimized.

Copy link
Member

ammen99 commented Jun 28, 2018

It should be buffer local.

I'm not trying to argue, just asking out of interest: what makes you think this way?

@ddevault

This comment has been minimized.

Copy link
Member

ddevault commented Jun 28, 2018

Can we just go with "it's been a long day and explaining the full answer is beyond the limits of my patience right now"?

@emersion emersion force-pushed the emersion:redesign-surface-state branch from 6aa8ff5 to ce5334f Jun 28, 2018

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jun 28, 2018

Moved bits that are computed as a delta of two states (the enhanced damage, the surface position, the buffer itself) outside of wlr_surface_state.

Decided to use damage in buffer-local coordinates, so that you can apply scale and transform to this region in the exact same way you render the associated texture.

@emersion emersion force-pushed the emersion:redesign-surface-state branch from ce5334f to 179febc Jul 1, 2018

@emersion emersion changed the title [WIP] surface: redesign state surface: redesign state Jul 1, 2018

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jul 1, 2018

This PR is ready for review.

cc @Ongy @Timidger @ammen99: this is a breaking change.

@ammen99

This comment has been minimized.

Copy link
Member

ammen99 commented Jul 1, 2018

So, after this, what is the intended solution for #1065?

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jul 1, 2018

So, after this, what is the intended solution for #1065?

Let's continue to discuss this in the other issue.

@ddevault

This comment has been minimized.

Copy link
Member

ddevault commented Jul 1, 2018

swaywm/sway#2185

Code looks good, running this as my daily driver for a while.

@atomnuker

This comment has been minimized.

Copy link
Contributor

atomnuker commented Jul 2, 2018

Pointer position is still off after applying this and the sway update PR. Do you compensate for scaling?

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jul 2, 2018

Pointer position is still off after applying this and the sway update PR. Do you compensate for scaling?

Input doesn't deal with scaling and transform. Can you describe how to reproduce? Are you sure you're running with the latest version of this branch? (A previous commit fucked up the input region)

@atomnuker

This comment has been minimized.

Copy link
Contributor

atomnuker commented Jul 2, 2018

Yep, this is with the latest version of this PR. There's no transform on anything, just scaling set in sway.
You've reproduced this before and I have mentioned this before too. Hotspot is fine, the image matches what's on the screen and sent as input, its just that if you open a right click menu its offset compared to the pointer location.

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jul 2, 2018

FYI I have input issues with some floating windows on Sway master (without this patch applied): swaywm/sway#2195

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jul 2, 2018

Hotspot is fine, the image matches what's on the screen and sent as input, its just that if you open a right click menu its offset compared to the pointer location.

Ah, so this is not a regression: this is a bug also there on master? How big is this offset?

@atomnuker

This comment has been minimized.

Copy link
Contributor

atomnuker commented Jul 2, 2018

In GTK - a few pixels (3 or 4) upwards and a pixel or two to the left of where the menu is created.
In xwayland (such as firefox), 6 or 7 pixels upwards (or downwards, if you try to create the menu too far down the screen) and about 1 or two pixels to the left.
Yes, this isn't a regression, its always been present, but since this commit touched some position code I thought it might also fix this issue.

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jul 3, 2018

Yes, this isn't a regression, its always been present, but since this commit touched some position code I thought it might also fix this issue.

I think this is more of a xdg-shell window geometry issue (this PR only updates the wl_surface implementation).

@emersion emersion force-pushed the emersion:redesign-surface-state branch from 179ac58 to daaaa80 Jul 3, 2018

emersion added some commits Jun 20, 2018

surface: don't clip input and opaque regions
These can be set to e.g. regions larger than the surface. If the
surface resizes itself, it doesn't need to set again these regions.

@emersion emersion force-pushed the emersion:redesign-surface-state branch from c0d476b to 4c9805f Jul 4, 2018

@emersion emersion force-pushed the emersion:redesign-surface-state branch from 4c9805f to 515d682 Jul 4, 2018

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jul 5, 2018

Bump: I've been using this for a few days.

@ddevault

This comment has been minimized.

Copy link
Member

ddevault commented Jul 5, 2018

aight

@ddevault ddevault merged commit 7c6588d into swaywm:master Jul 5, 2018

1 check passed

builds.sr.ht builds.sr.ht job completed successfully
Details

@emersion emersion deleted the emersion:redesign-surface-state branch Jul 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment