Fix error in clip reduction #659
Conversation
One bit of logic from the GLSL (piet-gpu) version of clip_reduce didn't get copied over. Fixes #648
Add many_clips test scene and snapshot test. Note that doing more than 128 clips, while it would exceed the 256 clip object limit (counting both begin and end clip), would not trigger the failure, as the end clip would be on a workgroup boundary. Using multiples of 3 (as in the original report) triggers the bug, and a full system hang was verified with the fix disabled.
| Affine::translate((100. * (x as f64 + 0.5), 100. * (y as f64 + 0.5))); | ||
| const CLIPS_PER_FILL: usize = 3; | ||
| for _ in 0..CLIPS_PER_FILL { | ||
| let rot = Affine::rotate(rng.gen_range(0.0..PI)); |
There was a problem hiding this comment.
I believe that the root path is rotationally symmetric every
I think that means there's a bias towards the range which is equivalent to
Practically, I don't think this matters here, but I'm wondering if this is an intentional choice.
(And of course, my maths understanding could be very wrong :P)
There was a problem hiding this comment.
Heh, you're right, there is a bias. Not thinking it through carefully, I assumed the symmetry would make it still uniform, and I wanted to make the code concise. I think I'll leave it, in the sprit of kintsugi.
There was a problem hiding this comment.
We could use TAU) here to still be fairly concise.
But practically, this is great, and the bias isn't an issue!
| scene.push_layer(Mix::Clip, 1.0, translate * rot, &base_tri); | ||
| } | ||
| let rot = Affine::rotate(rng.gen_range(0.0..PI)); | ||
| let color = Color::rgb(rng.gen(), rng.gen(), rng.gen()); |
There was a problem hiding this comment.
This produces remarkably pleasing colours considering that it's completely random - it's probably a good technique to use in other examples.
One bit of logic from the GLSL (piet-gpu) version of clip_reduce didn't get copied over.
Fixes #648