Skip to content
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

Infinitesimal stroked bezier fills whole tile #650

Closed
dominikh opened this issue Jul 31, 2024 · 5 comments · Fixed by #695
Closed

Infinitesimal stroked bezier fills whole tile #650

dominikh opened this issue Jul 31, 2024 · 5 comments · Fixed by #695

Comments

@dominikh
Copy link

The scene

let mut scene = Scene::new();
let mut p: BezPath = BezPath::default();
p.move_to((399.997, 400.0002));
p.quad_to((399.998, 400.0002), (400., 400.));
scene.stroke(
    &Stroke::default(),
    Affine::IDENTITY,
    Color::rgb(1., 0., 0.),
    None,
    &p,
);

renders as a solid square
image

This seems very sensitive to the coordinates, and many translations of the coordinates don't reproduce the issue. The same issue reproduces with a cubic bezier, with the point (400., 400.) repeated.

@raphlinus
Copy link
Contributor

raphlinus commented Jul 31, 2024

It's very likely this is another manifestation of #644 (edit: I meant #616), which I believe is a watertightness issue in the expanded stroke outlines. It's in my queue to look at.

@waywardmonkeys waywardmonkeys added this to the Vello 0.3 release milestone Aug 1, 2024
@raphlinus
Copy link
Contributor

I'm not able to repro directly (or #616 either) from the given example, but I do see an artifact with the following:

        let mut p: BezPath = BezPath::default();
        p.move_to((399.997, 400.0002));
        p.quad_to((399.998, 400.0002), (400., 400.));
        p.line_to((390., 390.));
        let a = Affine::translate((31.90625, 23.98875));
        scene.stroke(
            &Stroke::new(2.0).with_caps(Cap::Butt),
            a,
            Color::WHITE,
            None,
            &p,
        );

It definitely appears to be a watertightness bug, and I suspect it's the same underlying issue. I should be able to track it down now that I've got a repro.

Curious why the variation in reproducibility, but we'll most likely get to that when we have a proposed fix.

@DJMcNab
Copy link
Member

DJMcNab commented Aug 12, 2024

I suspect the difference could be a fast-math issue.

That's my general heuristic to explain if something is different on Mac :P

@raphlinus
Copy link
Contributor

I have a tentative analysis and proposed fix for this particular case. Basically the tangent computed for the start cap is not bit-identical to the tangent for the parallel curves (even though mathematically it's the same). Change line 847 of flatten.rs from:

                    let tangent = cubic_start_tangent(pts.p0, pts.p1, pts.p2, pts.p3);

to

                    let tangent = pts.p3 - pts.p0;

I'm not sure this fixes everything (still investigating), but did want to send an update.

@raphlinus
Copy link
Contributor

Quick followup on this, as I'm looking at PR'ing the change. It's to flatten.wsgl, not .rs, but of course a proper PR should also fix the CPU shader. Taking a look at this now, I think there are two cases: open subpaths (which the above patch addresses) and closed ones (which the above patch does not). I'll see if I can get a PR out soon.

github-merge-queue bot pushed a commit that referenced this issue Sep 23, 2024
At subpath end, the last path segment is encoded with the end point
chosen so that the vector from the current point to the end point is the
tangent, ie the same vector as cubic_start_tangent applied to the first
segment in the subpath. In those cases, compute the tangent by
subtracting those two points, rather than cubic_start_tangent applied to
the line. These are mathematically identical, but may give different
results because of roundoff.

There are two cases: an open subpath, in which case the case is applied
to the tangent at draw_start_cap, or a closed subpath, where the logic
is in the tangent calculation in read_neighboring_segment.

Both GPU and CPU shaders are updated. It hasn't been carefully
validated.

Hopefully fixes #616 and #650.
@DJMcNab DJMcNab linked a pull request Sep 23, 2024 that will close this issue
@DJMcNab DJMcNab closed this as completed Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants