error(168): Using 'return' in 'fragment' processor function results in undefined behavior

[Giwayume]

error(168): Using 'return' in 'fragment' processor function results in undefined behavior

Godot 3.5.1 stable complains about this error when compiling the shader.

Adding a couple of else statements seems to work around it, though the fill rule calculation in this shader does not appear to be stable on my laptop's integrated GPU at least:

Always a possibility it was a mistake on my part when editing the shader.

SVG used:

[SlugFiller]

Fixed in a76e934d6f581566fd7df267e854eff982fb2013

The graphical issue probably has to do with how you updated the shader. At the very least, it's working perfect for me after fixing the shader.

[Giwayume]

Cool. I updated, but the artifacts still exist. So it is a problem on that GPU. Or perhaps, a problem under Linux.

Here's specs if it helps: Ubuntu 20.04.3 LTS Intel Core i5-4210U Intel HD Graphics 4400 (HSW GT2)

Maybe test in a Linux distro if you haven't already, and see if it's as simple as that. Otherwise, I'll just close as it may be difficult for you to reproduce.

[Giwayume]

I wonder if it's reading junk from an unitialized buffer. I've seen this problem before where creating PoolIntArray or PoolRealArray and resizing it without setting values has junk from memory by default.

[SlugFiller]

Unlikely, since nothing is passed at a per-texture resolution to the shader, and the shader itself can't contain, let alone output, arrays of such dimensions. If it was an issue with the buffers being passed, you'd see the entire outline of the shape bending, not just a few pixels being "off".

I see two possible culprits to this:

1. Loops. Shaders don't like them very much. If the shader "breaks early" out of a loop, it could end up with an incorrect result.
2. Floating point precision. While I've done my best to minimize divisions inside the tracing code, it's still possible for rounding errors to cause issues. I've actually had to make various adjustments to use of < vs <= in various points of the shader to reduce chances of rounding errors effecting the trace. But I suppose it's not 100%. High precision should be the default, but I suppose there are no real guarantees when it comes to different hardware.

An example of trying to resolve 2 would be changing lines like:

    if (d > 0.0 && d < dot(end-start, end-start)) {
        d = abs(dot(vec2(end.y, -end.x), end-start)/distance(start, end));
        if (max_distance > d) {
            max_distance = d;
        }
    }

To

    if (d > 0.0 && d < dot(end-start, end-start)) {
        float d2 = distance(start, end));
        d = abs(dot(vec2(end.y, -end.x), end-start);
        if (max_distance*d2 > d) {
            max_distance = d/d2;
        }
    }

Or

    if (start.x-start.y*(end.x-start.x)/(end.y-start.y) < 0.0) {
        return 0;
    }

To

    if (end.y > start.y) {
        if (start.x*(end.y-start.y) < start.y*(end.x-start.x)) {
            return 0;
        }
    }
    else {
        if (start.x*(end.y-start.y) > start.y*(end.x-start.x)) {
            return 0;
        }
    }

But since I can't reproduce the issue, I can't test such solutions myself.

Incidentally, you should test with different feather values, to see if this is an issue with the distance measuring code, or with the winding counting code.

[Giwayume]

You should be able to specify high precision via highp when creating float variables, so it is not up to the default settings of the graphics driver.

https://docs.godotengine.org/en/3.0/tutorials/shading/shading_language.html#precision

[SlugFiller]

The specs specifically say (emphasis mine):

highp vec4 a = vec4(0.0, 1.0, 2.0, 3.0); // high precision, uses full float or integer range (default)

So if lower floating point precision is used, it's either a bug, or a limitation of the hardware.

Explicitly adding highp to every variable and parameter definition is doable, but not necessarily beneficial.

[Giwayume]

Worth a try at least, if precision is a suspect. I don't really trust the one-off comments Godot docs, because there's so many instances where someone changed the source code and never updated the docs, or some GPU driver works differently from what was tested. Especially on mobile devices.

[Giwayume]

Hey, this one fixed it! Good intuition!

        if (end.y > start.y) {
        if (start.x*(end.y-start.y) < start.y*(end.x-start.x)) {
            return 0;
        }
    }
    else {
        if (start.x*(end.y-start.y) > start.y*(end.x-start.x)) {
            return 0;
        }
    }

[Giwayume]

Well, that greatly reduced the artifacting. I can still see at specific zoom levels some rows break. I'll test if you think of anything else.

[SlugFiller]

The issue is due to the way winding is calculated when points have a value of 0. You have to deal with the following scenarios:

1. (1,1)->(2,-1) - sum winding should be -1
2. (1,-1)->(2,1) - sum winding should be 1
3. (1,1)->(1,0)->(2,0)->(2,1) - sum winding should be 0
4. (1,1)->(1,0)->(2,0)->(2,-1) - sum winding should be -1
5. (1,-1)->(1,0)->(2,0)->(2,1) - sum winding should be 1
6. (1,-1)->(1,0)->(2,0)->(2,-1) - sum winding should be 0

My current strategy is to ignore the case where end.y is 0. This allows cases 1, 2, 4, and 5 to process correctly, but causes 3 and 6 to incorrectly give 1 and -1 respectively, instead of 0.

I have a different strategy in mind, that relies on having "half winding", if one of the values is 0. That way, if it goes twice in the same direction, it would add half and half, and result in 1. If it goes in opposite directions, it would cancel out.

Here's a possible implementation:

int calculate_winding_linear(vec2 start, vec2 end) {
    if (start.x < 0.0 && end.x < 0.0) {
        return 0;
    }
    if (start.y < 0.0 && end.y < 0.0) {
        return 0;
    }
    if (start.y > 0.0 && end.y > 0.0) {
        return 0;
    }
    if (start.y == 0.0) {
        if (start.x < 0.0) {
            return 0;
        }
        if (end.y == 0.0) {
            return 0;
        }
        if (end.y > 0.0) {
            return 1;
        }
        return -1;
    }
    if (end.y == 0.0) {
        if (end.x < 0.0) {
            return 0;
        }
        if (start.y == 0.0) {
            return 0;
        }
        if (start.y < 0.0) {
            return 1;
        }
        return -1;
    }
    if (end.y > start.y) {
        if (start.x*(end.y-start.y) < start.y*(end.x-start.x)) {
            return 0;
        }
    }
    else {
        if (start.x*(end.y-start.y) > start.y*(end.x-start.x)) {
            return 0;
        }
    }
    if (start.y > 0.0) {
        return -2;
    }
    return 2;
}

[Giwayume]

Yep, that version of the function definitely works better. I can't find any line artifacts.

[SlugFiller]

Fixed by 1f14a3521949c46eb7ba267cee4126306a144dc4

I also had to make a small adjustment to make even/odd winding work correctly, even though it is rarely used (non-zero is much easier to work with).