-
Notifications
You must be signed in to change notification settings - Fork 42
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
Anti-Alias with complex skia.Path triggers shader compilation error and failed drawing #214
Comments
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. |
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 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). |
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. |
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. |
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? |
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. |
@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: If you have anything else obvious and simple enough to put into m120, let me know soon. |
@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. |
@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. |
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. |
I seem to have a similar error in a case with
The compilation error is:
I assume it is the same issue—a bad GL stack on macOS—but I wanted to note it here for completeness. |
v87.6 is out (should land in pypi in a few hours), mainly it is a build-matrix update. |
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). |
@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 |
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. |
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. |
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? |
I can try that, but not this coming week. |
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. |
I am wondering if I can modify these lines:
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. |
@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
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)
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) |
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 |
I think that's all correct — I just ran your
|
BTW, Metal-enabled build is out: On CI against m120, the code is failing on multiple places:
@kyamagu do you remember why there is a line |
@HinTak I remember that for some reason, |
Hi, @stenson and @pavpanchekha : Can I have both of you inserting this snipplet just after the
Mine reports now:
(was Github CI reports (when one gets a GL context - see further below):
@kyamagu I think you are right, pyopengl (its GLUT part) works, but glfw does not work in CI. |
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 (
glfw simply doesn't work on github CI, with or without these 4 lines, but pyopengl (it GLUT part) does. |
@HinTak thanks for looking into that! I'm seeing this on my setup:
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? |
Hi @HinTak, here's what I get:
I'm not using |
@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:
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? |
@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: |
@pavpanchekha found the SDL python equivalent in https://gist.github.com/hurricanerix/3be8221128d943ae2827 :
edit: added the 3rd line as best I can see. |
Ok, I tried this:
It worked: no shader compiler errors. I'll add it to our porting page. |
@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. |
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). |
Thanks for all your help, HinTak. I don't see anything else you might need to add. |
All sounds great to me. I don't have any hard stats on performance, but it does seem equal if not better just from looking at some informal numbers from my library. |
When #249 get merged, this should automatically close. No need to do it by hand. The glfw CI testing issue will continue in FlorianRhiem/pyGLFW#80 . There is an update of Google's own Skia & SDL c++ example to m126 in https://github.com/HinTak/skia-c-examples/ (it was removed in m93 when they want to get rid of all SDL dependency within Skia's repo), and https://github.com/HinTak/skia-python-examples/ contains three versions of this python example code, one for each of glfw, OSX glut, and pysdl2, as well as a port of the google SDL c++ example to skia-python & PySDL2 . @pavpanchekha I think you might be interested to read it - there is something clever from Google using skia to render directly to SDL's window. @pavpanchekha @stenson there is a paypal link somewhere on my profile page, if you feel like buying me a few cups of coffee :-). |
The glfw issue has gotten further upstream: glfw/glfw#2570 |
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):
On my setup, running that code produces this output:
Expected behavior
![image](https://private-user-images.githubusercontent.com/89770/285677718-7bc555a6-b0ec-40ee-b1bf-815b45b4a46c.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjQ4MDkwNTgsIm5iZiI6MTcyNDgwODc1OCwicGF0aCI6Ii84OTc3MC8yODU2Nzc3MTgtN2JjNTU1YTYtYjBlYy00MGVlLWIxYmYtODE1YjQ1YjRhNDZjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA4MjglMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwODI4VDAxMzIzOFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTRmMjYxZjU1NjgxNGY2NjE4YWY3NjJlMTMzMzJjY2JmZDc0NDY2ZWIwMjRhYTZkM2I2NDY3YTk1NTE0OWZmNWMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.O_v9siqnwkhcTptqO61qVCX9AuYoaWoY1pGjvu6b0V0)
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):
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!
The text was updated successfully, but these errors were encountered: