Fix RoundedCornersEffect #2655
Fix RoundedCornersEffect #2655
Conversation
lib/RoundedCornersEffect.vala
Outdated
|
|
||
| private void update_clip_radius () requires (actor != null) { | ||
| var resource_scale = actor.get_resource_scale (); | ||
| set_uniform_value ("clip_radius", Utils.scale_to_int (clip_radius, monitor_scale / resource_scale)); |
There was a problem hiding this comment.
Why do we divide by the resource scale here?
There was a problem hiding this comment.
I have no idea why but without it the corner radius scaling is incorrect ¯\_(ツ)_/¯
There was a problem hiding this comment.
In what cases (scale-monitor-framebuffer or not, 1.0, 2.0, etc.)? And how and where is it incorrect? I'm asking because resource scale is only really meant for the resolution of resources and not to alter dimensions of any kind.
There was a problem hiding this comment.
Because we use Clutter.ShaderEffect which is a subclass of Clutter.OffscreenEffect which scales the texture size based on the actor resource scale. Also mutter code explicitly states:
/**
* clutter_actor_get_resource_scale:
* @self: A #ClutterActor
*
* Retrieves the resource scale for this actor.
*
* The resource scale refers to the scale the actor should use for its resources.
* For example if an actor draws a a picture of size 100 x 100 in the stage
* coordinate space, it should use a texture of twice the size (i.e. 200 x 200)
* if the resource scale is 2.
Which is our case since Clutter.OffscreenEffect draws a texture so it must be scaled based on the resource scale
There was a problem hiding this comment.
In what cases (scale-monitor-framebuffer or not, 1.0, 2.0, etc.)? And how and where is it incorrect?
scale-monitor-framebuffer + 2x scaling
Expected corner radius is 6 but real corner radius is 12
There was a problem hiding this comment.
With #2664, and I checked that removing monitor_scale from Utils.scale_to_int (clip_radius, monitor_scale ...) produces incorrect corner radius too
There was a problem hiding this comment.
Huh that's weird because I got this (which looks pretty right to me) with scale monitor framebuffer, 2x scale on this branch but with all of the get_resource_scale usage removed both in clip radius and actor size.
whereas with the exact same settings on this branch but without any modifications i.e. with dividing by resource scale i get this
There was a problem hiding this comment.
I haven't tried removing scaling from actor size but it appears to look better than with it. So you were right here. Thanks for looking into this!
leolost2605
left a comment
There was a problem hiding this comment.
The corners look a lot better already but I'm still noticing another thing. On main with scale of 1.5 it looks like this
which has a corner radius of 5 pixels (if you ignore the semi transparent additional pixel btw is this intended?) which make sense being 3 * 1.5 rounded up.
With this branch however it has a corner radius of 4? Is that intended? It looks a lot less round in practice
Btw I don't know much about corner radii and where you start counting them etc. so correct me if I'm wrong somewhere I just noticed the difference here :)
Also is there a specific issue this branch fixes so that I can check that?
Third time's a charm
Rename .vert shaders to .frag because well, these are fragment shaders, not vertex shaders.
Fixes the strange offsets in shader code by correctly calculating offsets using clutter_actor_box_enlarge_for_effects which is mutter private api. This works with and without fractional scaling which was the flaw of my previous attempts