vello_hybrid: fix sweep gradient seam flicker#1352
Conversation
| // Before passing the position to the angle calculation, we bias | ||
| // very small coordinates to 0. Otherwise the sweep gradient's seam | ||
| // may flicker, because the angle calculation uses the coordinates' | ||
| // signs to select a quadrant. For coordinates around 0, slight | ||
| // noise in the coordinate calculation can then land the | ||
| // calculation in different quadrants. That flickering is quite | ||
| // noticeable as the seam is not anti-aliased, and may vary across | ||
| // machine. | ||
| grad_pos.x = select(grad_pos.x, 0.0, abs(grad_pos.x) < NEARLY_ZERO_TOLERANCE); | ||
| grad_pos.y = select(grad_pos.y, 0.0, abs(grad_pos.y) < NEARLY_ZERO_TOLERANCE); |
There was a problem hiding this comment.
There's an argument to be made for moving this into xy_to_unit_angle, but as that function is currently a relatively mechanical translation from Skia's code, I'm somewhat hesitant to do so.
|
I personally lean towards just removing all skew-only test cases (IIRC we have other test cases for gradients with transforms, so that should be covered anyway), and I remember there being another test case which we disabled because it caused issues in Windows CI. After all, applying a small delta might fix it for the 45 degree case, but it doesn't fundamentally solve the problem of floating point inaccuracies. |
I think this issue isn't related to skew (even though it happens to show up in this skew test). I expect to see the same flickering for a 45 degree rotation. I'll try that later. |
Oh yeah, it probably also happens for rotations. Otherwise, you could try changing the angle so that it's not exactly 45 degrees. |
|
The following is --- a/sparse_strips/vello_sparse_tests/tests/gradient.rs
+++ b/sparse_strips/vello_sparse_tests/tests/gradient.rs
@@ -1052,7 +1052,7 @@ mod sweep {
fn gradient_sweep_with_transform_rotate_1(ctx: &mut impl Renderer) {
gradient_with_transform(
ctx,
- Affine::rotate_about(50.0_f64.to_radians(), Point::new(50.0, 50.0)),
+ Affine::rotate_about(45.0_f64.to_radians(), Point::new(50.0, 50.0)),
25.0,
25.0,
75.0,
|
|
Maybe we just apply a 90 degrees rotation instead of 45 in the tests? Then it should work, right? |
tomcur
added
C-hybrid
The tests would be deterministic, but the issue of the sweep gradient seam flickering would remain. As the transforms to hit this are quite reasonable, it wouldn't take much for an application to encounter the issue |
On some machines (mine!) the sweep gradient seam currently flickers in the hybrid renderer, because of some floating point inaccuracy. That inaccuracy may vary across machines. This causes the `gradient_sweep_with_transform_skew_y_1_hybrid` test to fail on `main` on my machine. Before passing the position to the angle calculation, we bias very small coordinates to 0. Otherwise the sweep gradient's seam may flicker, because the angle calculation uses the coordinates' signs to select a quadrant. For coordinates around 0, slight noise in the coordinate calculation can then land the calculation in different quadrants. That flickering is quite noticeable as the seam is not anti-aliased, and may vary across machine.
tomcur
force-pushed
the
gradient-flicker
branch
from
39ac419 to
f58150d
Compare
Co-authored-by: Alex Gemberg <gemberg@canva.com>
LaurenzV
left a comment
LaurenzV
left a comment
There was a problem hiding this comment.
So we are just applying this change in the GPU shader? Doesn't that mean that vello_cpu and vello_hybrid have different behavior now? I think it would be good to align them.
|
I haven't seen this issue in vello_cpu. Perhaps the issue is caused by floating point behavior falling slightly differently on some GPU hardware? |
3 participants
On some machines (mine!) the sweep gradient seam currently flickers in the hybrid renderer, because of some floating point inaccuracy. That inaccuracy may vary across machines. This causes the
gradient_sweep_with_transform_skew_y_1_hybridtest to fail onmainon my machine.The following is the failing test's snapshot diff on my machine. Notice the flickering seam where blue and yellow meet in the right-hand side's gradient.
Before passing the position to the angle calculation, this now biases very small coordinates to 0. Otherwise the sweep gradient's seam may flicker, because the angle calculation uses the coordinates' signs to select a quadrant. For coordinates around 0, slight noise in the coordinate calculation can then land the calculation in different quadrants. That flickering is quite noticeable as the seam is not anti-aliased, and may vary across machine.