The current state of this is "works for me," and I'm feeling a bit uncertain whether it's fully portable. If we do get hit by fastmath issues, which is entirely plausible given the weak guarantees of the language, then I'm inclined to take this in a somewhat different direction: revert the encoding changes so that the final segment writes the unmodified point values, but change the GPU-side tangent calculation so that it's based on the encoded values rather than the cubic points after degree raising.

So I'm uploading this hoping to get more feedback. If it generally works, I think we can go forward.

As I mentioned on Zulip, I still see some watertightness failures detected by #416 with this PR. Are those out-of-scope for this change?

I would also like to see a snapshot test for the longpathdash test scene added (ideally validated as showing the failure)

I'll dig into this. I do think fixing watertightness issues is in scope, but also would consider unblocking 0.3 if we have no known regressions.

I've done a fair amount of investigation.

I did implement the calculation of tangents before degree raising. It didn't make a significant difference, so I'm not sure it's worth preparing the patch. It is possible that on some GPU it would be an improvement.

The watertightness issues get significantly worse when there is a rotation transform. I have a patch (which I will upload as a commit to this PR) that changes transform_apply (in flatten.wgsl) to use fma() instead of multiplication and addition, and that behavior goes away. That is extremely strong evidence that we're being hit by fastmath, and that in particular the same calculation in the source is being compiled with different rounding behavior in different callsites.

I'm doing some more exploration, but I'm inclined to declare victory soon, as we have a state that is not a regression and is in fact considerably better (with rotations) than any previous state. To solve this completely systematically may require a profound rethink - generate path points once then use indirection to use the same point as both the line start of one segment and line end of another, which would guarantee watertightness by construction.

As of the most recent commit, the new test is validated by failing on main.

Okay, so I'm a bit late to review this, but one nitpick:

This PR says it matches CPU and GPU logic. But the main change in shader code is:

    // Degree-raise
    if seg_type == PATH_TAG_LINETO {
        p3 = p1;
        p2 = p3 + (1.0 / 3.0) * (p0 - p3);
        p1 = p0 + (1.0 / 3.0) * (p3 - p0);
    } else if seg_type == PATH_TAG_QUADTO {
        p3 = p2;
        p2 = p1 + (1.0 / 3.0) * (p2 - p1);
        p1 = p1 + (1.0 / 3.0) * (p0 - p1);
    }

Whereas the equivalent code in the CPU shader in vello_shaders/src/cpu/flatten.rs is still:

    // Degree-raise
    if seg_type == PATH_TAG_LINETO {
        p3 = p1;
        p2 = p3.mix(p0, 1.0 / 3.0);
        p1 = p0.mix(p3, 1.0 / 3.0);
    } else if seg_type == PATH_TAG_QUADTO {
        p3 = p2;
        p2 = p1.mix(p2, 1.0 / 3.0);
        p1 = p1.mix(p0, 1.0 / 3.0);
    }

That is, same as the old GPU code.

Not sure what's going on here. Probably the CPU shader should be edited to match the change in the GPU shader?

Those look the same to me? Rust doesn't have fast math.

Unless you're claiming that mix isn't the same operation as the CPU side implementation?

Well the GPU side went from using mix to explicitly using operators. I assumed there was a reason for that. Maybe I'm reading too much into it.

Note: this changes the GPU calculation to write out the lerp calculation explicitly rather than use the mix intrinsic, so we can rely on the rounding behavior. In the presence of fastmath, the rounding behavior is not guaranteed, but it is verified to work on M1.

From the pr description

I should really read PR descriptions more often. I don't know when I got the habit of not doing that.