Anti-Alias with complex skia.Path triggers shader compilation error and failed drawing

[stenson]

Describe the bug
First off, just want to say I'm a big fan of skia-python! But I recently heard from some users of my library coldtype (which uses skia-python extensively), that when they try to use coldtype with Python 3.12, the only option is to install 119.0b4 which triggers all kinds of issues. I've started looking into the issues (many of which are related to the image changes, I think), but there's one fundamental bug I'd like to figure out before the others, which is that I can't seem to reliably draw complex curves with anti-aliasing without triggering some obscure shader compilation errors.

To Reproduce
I'm running this script on Python 3.12.0 on macOS 12.5.1 (a slightly modified version of the glfw example code here):

import contextlib, glfw, skia
from OpenGL import GL

WIDTH, HEIGHT = 640, 480

path = skia.Path()
path.moveTo(184, 445)
path.lineTo(249, 445)
path.quadTo(278, 445, 298, 438)
path.quadTo(318, 431, 331, 419)
path.quadTo(344, 406, 350, 390)
path.quadTo(356, 373, 356, 354)
path.quadTo(356, 331, 347, 312)
path.quadTo(338, 292, 320, 280) # <- comment out this line and shape will draw correctly with anti-aliasing
path.close()

@contextlib.contextmanager
def glfw_window():
    if not glfw.init():
        raise RuntimeError('glfw.init() failed')
    glfw.window_hint(glfw.STENCIL_BITS, 8)
    window = glfw.create_window(WIDTH, HEIGHT, '', None, None)
    glfw.make_context_current(window)
    yield window
    glfw.terminate()

@contextlib.contextmanager
def skia_surface(window):
    context = skia.GrDirectContext.MakeGL()
    (fb_width, fb_height) = glfw.get_framebuffer_size(window)
    backend_render_target = skia.GrBackendRenderTarget(
        fb_width,
        fb_height,
        0,  # sampleCnt
        0,  # stencilBits
        skia.GrGLFramebufferInfo(0, GL.GL_RGBA8))
    surface = skia.Surface.MakeFromBackendRenderTarget(
        context, backend_render_target, skia.kBottomLeft_GrSurfaceOrigin,
        skia.kRGBA_8888_ColorType, skia.ColorSpace.MakeSRGB())
    assert surface is not None
    yield surface
    context.abandonContext()

with glfw_window() as window:
    GL.glClear(GL.GL_COLOR_BUFFER_BIT)

    with skia_surface(window) as surface:
        with surface as canvas:
            canvas.drawCircle(100, 100, 40, skia.Paint(Color=skia.ColorGREEN, AntiAlias=True))

            paint = skia.Paint(Color=skia.ColorBLUE)
            paint.setStyle(skia.Paint.kStroke_Style)
            paint.setStrokeWidth(2)
            paint.setAntiAlias(True)
            
            canvas.drawPath(path, paint)

        surface.flushAndSubmit()
        glfw.swap_buffers(window)

        while (glfw.get_key(window, glfw.KEY_ESCAPE) != glfw.PRESS
            and not glfw.window_should_close(window)):
            glfw.wait_events()

On my setup, running that code produces this output:

Shader compilation error
------------------------
   1    #version 110
   2
   3    uniform vec4 sk_RTAdjust;
   4    uniform vec2 uatlas_adjust_S0;
   5    attribute vec4 fillBounds;
   6    attribute vec4 color;
   7    attribute vec4 locations;
   8    varying vec2 vatlasCoord_S0;
   9    varying vec4 vcolor_S0;
  10    void main() {
  11        vec2 unitCoord = vec2(float(gl_VertexID & 1), float(gl_VertexID >> 1));
  12        vec2 devCoord = mix(fillBounds.xy, fillBounds.zw, unitCoord);
  13        vec2 atlasTopLeft = vec2(abs(locations.x) - 1.0, locations.y);
  14        vec2 devTopLeft = locations.zw;
  15        bool transposed = locations.x < 0.0;
  16        vec2 atlasCoord = devCoord - devTopLeft;
  17        if (transposed) {
  18            atlasCoord = atlasCoord.yx;
  19        }
  20        atlasCoord += atlasTopLeft;
  21        vatlasCoord_S0 = atlasCoord * uatlas_adjust_S0;
  22        vcolor_S0 = color;
  23        gl_Position = vec4(devCoord, 0.0, 1.0);
  24        gl_Position = vec4(gl_Position.xy * sk_RTAdjust.xz + gl_Position.ww * sk_RTAdjust.yw, 0.0, gl_Position.w);
  25    }
  26
Errors:
ERROR: 0:11: Use of undeclared identifier 'gl_VertexID'
ERROR: 0:11: Use of undeclared identifier 'gl_VertexID'
ERROR: 0:12: Use of undeclared identifier 'unitCoord'
ERROR: 0:16: Use of undeclared identifier 'devCoord'
ERROR: 0:18: Use of undeclared identifier 'atlasCoord'
ERROR: 0:18: Use of undeclared identifier 'atlasCoord'
ERROR: 0:20: Use of undeclared identifier 'atlasCoord'
ERROR: 0:21: Use of undeclared identifier 'atlasCoord'
ERROR: 0:23: Use of undeclared identifier 'devCoord'

Expected behavior
For the curve to be drawn on screen without a shader compilation error. With 87.5 running the same code, this is the graphical output:

Desktop (please complete the following information):

• OS: macOS 12.5.1
• Python: 3.12.0
• skia-python version: 119.0b4

Additional context
I realize these releases are still pre-release, but as they install by default with Python3.12, I wanted to get ahead of the skia updates to make sure Coldtype can continue working correctly. Thanks for all the hard work on this awesome library!

[HinTak]

I'll have a look and see if I can reproduce the problem on Linux with 120bX (I only really have access to Linux - the rest is just pushing into github's CI, with the pytests and the logs).

However, I had some exchanges with @kyamagu at the beginning about the needs of having another 87.X release, and this (just back-porting the CI changes specific to python 3.12 support) seems simple enough to do.

I'll have a look at both.

[HinTak]

It doesn't look like github let's me run CI without it mergeable against an existing branch (so I tried in #216 #215 and closed them myself), so I have to create a v87.5 branch first. I have done so on my playground
https://github.com/HinTak/skia-m1xx-python/actions/runs/6998632212 instead.

If CI finishes over there, you @stenson can download the pip wheel from the artefact there instead. Or if you insist, we can put a v87.5 branch here and host the CI here later. (and possibly make a release out of a branch here).

[HinTak]

Oh, it runs nicely on 120bX here on Linux python 3.11... am afraid this is a Mac OS X GL problem, I think. GL on mac os X is behind, I think.

[HinTak]

CI finished, failing only later at doc building (a known issue) so I back-ported the doc building fix too. Grab the pip wheel from this https://github.com/HinTak/skia-m1xx-python/actions/runs/7000261476 if it makes you happier, but the earlier one should work again python 3.12 too.

[stenson]

Thanks for looking into this — I'm not too worried about an 87.5 build that works with python 3.12 as the 87.5 builds on previous versions of python work just fine, I'm more just interested in making sure future releases of skia-python will continue to work on macOS. When you say GL on mac is behind, I'm not sure I understand, isn't skia meant to be cross-platform?

[HinTak]

It is a bit more complicated than that - between m87 and m116 about 20% of skia API changed ... and google folks are quite aggressively changing things every release.

As for GL , which is exploiting graphic card's capabilility with driver support plus software emulation fallbacks, the last mile - software emulation fallbacks, is provided by ANGLE on windows (GL on directX, I think) from Google, OpenGL from Apple, and mesa on Linux. And I meant Apple's OpenGL is significantly behind mesa, I seem to have heard. And Google stick quite close to what latest mesa can do.

@stenson one question for you - is problem on arm CPU or on Intel CPU? We currently don't CI-test GL on arm-based macs since github CI 's arm mac image can't do it. I filed #217 and just closed it myself thinking that might be a issue.

The other thing is min-os version - I just bumped this up b0e3f3c - and would appreciate you test and file problems against that artefact when it finishes building, if possible, for the next release. We don't want to set it too high (dropping support for old mac os x), but it might be necessary.

Skia isn't as cross-platform as one would like - sometimes it is underlying os limitations. I recently closed two font-related issues for which can be worked around if we bundle freetype on non-Linux, and use that instead of coretext(mac) or directwrite(windows) to load fonts. (#138 #195).

That said, if you have more problems and/or missing API using m119/120 vs m87, we definitely want to know.

[HinTak]

@stenson I have gone back and revisited some of the Image.* which got broken in the m87 to m116 transition and re-enabled them again:
fb46ad4 these might be what you think are missing? The initial work was done by somebody who noticed something missing too, and this being in textured backed images, probably on the general area of your interests.

If you have anything else obvious and simple enough to put into m120, let me know soon.

[stenson]

@HinTak sorry for the delay here — I'm not quite sure I understand how to test these changes before they make it onto pypi (I tried pip installing from the specific sha but skia compilation fails on my setup). That said, I've just tested 120.0b5 from pypi and I'm getting the exact same error from the start of this thread.

And I am on an armbased mac to answer your earlier question.

[HinTak]

@stenson 120 b5 just went out today. If CI is successful, binary snapshot builds are available under https://github.com/kyamagu/skia-python/actions . Anyway, the reason why I ask about arm vs Intel mac is because github CI doesn't run pytest with arm mac's so GL on arm macs is different from intel macs according to github's CI infrastructure.

I am thinking of converting this example to ru n headless to see if it works on Intel macs. The other thing is, you might look into the Metal equivalent as that's what Apple wants you to use instead of GL. We might switch on the Metal code in m121.

[stenson]

Been looking into this further — I've gotten the full coldtype library working with 121.0b5 with some hacks (always drawing to an image rather than drawing curves directly to an opengl-backed window), but even drawing an image to the opengl-backed window is much slower than the equivalent operation with python3.11 + skia-python m87.

If switching on the metal code is still an option, I think that'd be a great change to make sure skia-python can be performant on macOS.

[pavpanchekha]

I seem to have a similar error in a case with AntiAlias. The Python code is the following:

        path = skia.Path().moveTo(self.x1, self.y1).lineTo(self.x2, self.y2)
        paint = skia.Paint(Color=parse_color(self.color))
        paint.setStyle(skia.Paint.kStroke_Style)
        paint.setStrokeWidth(self.thickness)
        canvas.drawPath(path, paint)

The compilation error is:

Shader compilation error
------------------------
   1	#version 110
   2	
   3	
   4	float _determinant2(mat2 m) {
   5	return m[0].x*m[1].y - m[0].y*m[1].x;
   6	}
   7	const float PI = 3.14159274;
   8	const float PRECISION = 4.0;
   9	const float NUM_TOTAL_EDGES = 16383.0;
  10	uniform vec4 sk_RTAdjust;
  11	uniform vec3 utessControlArgs_S0;
  12	uniform vec4 uaffineMatrix_S0;
  13	uniform vec2 utranslate_S0;
  14	attribute vec4 pts01Attr;
  15	attribute vec4 pts23Attr;
  16	attribute vec2 argsAttr;
  17	attribute float curveTypeAttr;
  18	vec2 robust_normalize_diff_f2f2f2(vec2 a, vec2 b) {
  19	    vec2 diff = a - b;
  20	    if (diff == vec2(0.0)) {
  21	        return vec2(0.0);
  22	    } else {
  23	        float invMag = 1.0 / max(abs(diff.x), abs(diff.y));
  24	        return normalize(invMag * diff);
  25	    }
  26	}
  27	vec2 unchecked_mix_f2f2f2f(vec2 a, vec2 b, float T) {
  28	    return ((b - a) * (vec2(T)) + (a));
  29	}
  30	float wangs_formula_max_fdiff_p2_ff2f2f2f2f22(vec2 p0, vec2 p1, vec2 p2, vec2 p3, mat2 matrix) {
  31	    vec2 d0 = matrix * (((vec2(-2.0)) * (p1) + (p2)) + p0);
  32	    vec2 d1 = matrix * (((vec2(-2.0)) * (p2) + (p3)) + p1);
  33	    return max(dot(d0, d0), dot(d1, d1));
  34	}
  35	float wangs_formula_conic_p2_fff2f2f2f(float _precision_, vec2 p0, vec2 p1, vec2 p2, float w) {
  36	    vec2 C = (min(min(p0, p1), p2) + max(max(p0, p1), p2)) * 0.5;
  37	    p0 -= C;
  38	    p1 -= C;
  39	    p2 -= C;
  40	    float m = sqrt(max(max(dot(p0, p0), dot(p1, p1)), dot(p2, p2)));
  41	    vec2 dp = ((vec2(-2.0 * w)) * (p1) + (p0)) + p2;
  42	    float dw = abs(((-2.0) * (w) + (2.0)));
  43	    float rp_minus_1 = max(0.0, ((m) * (_precision_) + (-1.0)));
  44	    float numer = length(dp) * _precision_ + rp_minus_1 * dw;
  45	    float denom = 4.0 * min(w, 1.0);
  46	    return numer / denom;
  47	}
  48	void main() {
  49	    float NUM_RADIAL_SEGMENTS_PER_RADIAN = utessControlArgs_S0.x;
  50	    float JOIN_TYPE = utessControlArgs_S0.y;
  51	    float STROKE_RADIUS = utessControlArgs_S0.z;
  52	    mat2 AFFINE_MATRIX = mat2(uaffineMatrix_S0.xy, uaffineMatrix_S0.zw);
  53	    vec2 TRANSLATE = utranslate_S0;
  54	    vec2 p0 = pts01Attr.xy;
  55	    vec2 p1 = pts01Attr.zw;
  56	    vec2 p2 = pts23Attr.xy;
  57	    vec2 p3 = pts23Attr.zw;
  58	    vec2 lastControlPoint = argsAttr;
  59	    float w = -1.0;
  60	    if (curveTypeAttr != 0.0) {
  61	        w = p3.x;
  62	        p3 = p2;
  63	    }
  64	    float numParametricSegments;
  65	    if (w < 0.0) {
  66	        if (p0 == p1 && p2 == p3) {
  67	            numParametricSegments = 1.0;
  68	        } else {
  69	            float _0_m = wangs_formula_max_fdiff_p2_ff2f2f2f2f22(p0, p1, p2, p3, AFFINE_MATRIX);
  70	            numParametricSegments = max(ceil(sqrt(3.0 * sqrt(_0_m))), 1.0);
  71	        }
  72	    } else {
  73	        float _1_n2 = wangs_formula_conic_p2_fff2f2f2f(PRECISION, AFFINE_MATRIX * p0, AFFINE_MATRIX * p1, AFFINE_MATRIX * p2, w);
  74	        numParametricSegments = max(ceil(sqrt(_1_n2)), 1.0);
  75	    }
  76	    vec2 tan0 = robust_normalize_diff_f2f2f2(p0 == p1 ? (p1 == p2 ? p3 : p2) : p1, p0);
  77	    vec2 tan1 = robust_normalize_diff_f2f2f2(p3, p3 == p2 ? (p2 == p1 ? p0 : p1) : p2);
  78	    if (tan0 == vec2(0.0)) {
  79	        tan0 = vec2(1.0, 0.0);
  80	        tan1 = vec2(-1.0, 0.0);
  81	    }
  82	    float edgeID = float(gl_VertexID >> 1);
  83	    if ((gl_VertexID & 1) != 0) {
  84	        edgeID = -edgeID;
  85	    }
  86	    float numEdgesInJoin = 4.0;
  87	    float turn = _determinant2(mat2(p2 - p0, p3 - p1));
  88	    float combinedEdgeID = abs(edgeID) - numEdgesInJoin;
  89	    if (combinedEdgeID < 0.0) {
  90	        tan1 = tan0;
  91	        if (lastControlPoint != p0) {
  92	            tan0 = robust_normalize_diff_f2f2f2(p0, lastControlPoint);
  93	        }
  94	        turn = _determinant2(mat2(tan0, tan1));
  95	    }
  96	    float cosTheta = clamp(dot(tan0, tan1), -1.0, 1.0);
  97	    float rotation = acos(cosTheta);
  98	    if (turn < 0.0) {
  99	        rotation = -rotation;
 100	    }
 101	    float numRadialSegments;
 102	    float strokeOutset = sign(edgeID);
 103	    if (combinedEdgeID < 0.0) {
 104	        numRadialSegments = numEdgesInJoin - 2.0;
 105	        numParametricSegments = 1.0;
 106	        p3 = (p2 = (p1 = p0));
 107	        combinedEdgeID += numRadialSegments + 1.0;
 108	        float sinEpsilon = 0.01;
 109	        bool tangentsNearlyParallel = abs(turn) * (1.0 / sqrt(dot(tan0, tan0) * dot(tan1, tan1))) < sinEpsilon;
 110	        if (!tangentsNearlyParallel || dot(tan0, tan1) < 0.0) {
 111	            if (combinedEdgeID >= 0.0) {
 112	                strokeOutset = turn < 0.0 ? min(strokeOutset, 0.0) : max(strokeOutset, 0.0);
 113	            }
 114	        }
 115	        combinedEdgeID = max(combinedEdgeID, 0.0);
 116	    } else {
 117	        float maxCombinedSegments = (NUM_TOTAL_EDGES - numEdgesInJoin) - 1.0;
 118	        numRadialSegments = max(ceil(abs(rotation) * NUM_RADIAL_SEGMENTS_PER_RADIAN), 1.0);
 119	        numRadialSegments = min(numRadialSegments, maxCombinedSegments);
 120	        numParametricSegments = min(numParametricSegments, (maxCombinedSegments - numRadialSegments) + 1.0);
 121	    }
 122	    float radsPerSegment = rotation / numRadialSegments;
 123	    float numCombinedSegments = (numParametricSegments + numRadialSegments) - 1.0;
 124	    bool isFinalEdge = combinedEdgeID >= numCombinedSegments;
 125	    if (combinedEdgeID > numCombinedSegments) {
 126	        strokeOutset = 0.0;
 127	    }
 128	    if (abs(edgeID) == 2.0) {
 129	        float _2_x = ((cosTheta) * (0.5) + (0.5));
 130	        strokeOutset *= (_2_x * JOIN_TYPE) * JOIN_TYPE >= 1.0 ? (1.0 / sqrt(_2_x)) : sqrt(_2_x);
 131	    }
 132	    vec2 tangent;
 133	    vec2 strokeCoord;
 134	    if (combinedEdgeID != 0.0 && !isFinalEdge) {
 135	        vec2 A;
 136	        vec2 B;
 137	        vec2 C = p1 - p0;
 138	        vec2 D = p3 - p0;
 139	        if (w >= 0.0) {
 140	            C *= w;
 141	            B = 0.5 * D - C;
 142	            A = (w - 1.0) * D;
 143	            p1 *= w;
 144	        } else {
 145	            vec2 E = p2 - p1;
 146	            B = E - C;
 147	            A = ((vec2(-3.0)) * (E) + (D));
 148	        }
 149	        vec2 B_ = B * (numParametricSegments * 2.0);
 150	        vec2 C_ = C * (numParametricSegments * numParametricSegments);
 151	        float lastParametricEdgeID = 0.0;
 152	        float maxParametricEdgeID = min(numParametricSegments - 1.0, combinedEdgeID);
 153	        float negAbsRadsPerSegment = -abs(radsPerSegment);
 154	        float maxRotation0 = (1.0 + combinedEdgeID) * abs(radsPerSegment);
 155	        for (int _0_exp = 4;_0_exp >= 0; --_0_exp) {
 156	            float testParametricID = lastParametricEdgeID + exp2(float(_0_exp));
 157	            if (testParametricID <= maxParametricEdgeID) {
 158	                vec2 testTan = ((vec2(testParametricID)) * (A) + (B_));
 159	                testTan = ((vec2(testParametricID)) * (testTan) + (C_));
 160	                float cosRotation = dot(normalize(testTan), tan0);
 161	                float maxRotation = ((testParametricID) * (negAbsRadsPerSegment) + (maxRotation0));
 162	                maxRotation = min(maxRotation, PI);
 163	                if (cosRotation >= cos(maxRotation)) {
 164	                    lastParametricEdgeID = testParametricID;
 165	                }
 166	            }
 167	        }
 168	        float parametricT = lastParametricEdgeID / numParametricSegments;
 169	        float lastRadialEdgeID = combinedEdgeID - lastParametricEdgeID;
 170	        float angle0 = acos(clamp(tan0.x, -1.0, 1.0));
 171	        angle0 = tan0.y >= 0.0 ? angle0 : -angle0;
 172	        float radialAngle = ((lastRadialEdgeID) * (radsPerSegment) + (angle0));
 173	        tangent = vec2(cos(radialAngle), sin(radialAngle));
 174	        vec2 norm = vec2(-tangent.y, tangent.x);
 175	        float a = dot(norm, A);
 176	        float b_over_2 = dot(norm, B);
 177	        float c = dot(norm, C);
 178	        float discr_over_4 = max(b_over_2 * b_over_2 - a * c, 0.0);
 179	        float q = sqrt(discr_over_4);
 180	        if (b_over_2 > 0.0) {
 181	            q = -q;
 182	        }
 183	        q -= b_over_2;
 184	        float _5qa = (-0.5 * q) * a;
 185	        vec2 root = abs(((q) * (q) + (_5qa))) < abs(((a) * (c) + (_5qa))) ? vec2(q, a) : vec2(c, q);
 186	        float radialT = root.y != 0.0 ? root.x / root.y : 0.0;
 187	        radialT = clamp(radialT, 0.0, 1.0);
 188	        if (lastRadialEdgeID == 0.0) {
 189	            radialT = 0.0;
 190	        }
 191	        float T = max(parametricT, radialT);
 192	        vec2 ab = unchecked_mix_f2f2f2f(p0, p1, T);
 193	        vec2 bc = unchecked_mix_f2f2f2f(p1, p2, T);
 194	        vec2 cd = unchecked_mix_f2f2f2f(p2, p3, T);
 195	        vec2 abc = unchecked_mix_f2f2f2f(ab, bc, T);
 196	        vec2 bcd = unchecked_mix_f2f2f2f(bc, cd, T);
 197	        vec2 abcd = unchecked_mix_f2f2f2f(abc, bcd, T);
 198	        float u = ((w - 1.0) * (T) + (1.0));
 199	        float v = (w + 1.0) - u;
 200	        float uv = ((v - u) * (T) + (u));
 201	        if (T != radialT) {
 202	            tangent = w >= 0.0 ? robust_normalize_diff_f2f2f2(bc * u, ab * v) : robust_normalize_diff_f2f2f2(bcd, abc);
 203	        }
 204	        strokeCoord = w >= 0.0 ? abc / uv : abcd;
 205	    } else {
 206	        tangent = combinedEdgeID == 0.0 ? tan0 : tan1;
 207	        strokeCoord = combinedEdgeID == 0.0 ? p0 : p3;
 208	    }
 209	    vec2 ortho = vec2(tangent.y, -tangent.x);
 210	    strokeCoord += ortho * (STROKE_RADIUS * strokeOutset);
 211	    vec2 devCoord = AFFINE_MATRIX * strokeCoord + TRANSLATE;
 212	    gl_Position = vec4(devCoord, 0.0, 1.0);
 213	    gl_Position = vec4(gl_Position.xy * sk_RTAdjust.xz + gl_Position.ww * sk_RTAdjust.yw, 0.0, gl_Position.w);
 214	}
 215	
Errors:
ERROR: 0:82: Use of undeclared identifier 'gl_VertexID'
ERROR: 0:83: Use of undeclared identifier 'gl_VertexID'
ERROR: 0:84: Use of undeclared identifier 'edgeID'
ERROR: 0:84: Use of undeclared identifier 'edgeID'
ERROR: 0:88: Use of undeclared identifier 'edgeID'
ERROR: 0:89: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:102: Use of undeclared identifier 'edgeID'
ERROR: 0:103: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:107: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:111: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:112: Use of undeclared identifier 'strokeOutset'
ERROR: 0:112: Use of undeclared identifier 'strokeOutset'
ERROR: 0:112: Use of undeclared identifier 'strokeOutset'
ERROR: 0:115: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:115: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:124: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:125: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:126: Use of undeclared identifier 'strokeOutset'
ERROR: 0:128: Use of undeclared identifier 'edgeID'
ERROR: 0:130: Use of undeclared identifier 'strokeOutset'
ERROR: 0:134: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:134: Use of undeclared identifier 'isFinalEdge'
ERROR: 0:152: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:154: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:157: Use of undeclared identifier 'maxParametricEdgeID'
ERROR: 0:161: Use of undeclared identifier 'maxRotation0'
ERROR: 0:162: Use of undeclared identifier 'maxRotation'
ERROR: 0:162: Use of undeclared identifier 'maxRotation'
ERROR: 0:163: Use of undeclared identifier 'maxRotation'
ERROR: 0:169: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:172: Use of undeclared identifier 'lastRadialEdgeID'
ERROR: 0:173: Use of undeclared identifier 'radialAngle'
ERROR: 0:173: Use of undeclared identifier 'radialAngle'
ERROR: 0:188: Use of undeclared identifier 'lastRadialEdgeID'
ERROR: 0:206: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:207: Use of undeclared identifier 'combinedEdgeID'
ERROR: 0:210: Use of undeclared identifier 'strokeOutset'

I assume it is the same issue—a bad GL stack on macOS—but I wanted to note it here for completeness.

[HinTak]

I am swtching on the METAL backend on mac os x for m125: a4e875b ( on #244 ) - since I don't even own a mac, this is just blindly adding Metal code which has some Vulkan equivalent out. See if that can be used to improve things.

[HinTak]

#244 metal-enabled build finishes. I don't know how to actually use it, but essentially all the vulkan Related API (I think there are 9 code blocks) has a Metal equivalent now. Contribution of examples / tests welcomed.

@kyamagu sorry about the noise with force-push and CI failures...

[HinTak]

v87.6 is out (should land in pypi in a few hours), mainly it is a build-matrix update.

[HinTak]

The build matrix update switches the macos build host from 11 to 12. I'd be interested to know if that, and python 12, has any impact on the shader errors appearing or disappearing (on the same hardware).

[stenson]

@HinTak i haven't had any luck yet with figuring out metal on macos with the m125 version, but the new 87.6 version works great for me on python3.12, no shader errors at all, as far as I can tell it performs just as well as python3.11 + 87.5

[HinTak]

okay, thanks for letting me know. We'll probably need to look for the shader error upstream... it is a bit unfortunate that it seems to be mac os x only. I'd like to know if @pavpanchekha has it with 87.6 or not.

[pavpanchekha]

I don't get any shader errors on 87.6 with Python 3.12. I do still get that mysterious segfault I told you about, HinTak, so probably that's not Skia-related. But it does now work great, no shader errors, everything shows up.

[HinTak]

A while ago we had somebody else's code doing intermittent/mysterious segfaults, and it turns out that it was a use-after-going-out-of-scope problem: referring to out-of-scope python/skia variables and structures sometimes work, as it depends on when the python interpreter removes it (that can be a while), so sometimes works and sometimes segfault. Keeping some variables longer/global was the solution.

I am at a loss about the shader error. Some people online say it can be caused by leftovers in the GPUCache (for mesa/linux), and clearing it would do. I couldn't find out where the mac os x equivalent is. That maybe an answer, as some cached files are probably invalid if you switch between 87 to 1xx. One thing to try is perhaps set up a 2nd user on your mac: GPUCache is per user (although i don't know where it is or how to clear it on mac os; I do know where it is on Linux/mesa), so you can start with an empty cache if you set up a new user, and don't ever run skia 87 on it, start with 1xx. See if that works?

[pavpanchekha]

I can try that, but not this coming week.

[stenson]

Thanks for the idea @HinTak, unfortunately I just created a new user on my mac and installed the 124.0b7 release without ever running an 87 skia-python with that user and I got all the same shader errors as I get when switching back and forth between an 87 install and a 1xx install. I'll try to look into that angle more, though.

[HinTak]

I am wondering if I can modify these lines:

   while (glfw.get_key(window, glfw.KEY_ESCAPE) != glfw.PRESS
            and not glfw.window_should_close(window)):
            glfw.wait_events()

To something like, display the window for 10 seconds then just quit by itself. That way, I can send it into github's CI as part of post-build tests.

[stenson]

@HinTak on my computer you can actually get rid of the while loop entirely and "Shader compilation error" will still be printed to stderr directly and the program will exit, i.e. with these two files I can run python glfw_test.py and immediately get a result on whether the shader failed to compile. Does that work for CI ?

minimal_glfw.py

import contextlib, glfw, skia
from OpenGL import GL

WIDTH, HEIGHT = 640, 480

path = skia.Path()
path.moveTo(184, 445)
path.lineTo(249, 445)
path.quadTo(278, 445, 298, 438)
path.quadTo(318, 431, 331, 419)
path.quadTo(344, 406, 350, 390)
path.quadTo(356, 373, 356, 354)
path.quadTo(356, 331, 347, 312)
path.quadTo(338, 292, 320, 280) # <- comment out this line and shape will draw correctly with anti-aliasing
path.close()

@contextlib.contextmanager
def glfw_window():
    if not glfw.init():
        raise RuntimeError('glfw.init() failed')
    glfw.window_hint(glfw.STENCIL_BITS, 8)
    window = glfw.create_window(WIDTH, HEIGHT, '', None, None)
    glfw.make_context_current(window)
    yield window
    glfw.terminate()

@contextlib.contextmanager
def skia_surface(window):
    context = skia.GrDirectContext.MakeGL()
    (fb_width, fb_height) = glfw.get_framebuffer_size(window)
    backend_render_target = skia.GrBackendRenderTarget(
        fb_width,
        fb_height,
        0,  # sampleCnt
        0,  # stencilBits
        skia.GrGLFramebufferInfo(0, GL.GL_RGBA8))
    surface = skia.Surface.MakeFromBackendRenderTarget(
        context, backend_render_target, skia.kBottomLeft_GrSurfaceOrigin,
        skia.kRGBA_8888_ColorType, skia.ColorSpace.MakeSRGB())
    assert surface is not None
    yield surface
    context.abandonContext()

with glfw_window() as window:
    GL.glClear(GL.GL_COLOR_BUFFER_BIT)

    with skia_surface(window) as surface:
        with surface as canvas:
            canvas.drawCircle(100, 100, 40, skia.Paint(Color=skia.ColorGREEN, AntiAlias=True))

            paint = skia.Paint(Color=skia.ColorBLUE)
            paint.setStyle(skia.Paint.kStroke_Style)
            paint.setStrokeWidth(2)
            paint.setAntiAlias(True)
            
            canvas.drawPath(path, paint)

        surface.flushAndSubmit()
        glfw.swap_buffers(window)

glfw_test.py

import subprocess

result = subprocess.run(["python", "minimal_glfw.py"], capture_output=True)

failed = "Shader compilation error" in result.stderr.decode("utf-8")

print("failed", failed)

[HinTak]

I have put a slightly straightened version of the code into HinTak/skia-m1xx-python#6 and change it to only build for mac os. (So it will finish either way in about 20 minutes rather than 5 hours - building for aarch64 linux is very slow...). Just to confirm, the code fails with a bang, right? Instead of drawing with flaws and exit normally with piles of messages on stderr?

I just cut it out, prepend it with import skia, cut out 4 spaces from each line, run it directly on Linux against 126b8 (it just been tagged so should land public any time now). It pops out a quick window, then exits quietly and normally. In CI against m120 (just a convenient old pull I can put extra stuff in), it segfaults against 3.9 - I'll play in that pull a bit...

[stenson]

I think that's all correct — I just ran your test_ShaderError with python3.12 & skia-python==124.0b7 and I got this on stderr (the same error we've been seeing before):

Shader compilation error
------------------------
   1    #version 110
   2
   3    uniform vec4 sk_RTAdjust;
   4    uniform vec2 uatlas_adjust_S0;
   5    attribute vec4 fillBounds;
   6    attribute vec4 color;
   7    attribute vec4 locations;
   8    varying vec2 vatlasCoord_S0;
   9    varying vec4 vcolor_S0;
  10    void main() {
  11        vec2 unitCoord = vec2(float(gl_VertexID & 1), float(gl_VertexID >> 1));
  12        vec2 devCoord = mix(fillBounds.xy, fillBounds.zw, unitCoord);
  13        vec2 atlasTopLeft = vec2(abs(locations.x) - 1.0, locations.y);
  14        vec2 devTopLeft = locations.zw;
  15        bool transposed = locations.x < 0.0;
  16        vec2 atlasCoord = devCoord - devTopLeft;
  17        if (transposed) {
  18            atlasCoord = atlasCoord.yx;
  19        }
  20        atlasCoord += atlasTopLeft;
  21        vatlasCoord_S0 = atlasCoord * uatlas_adjust_S0;
  22        vcolor_S0 = color;
  23        gl_Position = vec4(devCoord, 0.0, 1.0);
  24        gl_Position = vec4(gl_Position.xy * sk_RTAdjust.xz + gl_Position.ww * sk_RTAdjust.yw, 0.0, gl_Position.w);
  25    }
  26
Errors:
ERROR: 0:11: Use of undeclared identifier 'gl_VertexID'
ERROR: 0:11: Use of undeclared identifier 'gl_VertexID'
ERROR: 0:12: Use of undeclared identifier 'unitCoord'
ERROR: 0:16: Use of undeclared identifier 'devCoord'
ERROR: 0:18: Use of undeclared identifier 'atlasCoord'
ERROR: 0:18: Use of undeclared identifier 'atlasCoord'
ERROR: 0:20: Use of undeclared identifier 'atlasCoord'
ERROR: 0:21: Use of undeclared identifier 'atlasCoord'
ERROR: 0:23: Use of undeclared identifier 'devCoord'

[HinTak]

BTW, Metal-enabled build is out:
https://github.com/kyamagu/skia-python/releases/tag/v126.0b8

On CI against m120, the code is failing on multiple places:

context = skia.GrDirectContext.MakeGL()
assert context is not None                                # assert here

(fb_width, fb_height) = glfw.get_framebuffer_size(window) # segfault against 3.9, 3.11

assert surface is not None
# strerr "GLFWError: (65545) b'NSGL: Failed to find a suitable pixel format'"

@kyamagu do you remember why there is a lineCIBW_TEST_REQUIRES_MACOS: pytest pillow pyopengl (missing glfw) vs CIBW_TEST_REQUIRES: pytest pillow glfw in the CI yml?

[kyamagu]

@HinTak I remember that for some reason, pyopengl works, but glfw doesn't on the CI. Didn't look in depth at that time.

[HinTak]

Hi, @stenson and @pavpanchekha : Can I have both of you inserting this snipplet just after the context = skia.GrDirectContext.MakeGL() line (this snipplet needs a valid GL context, so it needs to be insert a bit after the windows creation, etc), for both under m87 and under current (m1xx)? ie. I want 2 x 4 lines, from both of you.

from OpenGL.GL import glGetString, GL_VENDOR, GL_RENDERER, GL_VERSION, GL_SHADING_LANGUAGE_VERSION
print(glGetString(GL_VENDOR))
print(glGetString(GL_RENDERER))
print(glGetString(GL_VERSION))
print(glGetString(GL_SHADING_LANGUAGE_VERSION))

Mine reports now:

b'AMD'
b'AMD Radeon R5 Graphics (radeonsi, stoney, LLVM 18.1.1, DRM 3.57, 6.8.10-300.fc40.x86_64)'
b'4.5 (Compatibility Profile) Mesa 24.0.8'
b'4.50'

(was 6.8.9-300.fc40.x86_64 and 24.0.7 before the most recent reboot)

Github CI reports (when one gets a GL context - see further below):

 # mac os x 12 
 "Apple Inc."
 "Apple Software Renderer"
 "2.1 APPLE-19.5.1"
 "1.20"

# mac os x 13
 "Apple Inc."
 "Apple Software Renderer"
 "2.1 APPLE-20.5.2"
 "1.20"

# mac os x 14
 "Apple Inc."
 "Apple Software Renderer"
 "2.1 APPLE-21.0.19"
 "1.20"

@kyamagu I think you are right, pyopengl (its GLUT part) works, but glfw does not work in CI.

[HinTak]

Hi, @stenson and @pavpanchekha - I also checked upstream m87 and m116 diff - they added a whole lot of conditionals based on those 4 answers (which is specific to your hard/ware software combination - mine is simple and quite generic - "Mesa" is basically the generic software render on linux; but if I had a NVidia graphic card and using their official graphics driver I would go down the non-Mesa code path).

According to https://www.glfw.org/faq#macos , glfw should have these 4 lines (<-). If your report some very low/wrong values, can you see if these improve the situation?

    if not glfw.init():
        raise RuntimeError('glfw.init() failed')
    glfw.window_hint(glfw.VISIBLE, glfw.FALSE)
    glfw.window_hint(glfw.STENCIL_BITS, 8)
    # see https://www.glfw.org/faq#macos
    glfw.window_hint(glfw.CONTEXT_VERSION_MAJOR, 3)                    #    <- 
    glfw.window_hint(glfw.CONTEXT_VERSION_MINOR, 2)                     #  <-
    glfw.window_hint(glfw.OPENGL_FORWARD_COMPAT, True)                     #  <-
    glfw.window_hint(glfw.OPENGL_PROFILE, glfw.OPENGL_CORE_PROFILE)    #   <-
    context = glfw.create_window(640, 480, '', None, None)

glfw simply doesn't work on github CI, with or without these 4 lines, but pyopengl (it GLUT part) does.

[stenson]

@HinTak thanks for looking into that! I'm seeing this on my setup:

b'Apple'
b'Apple M3 Pro'
b'2.1 Metal - 88'
b'1.20'

And the four additional lines mentioned do seem to completely fix the issue both for this test as well as for the coldtype library! Apologies that it hadn't occurred to me that glfw config might be the issue here.

Assuming this works for other macOS setups, would it make sense for those lines to be added to the glfw examples in this repo?

[pavpanchekha]

Hi @HinTak, here's what I get:

b'Apple'
b'Apple M1'
b'2.1 Metal - 88.1'
b'1.20'

I'm not using glfw, I'm using SDL2. Should I try to use glfw instead and try those four lines?

[HinTak]

@pavpanchekha I happened to have seen the SDL2 equivalent for those 4 lines while I was reading up on it. According to https://stackoverflow.com/questions/20931528/shader-cant-be-compiled, 3 of them are:

SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 3);
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 2); 
SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_CORE);

This is in C code, so something similar should be possible with python SDL. Could you try to look it up and post in here?

[HinTak]

@stenson thanks for confirming that. I am almost certain that the difference you see is due to changes in this file , e.g. https://github.com/google/skia/blob/229d94a8807ea5d2e3d5ecef898a7a1146ca8840/src/gpu/ganesh/gl/GrGLCaps.cpp#L4695 Between m87 and m116, google folks added a lot of "if Apple / if metal" in the file similar to this line. I am glad we get to the bottom of it now. Yes, we should add those 4 lines to our documentation / code. As I have some exchange with @kyamagu above, actually mac os x testing inside CI has never worked (even in m87 days) and so the examples have always been largely Linux based... as it turns out, linux/mesa and Apple's implementation of opengl differs a bit, and upstream skia actively exploits those differences.

I am trying to figure out why glfw testing for mac os x never worked:
FlorianRhiem/pyGLFW#80

[HinTak]

@pavpanchekha found the SDL python equivalent in https://gist.github.com/hurricanerix/3be8221128d943ae2827 :

sdl2.SDL_GL_SetAttribute(sdl2.SDL_GL_CONTEXT_MAJOR_VERSION, 3)
sdl2.SDL_GL_SetAttribute(sdl2.SDL_GL_CONTEXT_MINOR_VERSION, 2)
sdl2.SDL_GL_SetAttribute(sdl2.SDL_GL_CONTEXT_FORWARD_COMPATIBLE_FLAG, 1)
sdl2.SDL_GL_SetAttribute(sdl2.SDL_GL_CONTEXT_PROFILE_MASK, sdl2.SDL_GL_CONTEXT_PROFILE_CORE)

edit: added the 3rd line as best I can see.

[pavpanchekha]

Ok, I tried this:

            sdl2.SDL_GL_SetAttribute(sdl2.SDL_GL_CONTEXT_MAJOR_VERSION, 3)
            sdl2.SDL_GL_SetAttribute(sdl2.SDL_GL_CONTEXT_MINOR_VERSION, 2)
            sdl2.SDL_GL_SetAttribute(sdl2.SDL_GL_CONTEXT_FORWARD_COMPATIBLE_FLAG, True)
            sdl2.SDL_GL_SetAttribute(sdl2.SDL_GL_CONTEXT_PROFILE_MASK,
                                     sdl2.SDL_GL_CONTEXT_PROFILE_CORE)

It worked: no shader compiler errors. I'll add it to our porting page.

[HinTak]

@pavpanchekha thanks for the confirmation!

@stenson you mentioned that you saw some slow down with m1xx compared to m87. Do you still see it now?

@pavpanchekha @stenson I'll update README.m116, the examples/tutorials, and the CI (this last one still doesn't work, but might as well prepare for when it does) soon. Think of anywhere else I should insert a note/snipplet of code in? Currently I have examples for glfw, sdl2 and glut settings.

The way I understand this issue now, is that, between m87 and m116, Google folks added Apple-specific hardware/software detection code in skia, to optimze performance and/or work around graphics hardware/software bugs. So it is now necessary to request a matching OpenGL profile expected from the new code paths for best behavior from Apple hardware. In m87, it is likely that Apple users were getting some kind of generic non-Mesa/non-Nvidia behaviour. So @stenson it might be interesting to know if you see any improvements in speed over m87 now.

[HinTak]

This development in skia in the last few years is probably driven by Apple switching to Arm hardware on the desktop - I see on github CI the mac os x 14 runner is arm-based, and is about 1/2 the speed of of the Intel based 12/13 runners in building skia. (So optimization becomes important).