Potential fixes for GtkMakieWidget instabillity

[Joroks]

I took the time to really dig into the the cause for the instabillities with GtkMakieWidget, as using 'make_current' did not allways work for me.
I especially had the issue that some Blocks randomly dissapeared or rendered with missing elements.

I think the main cause for the crashes and general weird behaviour has to do with how 'ShaderAbstractions' handles the openGL context. The variable ShaderAbstractions.ACTIVE_OPENGL_CONTEXT keeps track of the openGL context manually and therefore becomes out of sync with the acutal context, when a context switch is triggered by Gtk. I've found two possible solutions to this problem. One is more intrusive and involves pirating functions from ShaderAbstractions but is generally more robust and allows for Gtk windows and GLMakie windows to be opened at the same time. The other is simpler and less intrusive but causes GLMakie windows to break when a Gtk window is open as well.

The robust but with piracy solution:

function ShaderAbstractions.current_context()
    isassigned(ShaderAbstractions.ACTIVE_OPENGL_CONTEXT) || error("No active context")
    ctx = ShaderAbstractions.ACTIVE_OPENGL_CONTEXT[]
    ctx === nothing && error("No active context")
    gtk_ctx = Gtk4.G_.get_current()
    if !isnothing(gtk_ctx)
        isa(ctx, GtkGLArea) || error("No active context")
        gtk_ctx == Gtk4.G_.get_context(ctx) || error("No active context")
    end
    return ctx
end

function ShaderAbstractions.is_current_context(x)
    ShaderAbstractions.ACTIVE_OPENGL_CONTEXT[] == x || return false
    gtk_ctx = Gtk4.G_.get_current()
    if !isnothing(gtk_ctx)
        isa(x, GtkGLArea) || return false
        return Gtk4.G_.get_context(x) == gtk_ctx
    end
    return true
end

The less intrusive but also less robust solution:

function ShaderAbstractions.switch_context!(a::GtkGLMakie)
    Gtk4.G_.get_realized(a) || return
    ShaderAbstractions.ACTIVE_OPENGL_CONTEXT[] == x
    Gtk4.make_current(a)
end

instead of defining ShaderAbstractions.native_switch_context!(a::GtkGLMakie)

With both of these solutions, manually calling make_current is no longer neccesary. It's probably possible to remove
all the other calls to make_current outside of ShaderAbstractions.native_switch_context!(a::GtkGLMakie) within this package as well.
I haven't set up a PR yet, as I've been experimentig with this im my own project, but I could do so tomorrow.

[jwahlstrand]

Thanks so much for digging into this. I'll play with your solutions this weekend. BTW I just submitted a PR to get things working with GLMakie 0.11.

A PR would be really welcome, and if the non-piratical way to fix this is to submit a PR to GLMakie or ShaderAbstractions, I think that should be pursued. Early on I did have to submit one or two's PR's to GLMakie to get this package working.

[jwahlstrand]

It looks like overriding is_current_context for GtkGLMakie is an alternative to overriding switch_context!:

function ShaderAbstractions.is_current_context(a::GtkGLMakie)
    Gtk4.G_.get_realized(a) || return false
    a == ShaderAbstractions.ACTIVE_OPENGL_CONTEXT[] || return false
    curr = Gtk4.G_.get_current()
    return curr !== nothing && curr == Gtk4.G_.get_context(a)
end

This removes the need for make_current in the "widgets.jl" example. Does this work for the additional issues you mentioned?

Maybe defining another is_current_context method for a GLFW window, perhaps that checked the output of GLFW.GetCurrentContext() would be an alternative to pirating is_current_context for all types? Not sure what to do about current_context yet.

I'm wondering if it's time to remove the window-based screen in version 0.4. It was always an awkward solution but until recently it was the only type of screen that worked well. It is still half-broken in the GLMakie 0.11 port (#22).

This is a painful breaking change but I think it will drastically reduce the effort required to keep this package working. I'll look at doing that and try to merge #22 today or tomorrow, then let's figure out the best way to add this. Never mind, fixing the window was easier than I thought -- it stays for now.

[Joroks]

Im currently looking into when a context switch is triggered. I think the cleanest solution would be to somehow get Makie to only write to the openGL context in the on_render callback. According to the Gtk documentation Gtk sets the correct context for us before calling the cb.

What I've noticed is that currently, both Makies own renderloop and the Gtk render callback are enabled. Wouldn't it make more sense to only use one of them?

[Joroks]

If we overload is_current_context for each type thats used with ShaderAbstractions in a similar way to what you're suggesting, then current_context could look something like this:

function current_context()
    isassigned(ACTIVE_OPENGL_CONTEXT) || error("No active context")
    ctx = ACTIVE_OPENGL_CONTEXT[]
    ctx === nothing && error("No active context")
    is_current_context(ctx) || error("No active context")
    return ctx
end

it would probably also be good to set ACTIVE_OPENGL_CONTEXT to nothing if we've already confirmed that the actual current context has already changed.

[jwahlstrand]

What I've noticed is that currently, both Makies own renderloop and the Gtk render callback are enabled. Wouldn't it make more sense to only use one of them?

I just checked and GLMakie's start_renderloop! is currently never called. Is there other machinery that needs to be disabled?

[jwahlstrand]

it would probably also be good to set ACTIVE_OPENGL_CONTEXT to nothing if we've already confirmed that the actual current context has already changed.

I don't see a way of getting GTK to tell us when it's changed context. Right now the only time we know it's changed is when we ask if a particular context is current, or switch to one. I should look at where current_context() is called.

[jwahlstrand]

I'm slow, you're suggesting setting ACTIVE_OPENGL_CONTEXT to nothing in is_current_context when we notice it's changed... I'm still not sure if we have to worry about GTK changing it on us.

[Joroks]

I just checked and GLMakie's start_renderloop! is currently never called. Is there other machinery that needs to be disabled?

You are correct. I just assumed that because screen.stop_renderloop is set to false (same goes for screen.config.pause_renderloop) that there would be a Makie renderloop running for the screen. But because the screen get's set up manually in Gtk4Makie this doesn't happen.

I don't see a way of getting GTK to tell us when it's changed context. Right now the only time we know it's changed is when we ask if a particular context is current, or switch to one. I should look at where current_context() is called.

Yeah I don't know of any way either, but at least in current_context where we query what the current context is either from Gtk or GLFW, we can get a different context than what is stored in ACTIVE_OPENGL_CONTEXT because something else has changed that context. Here we know for sure that ACTIVE_OPENGL_CONTEXT is not in sync anymore and it would make sense to set it to nothing in order to invalidate it.

Maybe in the end the solution would be to introduce native_is_current_context into ShaderAbstractions:

### in ShaderAbstractions
function current_context()
    isassigned(ACTIVE_OPENGL_CONTEXT) || error("No active context")
    ctx = ACTIVE_OPENGL_CONTEXT[]
    ctx === nothing && error("No active context")
    if !native_is_current_context(ctx)
        ACTIVE_OPENGL_CONTEXT[] = nothing
        error("The current context has been changed without notifying ShaderAbstractions")
    end
    return ctx
end

function is_current_context(x)
    ACTIVE_OPENGL_CONTEXT[] == x && native_is_current_context(x)
end

function native_is_current_context(x)
    return true # default to true to avoid breaking libraries that don't overload this method
end

### in Gtk4Makie
function ShaderAbstractions.native_is_current_context(x::GtkGLMakie)
    curr = Gtk4.G_.get_current()
    curr === nothing && return false
    Gtk4.G_.get_context(x) == curr
end

### somewhere else for GLFW
function ShaderAbstractions.native_is_current_context(x::GLFW.Window)
    curr = GLFW.GetCurrentContext()
    curr == GLFW.Window(C_NULL) && return false
    x == curr
end

To me that seems like a fairly rubust and generally aplicable solution

[Joroks]

However, I still think Makie must be writing to openGL outside of the Gtk4.on_render or rather the GL_Makie.render_frame callback. I don't know how Makie would be writing to the wrong context otherwise as Gtk is supposed to make sure to set the correct context for us before triggering the callback. Unless there are other reasons as well as to why julia crashes except for the openGL context. But because my solutions seem to fix the problems, I don't think that's the case.

[jwahlstrand]

However, I still think Makie must be writing to openGL outside of the Gtk4.on_render or rather the GL_Makie.render_frame callback. I don't know how Makie would be writing to the wrong context otherwise as Gtk is supposed to make sure to set the correct context for us before triggering the callback. Unless there are other reasons as well as to why julia crashes except for the openGL context. But because my solutions seem to fix the problems, I don't think that's the case.

Yeah e.g. when you add a scatter plot to an axis it must be using the current context to do something before requires_update returns true, which is what triggers the render callback.

Do you want to do a PR that fixes the "local" issues (i.e. preventing the need for make_current but not fixing interference with GLMakie windows) that could be included in a new release? Does the is_current_context override I suggested work as well as your original solution? You mentioned Blocks disappearing, etc.

Getting a fix into ShaderAbstractions may take more time and will involve further discussion of the right way to do this.

[Joroks]

Do you want to do a PR that fixes the "local" issues (i.e. preventing the need for make_current but not fixing interference with GLMakie windows) that could be included in a new release? Does the is_current_context override I suggested work as well as your original solution? You mentioned Blocks disappearing, etc.

I actually don't have access to the computer where the project is on right now because I'm somewhere else atm. I'll play around with the solution a bit to maybe reproduce the issues I had and will let you know once I'm confident wheter it works.

[jwahlstrand]

Sounds good

[Joroks]

I wasn't able to reproduce any of of the Blocks disapearing or rendering incompletely with your fix.
The only issue I still have is that the Toggle block doesn't work correctly. However it doesn't work with my proposed fixes either so that's a problem we can solve in the future (I'm currently using a Slider(range=false:true) as a workaround). I'll set up the PR