JuliaGL / GLFW.jl

Julia interface to GLFW, a multi-platform library for creating windows with OpenGL contexts and managing input and events.
http://www.glfw.org/
MIT License
138 stars 32 forks source link

make window mutable #150

Closed SimonDanisch closed 6 years ago

SimonDanisch commented 6 years ago

After debugging some issue, I figured it came down to me not correctly identifying closed/destroyed contexts. It is basically impossible to detect them if we have the window immutable, since OpenGL is free to reuse context pointers, so you can't possibly tell by just the pointer if it's a valid context. So I'd need to wrap GLFW.Window into MyMutableWindow, and then reimplement all GLFW functions on it, if I want to propperly check for valid contexts in all my types I pass the window to.

jayschwa commented 6 years ago

I have some thoughts on how we can make use of closed windows raise an exception rather than crash, but I don't have time to go into detail at the moment (since I'm at work). Will that solve the problem you're having?

SimonDanisch commented 6 years ago

No... because OpenGL uses hidden global state - I'm not literally using the window. I'm just binding textures etc, if I can't check if the window has been closed/reused etc, I can't figure out if I'm allowed to bind a texture from window X. Right now the situation is, that I might have window X and window Y, and I created textures within the context of X and Y. X and Y may have the same window pointer so, since they're immutable it's X === Y. But X might have been closed so all textures created under X are invalid now - and I have no way to find this out.

SimonDanisch commented 6 years ago

Just to illustrate what I'm fighting with here:


const current_ctx = Ref{Window}()
iscurrent(x) = current_ctx[] === x

mutable struct Texture
    id::GLuint
    context::Window
    function Texture(id)
        obj = new(id, current_ctx[])
        finalize(obj) do obj
            iscurrent(obj.context) && free(obj)
        end
        obj
    end
end

w1 = Window()
current_ctx[] = w1
tex1 = Texture(...)
destroy!(w1)
w2 = Window()
current_ctx[] = w2
w1 === w2 # opengl sucks! so this happens all the time ;)
tex2 = Texture(...)
tex1.id == tex2.id # yeah same for the ids of texture - they'll restart at 1 after destroyed context
tex1 = nothing; gc()
# now finalizer of tex1 will get called, will check if the context is current (which it looks like it is)
# and go ahead and happily free tex2
jayschwa commented 6 years ago

It makes sense after seeing that example. Feel free to merge if the examples still run fine. If you tag a release, please keep it an alpha.

My idea for raising exceptions rather than crashing requires the Window object to be mutable again anyway, so I'll follow up with that in a later PR.

SimonDanisch commented 6 years ago

I'll not call it alpha in the tag name, since Pkg3 can't deal with that and from what I understood never will... Hope that's okay!