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

GrDirectContext deallocation causes python interpreter to freeze #187

Open
sven-nobs opened this issue Jan 23, 2023 · 4 comments
Open

GrDirectContext deallocation causes python interpreter to freeze #187

sven-nobs opened this issue Jan 23, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@sven-nobs
Copy link

When using the GL backend, the python interpreter seems to crash/freeze once the GrDirectContext is deallocated. I noticed this happening in my own project, but it even happens in the GL backend example from the skia-python documentation. Therefore it doesn’t seem to be a problem on my end.

Running the sample from the command line causes the freeze. The Jupyter Notebook of this repo may have the problem too, but simply doesn’t cause the entire notebook to freeze. Would be interesting to see if this problem can be reproduced by others.

To Reproduce
Take the following sample from the docs, save it as a .py file, then run from the command line.

# Source: https://kyamagu.github.io/skia-python/tutorial/canvas.html
import skia
import glfw
import contextlib

@contextlib.contextmanager
def glfw_context():
    if not glfw.init():
        raise RuntimeError('glfw.init() failed')
    glfw.window_hint(glfw.VISIBLE, glfw.FALSE)
    glfw.window_hint(glfw.STENCIL_BITS, 8)
    window = glfw.create_window(640, 480, '', None, None)
    glfw.make_context_current(window)
    yield window
    glfw.terminate()

width, height = 200, 200
with glfw_context():
    context = skia.GrDirectContext.MakeGL()
    info = skia.ImageInfo.MakeN32Premul(width, height)
    surface = skia.Surface.MakeRenderTarget(context, skia.Budgeted.kNo, info)
    assert surface is not None

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

    image = surface.makeImageSnapshot()
    assert image is not None
    image.save('output.png', skia.kPNG)

It draws and saves the circle properly, but then freezes the interpreter at the end of execution.

Additional context
I used a C application with an embedded python interpreter, then paused the debugger to see what happens. It seems to be repeatedly decreasing reference counts of some object. Odd, because I'd think this would cause a crash rather than a freeze. In case this problem is caused by glfw, I tried the same example with a context created using moderngl instead. The problem persists.

Python version is 3.10.
Library version of python-skia is 87.5, installed through pip.

Workaround / Temporary Solution
I tried increasing the reference count of the context using its ref() function:

surface.recordingContext().ref()

And the problem went away. To ensure this isn't leaking the context, I double checked in C. The reference count reaches 0 now and the context is still deallocated properly.

This makes me even more certain there is something fishy going on with the GrDirectContext refcount. Manually increasing reference counts doesn’t seem like intended behaviour.

@kyamagu kyamagu added the bug Something isn't working label Jan 24, 2023
@HinTak
Copy link
Collaborator

HinTak commented Aug 2, 2023

In m88, GrContext is gone - all its methods got merged into GrDirectContext. In my m116 fork I chose to keep the shorter name, and also add an alias so using either with any of their combined methods work. So there is probably some truth in your assessment, but the problem is upstream - and Google folks make it go away in m88.

@HinTak
Copy link
Collaborator

HinTak commented Aug 19, 2023

Not sure what's going on, with m117, it does not like the last line image.save('output.png', skia.kPNG). If I comment it out the example finishes running. image.save() itself works elsewhere, so this appears to be a GPU-backed image/surface problem.

@HinTak
Copy link
Collaborator

HinTak commented Aug 19, 2023

Simply get RuntimeError: Failed to encode an image., which is not helpful.

@HinTak
Copy link
Collaborator

HinTak commented Nov 15, 2023

I think I fixed the image save problem with m118/m119, so the example runs quickly and correctly with m120. The image.save fix is
0273505

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants