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

scene: add support for damage tracking #3117

Merged
merged 6 commits into from Sep 8, 2021

Conversation

emersion
Copy link
Member

@emersion emersion commented Aug 17, 2021

Introduce wlr_scene_output to have per-output state in the scene-graph.

I've decided to not make wlr_scene_output a wlr_scene_node to make a clear cut between elements that are rendered on screen and viewports in the scene.

Depends on: #1966
Cage patch: cage-kiosk/cage#203

types/wlr_scene.c Outdated Show resolved Hide resolved
types/wlr_scene.c Outdated Show resolved Hide resolved
@emersion
Copy link
Member Author

This is ready for review.

Copy link
Member

@ifreund ifreund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if any compositor that doesn't already use wlr_output_layout will use the scene graph API. It seems like some integration there could save almost every compositor some boilerplate.

types/wlr_scene.c Outdated Show resolved Hide resolved
types/wlr_scene.c Outdated Show resolved Hide resolved
@emersion emersion force-pushed the scenegraph-output branch 2 times, most recently from 7304a26 to 790f3c5 Compare August 30, 2021 19:00
@emersion
Copy link
Member Author

I wonder if any compositor that doesn't already use wlr_output_layout will use the scene graph API. It seems like some integration there could save almost every compositor some boilerplate.

Yes, likely, but I'd prefer to keep this as a separate patch (feel free to submit a PR built on top of this one!). One still unanswered question is how we should plug existing wlroots modules to the scene-graph. Should the scene-graph add support for wlr_output_layout, or should wlr_output_layout add optional support for the scene-graph? In other words, should we have wlr_scene_create_with_output_layout(output_layout) or wlr_output_layout_create_with_scene(scene)? (API subject to be tweaked a bit to allow better extensibility.)

@ammen99
Copy link
Member

ammen99 commented Aug 30, 2021

wlr_output_layout_create_with_scene(scene)

Would this support NULL scene? In Wayfire, I'd like to use wlr_output_layout but not scene for obvious reasons ..

@emersion
Copy link
Member Author

Yes, of course. output-layout and the scene-graph will both stay optional and compositors will still be able to use one without the other.

@emersion
Copy link
Member Author

emersion commented Sep 2, 2021

Rebased on top of @djpohly's #3140.

@djpohly
Copy link
Contributor

djpohly commented Sep 6, 2021

Something is off with wlr_scene_output positioning. Here's the scene-graph example running Alacritty:
2021-09-06-000243_958x519_scrot
I added the line

wlr_scene_output_set_position(output->scene_output, 30, 10);

and - if I had to guess - it looks like the damage rectangles are correctly translated with respect to the viewport, but the rendered surfaces are missing a pair of minus signs somewhere.:
2021-09-06-000105_958x519_scrot

@emersion emersion force-pushed the scenegraph-output branch 2 times, most recently from e34c13a to 7f64d90 Compare September 6, 2021 07:26
@emersion
Copy link
Member Author

emersion commented Sep 6, 2021

Something is off with wlr_scene_output positioning

Indeed, must've missed something in a refactoring. Fixed in "scene: fix wlr_scene_render_output offset".

@djpohly
Copy link
Contributor

djpohly commented Sep 7, 2021

Here is a MWE for another issue I'm seeing. Run on DRM backend, move the (invisible) mouse cursor to generate redraws, notice the flickering rectangle, and click to exit.

I'm still not very confident in my understanding of double-buffering and damage, but it looks like... maybe existing bits of scene aren't ending up being rendered to one of the buffers when a new output is created? I'm not actually sure whether the problem is specific to the scene-graph code.

When providing non-zero layout-local coordinates to
wlr_scene_render_output, the viewport should be translated by the
given values. However the viewport was translated by the opposite
values: when giving 42,42 the viewport's position would be set to
-42,-42.
These allow describing an output's viewport inside the scene-graph.
This allows us to get damage tracking for free™.
@emersion
Copy link
Member Author

emersion commented Sep 7, 2021

Indeed, what you're seeing is a wlr_output_damage issue, fixed here: #3175

@djpohly
Copy link
Contributor

djpohly commented Sep 7, 2021

Not sure whether this belongs here or there, but with #3175 applied, I still get the behavior when the scene_output is created after the initial output_commit (MWE).

@emersion
Copy link
Member Author

emersion commented Sep 7, 2021

I've updated this branch to call wlr_output_damage_add_whole in wlr_scene_output_create to make sure this doesn't happen. Have you fetched the latest force-push from this branch before applying #3175?

@djpohly
Copy link
Contributor

djpohly commented Sep 7, 2021

It's possible that I missed that push. I'll give it a full recheck and report back.

@djpohly
Copy link
Contributor

djpohly commented Sep 7, 2021

Guess this one was PEBKAC - sorry about that! (I'm still trying to track down another issue with what looks like the wrong regions being damaged on my second monitor, but I don't have anything too useful yet.)

@bl4ckb0ne
Copy link
Contributor

LGTM!

@bl4ckb0ne bl4ckb0ne merged commit e05c884 into swaywm:master Sep 8, 2021
@emersion emersion deleted the scenegraph-output branch September 8, 2021 13:51
emersion added a commit to emersion/cage that referenced this pull request Sep 9, 2021
emersion added a commit to emersion/cage that referenced this pull request Sep 22, 2021
emersion added a commit to emersion/cage that referenced this pull request Sep 28, 2021
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.

None yet

6 participants