[do not merge] Surface state refactor

[vyivel]

Also see individual commits.

State squashing

(The term squashing is taken from Git, but maybe another word would fit here better.)

"Squashing" refers to an action of combining a state with the state right before it in the queue. The older state becomes a "sum" of two states, and the newer state becomes empty.

Changes

• wlr_surface.cached is replaced with wlr_surface.states which also includes wlr_surface.current and wlr_surface.pending

It makes squashing easier, removing potential corner cases with wlr_surface.{current,pending}.

Data structure

The following data structure is maintained:

When a state is cached, a column of cachedn+1 states is added right before the last one (pending).
When a surface extension is added, a row of extensionk+1 states is added. Note that rows have undefined order; wlr_surface_state::extensions must be treated as an unordered set.

[emersion]

The design overall looks good to me!

As mentioned on IRC maybe it's not worth it to have two separate interfaces, maybe both can be merged. Maybe it's possible to re-use wlr_addon instead of re-implementing it, but maybe it's not worth the hassle.

Using wlr_client_buffer has an (unintentional?) interesting side-effect: upon creation, the original buffer is unlocked, which results in using 0 client wl_buffers (i.e. there is no busy wl_buffer). This PR nihilates this optimization, as buffers, being absolute state, can be present in multiple wlr_surface_states.

While this is correct from a protocol conformance point-of-view, this isn't great because this forces clients to re-render. For instance foot has optimizations for this particular case.

Maybe the client's wl_buffer can be "relative state"?

wlr_client_buffer shouldn't be part of the state and should stay as a mere helper to upload/import the buffer to the renderer.

This PR unintentionally fixes all Firefox issues with gfx.webrender.compositor I had before.

Excellent!

wlr_surface_state.committed is removed

Hm, why is that? It's pretty handy to be able to figure out what changed in a particular commit. For instance, if the sub-surface order hasn't changed, no need to re-arrange the scene-graph on parent commit.

Resizing windows with subsurfaces still causes flickering, probably due to surface_damage_subsurfaces() doing something wrong (should it even exist?).

Yeah, there is a bit too much magic going on in the functions computing damage. I'm not sure this should stay as-is, especially around the sub-surface logic.

[vyivel]

Maybe it's possible to re-use wlr_addon instead of re-implementing it.

Making wlr_surface_extension_state an addon wouldn't make much sense as we need to a) store the order in wlr_surface_extension::states and b) iterate over wlr_surface_state::extensions to squash every extension state. Iterating over wlr_surface::extensions is required to initialize newly created cached state properly. wlr_addon_set isn't really fit to be iterated over.

this isn't great because this forces clients to re-render

Hmm, how so? I don't really see why would that change make clients re-render.

Maybe the client's wl_buffer can be "relative state"?

Well, a buffer isn't a difference between two objects, so it's not relative. Absolute/relative state could be renamed to surface-local/commit-local state respectively though, and wlr_buffer would simply be moved from one state to another then (and not copied).

wlr_client_buffer shouldn't be part of the state and should stay as a mere helper to upload/import the buffer to the renderer.

Yes, the only wlr_client_buffer is in wlr_surface.

For instance, if the sub-surface order hasn't changed, no need to re-arrange the scene-graph on parent commit.

Fair point. I originally thought that change would make state squashing simpler, but actually it doesn't really add much complexity so I guess it could be left.

[vyivel]

Merged extension interfaces, returned wlr_surface_state.committed back, updated the PR description to match the changes.

[emersion]

This needs a rebase, now that the first part of this PR has been merged in #3169.

this isn't great because this forces clients to re-render

Hmm, how so? I don't really see why would that change make clients re-render.

See the discussion in #2705 (comment), and the comment a bit below with a performance comparison table.

"Immediate release" means the compositor uploads the shm buffer to the GPU on commit and then releases the buffer.

Well, a buffer isn't a difference between two objects, so it's not relative. Absolute/relative state could be renamed to surface-local/commit-local state respectively though, and wlr_buffer would simply be moved from one state to another then (and not copied).

Right, I think that would make sense. We just need a way to make the buffer non-sticky.

[vyivel]

Closed by accident.
Will be rebased soon™.

As for buffer thing, none of that absolute/relative state matters anymore now that wlr_surface_state.committed is back.

[vyivel]

Rebased on top of master, removed outdated info from the description.

[vyivel]

Rebased against #3191.

[vyivel]

Closing as an outdated part of #3151.