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

Choose more consistent commit behavior #2098

Open
emersion opened this issue Apr 2, 2020 · 5 comments
Open

Choose more consistent commit behavior #2098

emersion opened this issue Apr 2, 2020 · 5 comments
Labels
output proposal surface

Comments

@emersion
Copy link
Member

emersion commented Apr 2, 2020 •
edited

Right now we have two commit events: one for wlr_surface, one for wlr_output. Each of these structs have a state struct with a pending and current field.

When commit is emitted, I'd expect these assumptions to be true:

  1. Users should have access to the committed state in the current state
  2. Users should be able to figure out what changed in this commit (how?)
  3. The pending state should be empty (it's just been applied)

Questions:

  • Should we allow access to the previous state? Note that we don't want to retain locks on buffers longer than necessary.
  • pending.committed is a bitmask of fields changed since last commit. What should current.committed mean? Right now it seems like it's the union of all commits' pending.committed.

Right now (1) is true, however:

  • wlr_output clears the pending state after commit is emitted, breaking (3). The pending state is used by users to figure out what changed (2). wlr_output provides no access to the previous state.
  • wlr_surface has a previous state, however previous.buffer is an exception and is always set to NULL. (3) is true. Also, the previous state seems to be only used by wlr_surface itself. Users have no way to figure out what changed (2).

It would be nice to decide on a consistent behavior.

wlroots has migrated to gitlab.freedesktop.org. This issue has been moved to:

https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/2098
@emersion
Copy link
Member Author

emersion commented Apr 11, 2020 •
edited

Oh, and each shell (xdg-shell, layer-shell) also has state structs with similar issues.

@emersion emersion mentioned this issue Jun 22, 2020
Open
@emersion
Copy link
Member Author

emersion commented Jul 2, 2020

Should we allow access to the previous state? Note that we don't want to retain locks on buffers longer than necessary.

Probably not, because users can easily cache any value if they need to.

emersion added a commit to emersion/wlroots that referenced this issue Jul 2, 2020
@emersion
output: introduce wlr_output_event_commit
00eb6f8 
This event contains a `committed` bitfield, which allows callers to know
which output fields changed during the commit.

This allows users to setup a single atomic commit listener, instead of
setting up one listener for each event (mode, scale, transform, and so
on).

References: swaywm#2098
@emersion emersion mentioned this issue Jul 2, 2020
Merged
@emersion
Copy link
Member Author

emersion commented Jul 2, 2020

Users should be able to figure out what changed in this commit (how?)

#2315 introduces a wlr_output_event_commit struct which contains a committed field.

emersion added a commit to emersion/wlroots that referenced this issue Aug 27, 2020
@emersion
output: introduce wlr_output_event_commit
7e2087f 
This event contains a `committed` bitfield, which allows callers to know
which output fields changed during the commit.

This allows users to setup a single atomic commit listener, instead of
setting up one listener for each event (mode, scale, transform, and so
on).

References: swaywm#2098
ddevault pushed a commit that referenced this issue Aug 27, 2020
@emersion @ddevault
output: introduce wlr_output_event_commit
c674241 
This event contains a `committed` bitfield, which allows callers to know
which output fields changed during the commit.

This allows users to setup a single atomic commit listener, instead of
setting up one listener for each event (mode, scale, transform, and so
on).

References: #2098
emersion added a commit to emersion/wlroots that referenced this issue Jan 10, 2021
@emersion
output: send commit event after pending state is cleared
a9ce75a 
References: swaywm#2098
@emersion emersion mentioned this issue Jan 10, 2021
Merged
emersion added a commit that referenced this issue Jan 10, 2021
@emersion
output-damage: use output commit event
edf5082 
Instead of relying on output.pending.committed, use
wlr_output_event_commit to find out whether a buffer was committed.
Eventually output.pending will be cleared before the commit event is
emitted.

References: #2098
@emersion emersion mentioned this issue Jan 10, 2021
Merged
emersion added a commit to emersion/wlroots that referenced this issue Jan 10, 2021
@emersion
output: send commit event after pending state is cleared
c1bae9c 
References: swaywm#2098
emersion added a commit to emersion/wlroots that referenced this issue Jan 10, 2021
@emersion
output-damage: stop relying on output pending state on commit
b8f1cf1 
References: swaywm#2098
emersion added a commit to emersion/wlroots that referenced this issue Jan 10, 2021
@emersion
output: send commit event after pending state is cleared
86f6103 
References: swaywm#2098
@emersion
Copy link
Member Author

wlr_output clears the pending state after commit is emitted, breaking (3). The pending state is used by users to figure out what changed (2). wlr_output provides no access to the previous state.

Addressed in #2630

emersion added a commit to emersion/wlroots that referenced this issue Jan 10, 2021
@emersion
output-damage: stop relying on output pending state on commit
beeeef2 
References: swaywm#2098
emersion added a commit to emersion/wlroots that referenced this issue Jan 10, 2021
@emersion
output: send commit event after pending state is cleared
256b70a 
References: swaywm#2098
BrassyPanache pushed a commit to BrassyPanache/wlroots that referenced this issue Jan 12, 2021
@emersion
output-damage: use output commit event
dacf86e 
Instead of relying on output.pending.committed, use
wlr_output_event_commit to find out whether a buffer was committed.
Eventually output.pending will be cleared before the commit event is
emitted.

References: swaywm#2098
emersion added a commit that referenced this issue Jan 16, 2021
@emersion
output-damage: stop relying on output pending state on commit
1fb9535 
References: #2098
emersion added a commit that referenced this issue Jan 16, 2021
@emersion
output: send commit event after pending state is cleared
b3e76d6 
References: #2098
@emersion
Copy link
Member Author

wlr_surface and wlr_output have a commit event, however things are more complicated for add-on protocols like xdg-shell, layer-shell and xdg-decoration. It'd be nice to be able to figure out what the newly committed state fields are in the wl_surface commit handler, to be able to have atomic logic that depends on the whole new state.

One solution would be to re-pupose the current.committed bitfield as the set of fields that changed since the last commit. Not sure it's a good idea.

Another solution would be to introduce some kind of "request_configure" event on the add-on objects, that runs before the wlr_surface commit event. This would allow compositor to stash the newly-committed state somewhere, and handle it on commit.

Add-ons like xdg-decoration would still need to update its state in the "precommit" phase. We'd need a new wlr_surface event for that.

@emersion emersion mentioned this issue Mar 19, 2021
Closed
@emersion emersion mentioned this issue Jul 7, 2021
Merged
@emersion emersion mentioned this issue Aug 11, 2021
Merged
@vyivel vyivel mentioned this issue Sep 24, 2021
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
output proposal surface
Milestone
No milestone
Development

No branches or pull requests
1 participant
@emersion