Skip to content
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

[WLR Scene] Add blur #33

Open
wants to merge 38 commits into
base: main
Choose a base branch
from
Open

[WLR Scene] Add blur #33

wants to merge 38 commits into from

Conversation

ErikReider
Copy link
Member

@ErikReider ErikReider commented Feb 26, 2024

Depends on #30

Also depends on #66 for the initial shadow_node impl which the blur_node should base off of. Could potentially also add a custom_node impl for both of those effect nodes?

Todo:

  • Figure out the best way for the compositor to know when to rerender the optimized blur
  • tinywl example for optimized blur
  • Make a seperate blur_node instead of forcing it onto a scene_buffer (Will be difficult to separate the discard transparent logic, so we should wait with this)

Tesing:

  • Run tinywl
  • Or run fx-comp (instructions in the README). NOTE: the default keyboard modifier is left alt

@ErikReider ErikReider added the enhancement New feature or request label Feb 26, 2024
@ErikReider ErikReider self-assigned this Feb 26, 2024
types/scene/wlr_scene.c Outdated Show resolved Hide resolved
@ErikReider ErikReider force-pushed the wlr_scene_blur branch 2 times, most recently from f8a24c0 to 39756c6 Compare April 15, 2024 22:32
@ErikReider ErikReider marked this pull request as ready for review May 7, 2024 10:17
@ErikReider
Copy link
Member Author

An optimized blur example in tinywl wouldn't really make it that tiny and simple anymore, so skipping that :)

@WillPower3309
Copy link
Member

An optimized blur example in tinywl wouldn't really make it that tiny and simple anymore, so skipping that :)

Understandable, but do you mind running me through what's required on the comp side for getting this to work in scene? I'm following it along but a summary may be helpful

My thinking here is that one of the main goals of scene is for compositors to easily leverage our effects in very few LOC

@ErikReider
Copy link
Member Author

do you mind running me through what's required on the comp side for getting this to work in scene?

So the optimized blur is a new scene node that should be inserted between regular scene trees, which would be between bottom and workspace in sways use-case. The comp is responsible for resizing the node to the desired size, which in most use-cases would be the monitor's size and coordinates.

The comp is also responsible for redrawing the optimized blur by marking it as “dirty”. Sway currently redraws the blur when any background or bottom layer surface commits.

So actually implementing it in a fully fledged comp doesn't require that many LOC, but adding an example in TinyWL would require adding extra non-blur related code (the extra layers and a good way of showing off the optimized blur)

@ErikReider
Copy link
Member Author

Optimized blur (x-ray) example in tinywl is now added. It's very simple but it works good enough to demonstrate what it does and how it works :)

image

Notice how only the white rectangle gets blurred 😉

@ErikReider
Copy link
Member Author

Might want to merge this after 0.2? Would give us some more time to think of a better impl, especially if we get a #60 impl merged

@ErikReider ErikReider marked this pull request as draft September 18, 2024 10:40
@WillPower3309
Copy link
Member

Might want to merge this after 0.2? Would give us some more time to think of a better impl, especially if we get a #60 impl merged

Sounds good to me!

types/fx/blur_data.c Outdated Show resolved Hide resolved
include/scenefx/types/wlr_scene.h Show resolved Hide resolved
include/scenefx/types/wlr_scene.h Show resolved Hide resolved
Comment on lines 569 to 578
/**
* Tells the renderer to re-render the optimized blur for the specified output.
* A NULL output will re-render the optimized blur on every output.
* This is very expensive so should only be called when needed.
*
* An example use would be to call this when a "static" node changes, like a
* wallpaper.
*/
void wlr_scene_optimized_blur_mark_dirty(struct wlr_scene *scene,
struct wlr_output *output);
Copy link
Member

Choose a reason for hiding this comment

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

It would be great if we could do this dynamically in scene and ditch this function. Considering that scene optimizes the rendering by ignoring regions below opaque regions of the nodes above, I'd bet we could flip the script on that damage tracking optimization to do this dynamically when nodes below are updated

Copy link
Member Author

Choose a reason for hiding this comment

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

What SwayFX does is that the optimized blur texture only gets updated when a bottom or background layer signals a commit (updates). I'm pretty sure that it would be impossible for us to check that while also giving the comp choices on how to render this (scene doesn't know where or even if layer surfaces exist).

Copy link
Member

@WillPower3309 WillPower3309 Nov 9, 2024

Choose a reason for hiding this comment

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

Hmm - but couldn't we traverse the scene tree to find the surfaces below? whether or not it's a layer surface wouldn't be relevant. Unless the impl would have separate trees for each layer that aren't linked at all - not sure if we have one root tree that has sub trees per layer or if they exist independently

Copy link
Member Author

Choose a reason for hiding this comment

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

Wouldn't iterating through the tree an extra time bring some possible performance issues?

types/scene/wlr_scene.c Show resolved Hide resolved
types/scene/wlr_scene.c Show resolved Hide resolved
types/scene/wlr_scene.c Show resolved Hide resolved
types/scene/wlr_scene.c Outdated Show resolved Hide resolved
@@ -152,6 +156,12 @@ struct wlr_scene_shadow {
float blur_sigma;
};

/** A scene-graph node telling SceneFX to render the optimized blur */
Copy link
Member Author

Choose a reason for hiding this comment

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

We should maybe write that it doesn't actually render anything onto the tree

Comment on lines 569 to 578
/**
* Tells the renderer to re-render the optimized blur for the specified output.
* A NULL output will re-render the optimized blur on every output.
* This is very expensive so should only be called when needed.
*
* An example use would be to call this when a "static" node changes, like a
* wallpaper.
*/
void wlr_scene_optimized_blur_mark_dirty(struct wlr_scene *scene,
struct wlr_output *output);
Copy link
Member Author

Choose a reason for hiding this comment

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

What SwayFX does is that the optimized blur texture only gets updated when a bottom or background layer signals a commit (updates). I'm pretty sure that it would be impossible for us to check that while also giving the comp choices on how to render this (scene doesn't know where or even if layer surfaces exist).

types/scene/wlr_scene.c Show resolved Hide resolved
types/scene/wlr_scene.c Outdated Show resolved Hide resolved
types/scene/wlr_scene.c Show resolved Hide resolved
types/scene/wlr_scene.c Show resolved Hide resolved
@WillPower3309
Copy link
Member

Optimized blur (x-ray) example in tinywl is now added. It's very simple but it works good enough to demonstrate what it does and how it works :)

image

Notice how only the white rectangle gets blurred 😉

both rectangles now seem part of the blurring, other windows are not though so just seems like a layer order thing!

@ErikReider
Copy link
Member Author

both rectangles now seem part of the blurring, other windows are not though so just seems like a layer order thing!

Fixed by 698373b :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants