-
Notifications
You must be signed in to change notification settings - Fork 142
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
Sweep gradients #435
Sweep gradients #435
Conversation
Implements support for rendering sweep gradients. Based on Skia's SkSweepGradient shader.
For reviewers: 0x254 = 0b001001_0_101_00 |
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.
Should this also be added to the fine
cpu shader (on a best-effort basis, given we can't really test it)? We probably should have a coherent policy on that (see also #386)
Otherwise, I've checked code quality. Haven't checked the maths, but it works in the test scenes you've created, on both CPU and GPU
let is_radial = y & 1 != 0; | ||
for (y, kind) in [Kind::Linear, Kind::Radial, Kind::Sweep] | ||
.into_iter() | ||
.enumerate() |
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.
These now go off-screen in the default resolution. I think the resolution
field of params
can be set to counteract this (see #453)
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 wasn't aware this existed! Added to this scene and made sure it scales properly.
src/cpu_shader/draw_leaf.rs
Outdated
let z = xform.0; | ||
let inv_det = (z[0] * z[3] - z[1] * z[2]).recip(); | ||
let inv_mat = [ | ||
z[3] * inv_det, | ||
-z[1] * inv_det, | ||
-z[2] * inv_det, | ||
z[0] * inv_det, | ||
]; | ||
let inv_tr = [ | ||
-(inv_mat[0] * z[4] + inv_mat[2] * z[5]), | ||
-(inv_mat[1] * z[4] + inv_mat[3] * z[5]), | ||
]; |
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 it worth adding the inverse
method to Transform
directly?
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.
Definitely. I considered this but side-stepped it when adding the code.
Done.
- use params.resolution to fit test scene to current viewport - move repeated inverse calculation to Transform::inverse() method Also add CPU implementation of radial gradients in draw_leaf.rs
Since we're still missing texture bindings in the CPU pipeline, we can't do much with this and my preference is to defer until that is more fully fleshed out. I did however go ahead and sneak the CPU-side radial gradient fixes into this PR. |
Implements support for rendering sweep gradients. Just had an itch to get this done, mostly to support COLRv1 fonts.
Based on Skia's
SkSweepGradient
shader.Here's a comparison of a sweep gradient with the three extend modes. Skia on top and Vello on bottom.