| Summary: | glX doesn't need a temp context to build a GL3 context. | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Ryan C. Gordon <icculus> |
| Component: | video | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | gabomdq, matthias.bentrup |
| Version: | HG 2.0 | ||
| Hardware: | x86 | ||
| OS: | Other | ||
(Sorry if you get a lot of copies of this email, we're touching dozens of bug reports right now.) Tagging a bunch of bugs as target-2.0.0, Priority 2. This means we're in the final stretch for an official SDL 2.0.0 release! These are the bugs we really want to fix before shipping if humanly possible. That being said, we don't promise to fix them because of this tag, we just want to make sure we don't forget to deal with them before we bless a final 2.0.0 release, and generally be organized about what we're aiming to ship. Hopefully you'll hear more about this bug soon. If you have more information (including "this got fixed at some point, nevermind"), we would love to have you come add more information to the bug report when you have a moment. Thanks! --ryan. Taking this one. --ryan. Oh, it turns out we return the (otherwise temporary) pre-3.x context if we fail to make the 3.x context. The Windows code is doing this too. The Cocoa code returns NULL in this case. I tend to think returning a < 3 context is a bug if the user asked for >= 3. There are a lot of assumptions the app is making about OpenGL if they ask for a specific version, including removal of cruft from the API, and more importantly, functions and features that are expected to be there. I'm tossing this back to Sam to consider. If you want to punt on this for 2.0.0, just remove the target-2.0.0 tag for now. --ryan. Eh, screw it, I'm yanking the target-2.0.0 tag. Look at this later. --ryan. Yeah, returning a < 3 context is a bug if the user asked for >= 3. I'm not sure how that relates to the original bug report, but I agree. Tossing back to you, this is fine to fix now or later, your call. I'd lean toward sooner if leaving it there for 2.0 means some poor sod may suddenly have their game break when we fix it This probably applies to X11_GL_InitExtensions as well, I commented out the context creation stuff there and the extension query seemed to work fine. There is another problem in the context creation code: you assume that glXCreateContextAttribsARB is supported if glXgetProcAddressARB returns a non-NULL value, but the specs allow to (and some drivers do) return a dummy function instead of NULL. The only reliable way is to check for the presence of the GLX_ARB_create_context_profile extension. (In reply to Sam Lantinga from comment #5) > Yeah, returning a < 3 context is a bug if the user asked for >= 3. I'm not > sure how that relates to the original bug report, but I agree. Just that it was our existing behavior. No relation otherwise. But the existing behavior was basically going to cause the game to crash or act strangely if the GL wasn't up to the task, so I decided the change was okay. > Tossing back to you, this is fine to fix now or later, your call. I'd lean > toward sooner if leaving it there for 2.0 means some poor sod may suddenly > have their game break when we fix it Fixed this in https://hg.libsdl.org/SDL/rev/26eb20aad02f . (In reply to Matthias Bentrup from comment #7) > There is another problem in the context creation code: you assume that > glXCreateContextAttribsARB is supported if glXgetProcAddressARB returns a > non-NULL value, but the specs allow to (and some drivers do) return a dummy > function instead of NULL. This was fixed, too. (In reply to Gabriel Jacobo from comment #6) > This probably applies to X11_GL_InitExtensions as well, I commented out the > context creation stuff there and the extension query seemed to work fine. Removed that code too: https://hg.libsdl.org/SDL/rev/0d1e81f10b7e --ryan. |
(I think.) In X11_GL_CreateContext(), we create a legacy context, then call glXGetProcAddress("glXCreateContextAttribsARB"), throw that context away and build the new context. This isn't necessary. It was probably built this way because WGL requires it, because wglGetProcAddress() needs a current context. glXGetProcAddress(), however, has to provide a valid function pointer even if there is no context at all and it's up to the app to make sure it's allowed to call it for a given context. Details are here: http://dri.freedesktop.org/wiki/glXGetProcAddressNeverReturnsNULL So we can probably delete this code and, if glXCreateContextAttribsARB exists, just create the 3.x context. --ryan.