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

Add box shadows #64

Merged
merged 28 commits into from
Jan 18, 2023
Merged

Add box shadows #64

merged 28 commits into from
Jan 18, 2023

Conversation

WillPower3309
Copy link
Owner

@WillPower3309 WillPower3309 commented Nov 29, 2022

Adds box shadows in (probably?) the most efficient way we can following the method outlined in https://madebyevan.com/shaders/fast-rounded-rectangle-shadows/.

TODO:

  • Avoid rendering shadows behind surfaces (likely involves glStencil)
  • Add configuration options / commands for controlling shadows (color, blur_sigma)
  • Render shadows for all container layouts
  • Render shadows for floating containers

closes #6

@WillPower3309 WillPower3309 marked this pull request as draft November 29, 2022 05:37
@WillPower3309
Copy link
Owner Author

WillPower3309 commented Nov 29, 2022

Currently has an issue with the pos_attrib, will be fixed soon (TM). Right now only the non rounded corner shader is implemented. I think I may have a shader for the rounded case and for the non rounded case for maximum speed B)

Very much a draft, no config options added yet either.

@WillPower3309
Copy link
Owner Author

Currently held back by

00:00:01.159 [wlr] [GLES2] GL_INVALID_VALUE in glVertexAttribPointerARB(idx)
00:00:01.159 [wlr] [GLES2] GL_INVALID_VALUE in glEnableVertexAttribArray(index)
00:00:01.159 [wlr] [GLES2] GL_INVALID_VALUE in glDisableVertexAttribArray(index)

I'm looking into this but any additional eyes would be amazing. Once this is done we just need #58 done (which is just an hours worth of work) and we can release 0.2!

@ErikReider
Copy link
Collaborator

Currently held back by

00:00:01.159 [wlr] [GLES2] GL_INVALID_VALUE in glVertexAttribPointerARB(idx)
00:00:01.159 [wlr] [GLES2] GL_INVALID_VALUE in glEnableVertexAttribArray(index)
00:00:01.159 [wlr] [GLES2] GL_INVALID_VALUE in glDisableVertexAttribArray(index)

I'm looking into this but any additional eyes would be amazing. Once this is done we just need #58 done (which is just an hours worth of work) and we can release 0.2!

Found it. My eyes need a vacation after trying to spot this...

 include @sysconfdir@/sway/config.d/*
diff --git a/sway/desktop/fx_renderer.c b/sway/desktop/fx_renderer.c
index f12a705f..1dea93cc 100644
--- a/sway/desktop/fx_renderer.c
+++ b/sway/desktop/fx_renderer.c
@@ -238,7 +238,7 @@ struct fx_renderer *fx_renderer_create(struct wlr_egl *egl) {
 	}
 	renderer->shaders.box_shadow.proj = glGetUniformLocation(prog, "proj");
 	renderer->shaders.box_shadow.color = glGetUniformLocation(prog, "color");
-	renderer->shaders.box_shadow.pos_attrib = glGetUniformLocation(prog, "pos");
+	renderer->shaders.box_shadow.pos_attrib = glGetAttribLocation(prog, "pos");
 	renderer->shaders.box_shadow.position = glGetUniformLocation(prog, "position");
 	renderer->shaders.box_shadow.size = glGetUniformLocation(prog, "size");
 	renderer->shaders.box_shadow.blur_sigma = glGetUniformLocation(prog, "blur_sigma");

The shadow doesn't seem to work though :/

@WillPower3309
Copy link
Owner Author

Gah I'm so mad that the fix was that 💀 thanks man! I'll try to fix up the shader

@WillPower3309
Copy link
Owner Author

Alright, can confirm that the issue is in the fx_render_box_shadow function, likely in the shader. Heres the result of just calling fx_render_rect, which renders a rectangle over the window region as expected:

2022-12-11_15-12-1670790919

@WillPower3309
Copy link
Owner Author

LOL, can confirm shader seems off: https://www.shadertoy.com/view/DdlSD8

@ErikReider
Copy link
Collaborator

Tried messing with the shader a few nights ago but wasn't able to fix it... I was only able to get two edges working properly :/

@WillPower3309
Copy link
Owner Author

Alright I fixed it, no one is allowed to look at the commit change or they'll lose all faith in me as a maintainer 🥲

@ErikReider
Copy link
Collaborator

Looks pretty good!

This is after messing around with some margins and moving the render logic order:
image

@WillPower3309
Copy link
Owner Author

Awesome! Still need to do that myself but good to see it's working!

@WillPower3309
Copy link
Owner Author

WillPower3309 commented Dec 11, 2022

Looks pretty good!

This is after messing around with some margins and moving the render logic order:
image

I'll be having the damage tracking avoid the window region so the shadow isnt rendered where it won't be visible, so that should avoid the issue of needing to render it before the window so it appears behind (while giving us a lot of performance savings). I don't think many users want to see the shadow behind transparent regions either, as it appears unexpectedly dark with the shadow rendered as well, but I'll keep an eye on requests there

@ErikReider
Copy link
Collaborator

ErikReider commented Dec 11, 2022

I don't think many users want to see the shadow behind transparent regions either, as it appears unexpectedly dark with the shadow rendered as well, but I'll keep an eye on requests there.

I was thinking the same thing! A CSS box-shadow would do the same.

I can throw my diff here in case you find anything interesting :)

Note that this doesn't include any "spread" control :/

diff --git a/sway/desktop/render.c b/sway/desktop/render.c
index 0e1bb647..91c6e4a8 100644
--- a/sway/desktop/render.c
+++ b/sway/desktop/render.c
@@ -331,7 +331,8 @@ damage_finish:
 // _box.x and .y are expected to be layout-local
 // _box.width and .height are expected to be output-buffer-local
 void render_box_shadow(struct sway_output *output, pixman_region32_t *output_damage,
-		const struct wlr_box *_box, const float color[static 4]) {
+		const struct wlr_box *_box, const float color[static 4],
+		float blur_sigma) {
 	struct wlr_output *wlr_output = output->wlr_output;
 	struct fx_renderer *renderer = output->server->renderer;
 
@@ -354,7 +355,8 @@ void render_box_shadow(struct sway_output *output, pixman_region32_t *output_dam
 	pixman_box32_t *rects = pixman_region32_rectangles(&damage, &nrects);
 	for (int i = 0; i < nrects; ++i) {
 		scissor_output(wlr_output, &rects[i]);
-		fx_render_box_shadow(renderer, &box, color, wlr_output->transform_matrix, 0, 0.3);
+		fx_render_box_shadow(renderer, &box, color, wlr_output->transform_matrix,
+				0, blur_sigma);
 	}
 
 damage_finish:
@@ -977,6 +979,19 @@ static void render_containers_linear(struct sway_output *output,
 				marks_texture = child->marks_unfocused;
 			}
 
+			// render shadow
+			const float color[4] = {0, 0, 0, 0.5};
+			// Mimic how a CSS box-shadow looks
+			float blur_sigma = 20;
+			struct wlr_box box = {
+				state->x - blur_sigma,
+				state->y - blur_sigma,
+				state->width + blur_sigma * 2,
+				state->height + blur_sigma * 2
+			};
+			scale_box(&box, output->wlr_output->scale);
+			render_box_shadow(output, damage, &box, color, blur_sigma);
+
 			bool has_titlebar = state->border == B_NORMAL;
 			struct decoration_data deco_data = {
 				.alpha = child->alpha,
@@ -998,12 +1013,6 @@ static void render_containers_linear(struct sway_output *output,
 			} else if (state->border == B_PIXEL) {
 				render_top_border(output, damage, state, colors, deco_data.alpha, deco_data.corner_radius);
 			}
-
-			// render shadow
-			const float color[4] = {0,0,0,0.7};
-			struct wlr_box box = {state->x, state->y, state->width, state->height};
-			scale_box(&box, output->wlr_output->scale);
-			render_box_shadow(output, damage, &box, color);
 		} else {
 			render_container(output, damage, child,
 					parent->focused || child->current.focused);
diff --git a/sway/desktop/shaders/box_shadow.frag b/sway/desktop/shaders/box_shadow.frag
index 5dd50a25..7b5178d7 100644
--- a/sway/desktop/shaders/box_shadow.frag
+++ b/sway/desktop/shaders/box_shadow.frag
@@ -29,7 +29,10 @@ float random() {
 }
 
 void main() {
-    float frag_alpha = v_color.a * box_shadow(position, position + size, gl_FragCoord.xy, blur_sigma);
+    float frag_alpha = v_color.a * box_shadow(
+            position + blur_sigma,
+            position + size - blur_sigma,
+            gl_FragCoord.xy, blur_sigma * 0.5);
 
     // dither the alpha to break up color bands
     frag_alpha += (random() - 0.5) / 128.0;

@WillPower3309
Copy link
Owner Author

Thanks for sharing! Are the adjustments to the box arbitrary? Curious about why you set the sigma to be 0.5 it's original value in the shader

@ErikReider
Copy link
Collaborator

Are the adjustments to the box arbitrary?

Otherwise the shadow would be clipped to the window size :/

Curious about why you set the sigma to be 0.5 it's original value in the shader

Yeah me too... I was just testing different things and it seemed to work the best then for some reason????

@ErikReider
Copy link
Collaborator

Got the rounded shadows working. Should I push my changes?

@ErikReider
Copy link
Collaborator

Setting glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); fixes alpha with non-black shadows

@ErikReider
Copy link
Collaborator

I've also been experimenting with the damage that you mentioned earlier. I got a decent result but without the rounding of the corner... :/

image

@WillPower3309
Copy link
Owner Author

Nice work! Feel free to push the shadow changes if they implement the rounded shadow outlined in the blog post attached to the PR description

@WillPower3309
Copy link
Owner Author

Otherwise I'd love to see the damage tracking changes, and I can implement the rounded corners. Apologies for leaving this up without much progress this week, aiming to finish this soon so i can work on blur over the holiday downtime

@ErikReider
Copy link
Collaborator

Here's the small damage refactor:

diff --git a/sway/desktop/render.c b/sway/desktop/render.c
index 27567b58..55eb2076 100644
--- a/sway/desktop/render.c
+++ b/sway/desktop/render.c
@@ -345,6 +345,17 @@ void render_box_shadow(struct sway_output *output, pixman_region32_t *output_dam
 	pixman_region32_init(&damage);
 	pixman_region32_union_rect(&damage, &damage, box.x, box.y,
 		box.width, box.height);
+	
+	pixman_region32_t inner_damage;
+	pixman_region32_init(&inner_damage);
+	pixman_region32_union_rect(&inner_damage, &inner_damage,
+			box.x + blur_sigma,
+			box.y + blur_sigma,
+			box.width - blur_sigma * 2.0,
+			box.height - blur_sigma * 2.0);
+	pixman_region32_subtract(&damage, &damage, &inner_damage);
+	pixman_region32_fini(&inner_damage);
+
 	pixman_region32_intersect(&damage, &damage, output_damage);
 	bool damaged = pixman_region32_not_empty(&damage);
 	if (!damaged) {

There are still some damage issues when moving a floating window with shadows though. Not really sure why... :/

@fuegoio
Copy link
Contributor

fuegoio commented Dec 18, 2022

I've also been experimenting with the damage that you mentioned earlier. I got a decent result but without the rounding of the corner... :/

image

I pulled this code to test locally and try to improve the damage tracking and ran into this issue as well.
Especially, without your last comment, the shadow was rendering on top of windows. It can be fixed by inverting the order:

diff --git a/sway/desktop/render.c b/sway/desktop/render.c
index 27567b58..a9d15018 100644
--- a/sway/desktop/render.c
+++ b/sway/desktop/render.c
@@ -992,14 +992,6 @@ static void render_containers_linear(struct sway_output *output,
                                .saturation = child->saturation,
                                .has_titlebar = has_titlebar,
                        };
-                       render_view(output, damage, child, colors, deco_data);
-                       if (has_titlebar) {
-                               render_titlebar(output, damage, child, floor(state->x), floor(state->y),
-                                               state->width, colors, deco_data.alpha, deco_data.corner_radius,
-                                               title_texture, marks_texture);
-                       } else if (state->border == B_PIXEL) {
-                               render_top_border(output, damage, state, colors, deco_data.alpha, deco_data.corner_radius);
-                       }

                        // render shadow
                        const float color[4] = {0.0, 0.0, 0.0, 0.5};
@@ -1013,6 +1005,18 @@ static void render_containers_linear(struct sway_output *output,
                        };
                        scale_box(&box, output->wlr_output->scale);
                        render_box_shadow(output, damage, &box, color, blur_sigma, deco_data.corner_radius);
+
+                       render_view(output, damage, child, colors, deco_data);
+
+                       if (has_titlebar) {
+                               render_titlebar(output, damage, child, floor(state->x), floor(state->y),
+                                               state->width, colors, deco_data.alpha, deco_data.corner_radius,
+                                               title_texture, marks_texture);
+                       } else if (state->border == B_PIXEL) {
+                               render_top_border(output, damage, state, colors, deco_data.alpha, deco_data.corner_radius);
+                       }
                } else {
                        render_container(output, damage, child,
                                        parent->focused || child->current.focused);

I also fixed the squared corner that you have that appears because of the damaging that doesn't take into account the corner radius:

diff --git a/sway/desktop/render.c b/sway/desktop/render.c
index 27567b58..624eb2d2 100644
--- a/sway/desktop/render.c
+++ b/sway/desktop/render.c
@@ -345,6 +345,17 @@ void render_box_shadow(struct sway_output *output, pixman_region32_t *output_dam
        pixman_region32_init(&damage);
        pixman_region32_union_rect(&damage, &damage, box.x, box.y,
                box.width, box.height);
+
+       pixman_region32_t inner_damage;
+       pixman_region32_init(&inner_damage);
+       pixman_region32_union_rect(&inner_damage, &inner_damage,
+                       box.x + blur_sigma + corner_radius,
+                       box.y + blur_sigma + corner_radius,
+                       box.width - (blur_sigma + corner_radius) * 2.0,
+                       box.height - (blur_sigma + corner_radius) * 2.0);
+       pixman_region32_subtract(&damage, &damage, &inner_damage);
+       pixman_region32_fini(&inner_damage);
+
        pixman_region32_intersect(&damage, &damage, output_damage);
        bool damaged = pixman_region32_not_empty(&damage);
        if (!damaged) {

I don't know if it is the best solution (I think the damaging is less precise :/ ?), but it works!

@fuegoio
Copy link
Contributor

fuegoio commented Dec 18, 2022

I also have an issue with the glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); flag, which is on my machine creating a glitch with rounded corners, that I don't have without this flag:

image140
image322

But I don't know why ahah, I tried playing with the rounded corner shader but with no luck.

@ErikReider
Copy link
Collaborator

ErikReider commented Dec 19, 2022

the shadow was rendering on top of windows. It can be fixed by inverting the order:

With a "fixed" damage tracker this wouldn't be necessary. It would probably also make it easier to exclude the shadow from any future blurring done to the window :)

I also fixed the squared corner that you have that appears because of the damaging that doesn't take into account the corner radius:

This was also one of my solutions until I looked at a transparent window...

image

I also have an issue with the glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); flag, which is on my machine creating a glitch with rounded corners, that I don't have without this flag:

Strange... This shouldn't be an issue when a proper damage tracker is added. Without the blending, any other shadow color than black would look like this:

image

@WillPower3309
Copy link
Owner Author

I suspect that in order to account for the rounded corner case, we will need to damage track similar to how @fuegoio demonstrated, however avoid rendering in the area the window encompasses with a gl stencil

A stencil of the surface will need to be done for blur and work in improving rounded borders in the future too

@WillPower3309
Copy link
Owner Author

Are the adjustments to the box arbitrary?

Otherwise the shadow would be clipped to the window size :/

Curious about why you set the sigma to be 0.5 it's original value in the shader

Yeah me too... I was just testing different things and it seemed to work the best then for some reason????

Interesting find that I had, it seems as though the radius equates to roughly 2 * the blur sigma, so this makes sense

@WillPower3309
Copy link
Owner Author

Instead of cutting the sigma in half, and adjusting the box with the original sigma, let's keep the default sigma and adjust the box with 2 * the sigma. This would better match what should be expected of the sigma

@WillPower3309
Copy link
Owner Author

After this, we just need to implement the gl stencil (which I'm looking into), add the config options / commands (which would be a great first task for anyone looking to be a new contributor), and then apply the box shadows on all container layouts (right now it's just for linear laid out containers)

@WillPower3309
Copy link
Owner Author

I also have an issue with the glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); flag, which is on my machine creating a glitch with rounded corners, that I don't have without this flag:

image140 image322

But I don't know why ahah, I tried playing with the rounded corner shader but with no luck.

in regards to this, I suspect we could set the blend function back to glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA); after shadows are done rendering to fix, but I can't reproduce your issue yet

@ErikReider
Copy link
Collaborator

ErikReider commented Jan 9, 2023

Did your ignore windows with CSD commit help at all? I can reproduce the issue, and if the CSD commit aimed to fix it we would revert that if it didn't work

It fixed another issue where shadows were being drawn twice (one by gtk and one by swayfx). An example would be the GTK file picker in Firefox:

swappy-20230109_122343

@ErikReider
Copy link
Collaborator

Have you tried removing the view color buffer rendering? Makes it a lot more noticeable. It only seems to show when dragging a window over a location where the stencil mask resides (inside of the view).

@WillPower3309
Copy link
Owner Author

WillPower3309 commented Jan 9, 2023

Have you tried removing the view color buffer rendering? Makes it a lot more noticeable. It only seems to show when dragging a window over a location where the stencil mask resides (inside of the view).

Not yet, the other thing I've observed is that this seems to specifically impact popup windows with CSD (where the stencil is a bit off and too large since it thinks the decorations are part of the window). Not sure if there are cases outside of CSD decorated floating popup windows though

@head-gardener
Copy link
Contributor

head-gardener commented Jan 9, 2023

Not sure if this is directly related, but telegram-desktop has similar issue with shadows being bigger than the window itself whenever applications draws its own window frame with rounded corners.
image

@ErikReider
Copy link
Collaborator

@WillPower3309 Reverting back to drawing a rounded box as stencil mask fixes the floating bug and some CSD issues like the @head-gardener's comment above but not the GTK file dialog.

Seems like Hyprland doesn't have the file picker CSD issue, so I'll have to investigate further tomorrow.

I'll push the revert :)

@WillPower3309
Copy link
Owner Author

@WillPower3309 Reverting back to drawing a rounded box as stencil mask fixes the floating bug and some CSD issues like the @head-gardener's comment above but not the GTK file dialog.

Seems like Hyprland doesn't have the file picker CSD issue, so I'll have to investigate further tomorrow.

I'll push the revert :)

Awesome, good stuff. I'll focus on fixing the titlebar sizing in the meantime (#90)

@shaunsingh
Copy link

For me only part of the screen gets shadows, sorry if its hard to notice on the background

20230113_21h43m55s_grim

The top renders just fine. You can see a diagonal where the shadow stops rendering

@ErikReider
Copy link
Collaborator

ErikReider commented Jan 14, 2023

For me only part of the screen gets shadows, sorry if its hard to notice on the background

The top renders just fine. You can see a diagonal where the shadow stops rendering

Does it disappear if you open another terminal? Any way of reproducing this?

@shaunsingh
Copy link

Happens with all applications all the time. I am running asahi linux with the experimental gpu driver, but that has full opengl/es 2 support so it should be fine

let me know if there's any kind of logs you'd like me to share

@WillPower3309
Copy link
Owner Author

Happens with all applications all the time. I am running asahi linux with the experimental gpu driver, but that has full opengl/es 2 support so it should be fine

let me know if there's any kind of logs you'd like me to share

Good to know! Thanks for letting us know, I am tempted to believe it's an asahi problem. I'm going to try to get this merged today pending some more testing on my end. If it still exhibits the problem when merged please raise an issue with some logs. This will allow us to get some more visibility on if other people experience the issue prior to making a release with this feature.

@shaunsingh
Copy link

Will do! It can very well just be an issue with the asahi driver. I'll find another machine to test my config on just in case

@ErikReider
Copy link
Collaborator

What we could do regarding the CSD concern is to hide it behind a config option. Something like shadows_on_csd?

Copy link
Collaborator

@ErikReider ErikReider left a comment

Choose a reason for hiding this comment

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

LGTM! We can always add the csd option later :)

@WillPower3309
Copy link
Owner Author

LGTM! We can always add the csd option later :)

My thoughts exactly. May do some minor refactoring then I'll merge

@WillPower3309
Copy link
Owner Author

Moved the shadow rendering back to render_view, as it already checks for CSD borders

@WillPower3309
Copy link
Owner Author

@ErikReider looks good on my end but I'll let you try it out again just in case anything changed in my small refactor. I may rename the config options prior to a merge tomorrow.

Interestingly the bad stencilling from #82 only appears on tabbed / stacked layouts for me now, not on regularly tiled windows with a titlebar. Strange, as logically that wouldn't make a difference.

@ErikReider
Copy link
Collaborator

Strange... I'll take a look at it later today :)

@ErikReider
Copy link
Collaborator

I'll let you merge this in case you want to change the config names

@WillPower3309
Copy link
Owner Author

There are a few changes I may make prior to a release, namely different config names and an adjustment to damage tracking. However, I think that this can be merged as is so that it can be more widely tested. Good work everyone!

@WillPower3309 WillPower3309 merged commit 588ea8e into master Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add drop shadows
5 participants