-
Notifications
You must be signed in to change notification settings - Fork 720
Fix gradient clipping with border radius in software renderer #9098
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
base: master
Are you sure you want to change the base?
Conversation
Resolves issue where gradients were not properly clipped along rounded rectangle boundaries in the software renderer. Fixes: slint-ui#4176
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch.
Sorry for the delayed review, i was on vacation earlier.
I have been thinking of ways to implement that before, and the idea i had was to do it a bit differently. Instead of having a specific command for rounded gradients, we would have a specific command for a rounded clip, with the number of command item that need to be clipped inside.
Then we'd call the draw_functions only within the clip.
(But then we would also need a solution for the anti-aliasing)
@@ -521,6 +485,244 @@ pub(super) fn draw_rounded_rectangle_line( | |||
}); | |||
} | |||
|
|||
// Border radius clipping utility functions | |||
// Extracted from draw_rounded_rectangle_line for reuse in gradient functions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Extracted from draw_rounded_rectangle_line for reuse in gradient functions |
This historical note will not be useful to someone reading the code later.
// Find a color for the border which is an equivalent to blend the background and then the border. | ||
// In the end, the resulting of blending the background and the color is | ||
// (A + B) + C, where A is the buffer color, B is the background, and C is the border. | ||
// which expands to (A*(1-Bα) + B*Bα)*(1-Cα) + C*Cα = A*(1-(Bα+Cα-Bα*Cα)) + B*Bα*(1-Cα) + C*Cα | ||
// so let the new alpha be: Nα = Bα+Cα-Bα*Cα, then this is A*(1-Nα) + N*Nα | ||
// with N = (B*Bα*(1-Cα) + C*Cα)/Nα | ||
// N being the equivalent color of the border that mixes the background and the border | ||
// In pre-multiplied space, the formula simplifies further N' = B'*(1-Cα) + C' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove this comment?
// No border radius - process normally | ||
match &args.background { | ||
Brush::LinearGradient(g) => { | ||
let angle = g.angle() + args.rotation.angle(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a lot of it is duplicated, maybe this could be moved into a function?
if is_point_inside_rounded_rect(pixel_x, pixel_y, rect, &rr.radius, rr.width) { | ||
let alpha = get_rounded_rect_alpha(pixel_x, pixel_y, rect, &rr.radius, rr.width); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that not quite slow to do for every pixel?
There should ideally be some caching and compute the region which can be rendered without having to call this.
self.red = ((self.red as u16 * a / 255) + color.red as u16).min(255) as u8; | ||
self.green = ((self.green as u16 * a / 255) + color.green as u16).min(255) as u8; | ||
self.blue = ((self.blue as u16 * a / 255) + color.blue as u16).min(255) as u8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this might be a very performance sensitive operation.
Please run the printerdemo_mcu benchmark and see if this doesn't regress.
Maybe this could be improved using the Rust's builtin saturating_add
function.
cargo bench -p printerdemo_mcu --
Resolves issue where gradients were not properly clipped along rounded rectangle boundaries in the software renderer.
Fixes: #4176