Conversation
4e6325e to
3bf8724
Compare
|
Still need to look at wlr_scene.c |
|
Converting to draft as there still are a few damage issues :/ |
|
I've been using this for a few months now on multiple machines with the SwayFX PR, and no crashes. Marking as ready :) |
Don't render the artifact region of the shadow, only the buffer size + blur radius
Allows for multiple blur sizes to be compensated for unlike the previous one which broke when there were different blur sizes. Would allow for per-node blur_data
Helps when the shadow is far from the reference buffer where the original buffer damage wouldn't reach the shadow node
f83d744 to
3ee8b33
Compare
| #include <stdbool.h> | ||
| #include <wlr/util/addon.h> | ||
|
|
||
| #define drop_shadow_calc_size(blur_sigma) (ceil(1.5f * blur_sigma) * 2) |
There was a problem hiding this comment.
Q: does an equal blur radius value result in an equal spread in drop shadow and box shadow?
There was a problem hiding this comment.
Hmm, ideally they'd look the same but I haven't dived deep enough to know if this is a trivial problem to solve or not haha
There was a problem hiding this comment.
Unfortunately the single pixel transparent area between the shadow and texture would need texture shader side changes (so we can keep that out of scope)
There was a problem hiding this comment.
Hmm, ideally they'd look the same
Yeah, I noticed that the drop shadow spread is a bit further than the box one, so I'll be working on fixing that after the university finals season has passed
include/scenefx/types/wlr_scene.h
Outdated
| enum wlr_scene_shadow_type { | ||
| WLR_SCENE_SHADOW_TYPE_DROP, | ||
| WLR_SCENE_SHADOW_TYPE_BOX, | ||
| }; |
There was a problem hiding this comment.
do we need these? We should be able to tell if buffer_link is NULL or not
include/scenefx/types/wlr_scene.h
Outdated
| * shadow-type. A drop-shadow returns a larger integer compared to the | ||
| * box-shadow, so this can be used to dynamically adjust the size and position |
include/scenefx/types/wlr_scene.h
Outdated
| * NOTE: Please use the `wlr_scene_shadow_get_offset` function to get a dynamic | ||
| * offset depending on the type. | ||
| */ | ||
| void wlr_scene_shadow_set_type(struct wlr_scene_shadow *shadow, enum wlr_scene_shadow_type type); |
There was a problem hiding this comment.
I think we can remove this call for now, if we're aligned on my earlier comment
| @@ -0,0 +1,27 @@ | |||
| #ifndef TYPES_LINKED_NODES_H | |||
There was a problem hiding this comment.
nit: this file should be linked_node.h
| uniform float blur_sigma; | ||
| uniform vec4 color; | ||
| uniform bool is_horizontal; |
There was a problem hiding this comment.
size, blur_sigma and is_horizontal are unused
| uniform vec2 size; | ||
| uniform float blur_sigma; | ||
| uniform vec4 color; | ||
| uniform bool is_horizontal; |
There was a problem hiding this comment.
size, blur_sigma, is_horizontal unused
types/scene/wlr_scene.c
Outdated
| int wlr_scene_shadow_get_offset(struct wlr_scene_shadow *shadow) { | ||
| switch (shadow->type) { | ||
| case WLR_SCENE_SHADOW_TYPE_DROP: | ||
| return drop_shadow_calc_size(shadow->blur_sigma); | ||
| default: | ||
| return shadow->blur_sigma; | ||
| } | ||
| } |
There was a problem hiding this comment.
have you tested that the shadows are functionally identical between box and drop? We'd want the same appearance minus the transparent areas between types. Also, why do you refer to the sigma as offset for the drop shadow?
There was a problem hiding this comment.
Realize I already left this comment earlier in blur_data.h
types/scene/wlr_scene.c
Outdated
| struct linked_node *shadow_link = linked_nodes_get_sibling(&scene_buffer->drop_shadow_link); | ||
| if (shadow_link != NULL) { | ||
| struct wlr_scene_shadow *scene_shadow = wl_container_of(shadow_link, scene_shadow, buffer_link); | ||
| if (scene_shadow && SCENE_SHADOW_IS_DROP(scene_shadow)) { |
There was a problem hiding this comment.
do we need this check? We already have an assertion on the link. Maybe we should add another assertion here instead
types/scene/wlr_scene.c
Outdated
|
|
||
| const int shadow_size = drop_shadow_calc_size(scene_shadow->blur_sigma) * data->scale; | ||
|
|
||
| // Optimization: Fallback to a regular box-shadow if the texture is fully opaque |
|
Late here, but do you think it's worth splitting the shadow node into separate box shadow and drop shadow nodes? Just thinking that there's not really any use case where a comp would want to swap between them on the same node, and it could help split up the impl. Open to your thoughts as well! |
910bb1c to
bc55ef5
Compare
|
Converting to draft as I think that I can optimize the shader a bit more like the blur shader (maybe even use that shader instead?). I would also like to land #129 to test these changes :) |


Allows for a dynamic shadow that isn't a fixed shape, like our shadow_node. This allows for shadows that "exclude" transparent regions like weirdly shaped surfaces and text buffers.
So according to the Mozilla docs, the drop-shadow works by applying a blur to the base textures alpha mask. So this should mimic the implementation that's used by web browsers. So a 10px radii in the browser should result in the same looking blur here.
Swaync with the smart shadows enabled:

TODO:
Use kawase blur instead?(Use a two-pass Gaussian blur, like the web browsers use)scene_bufferinstead?Add post-processing or use the regularpass_add_texturefunction instead of a dedicated shader (or just blit for gles3?)