sensics / OSVR-RenderManager

Apache License 2.0
63 stars 45 forks source link

61a5ade breaks OpenGL core profile on mesa #231

Closed ChristophHaag closed 7 years ago

ChristophHaag commented 7 years ago

Specifically, this line causes an error to be raised in mesa: https://github.com/sensics/OSVR-RenderManager/commit/61a5ade12315dd9ca0b25a7f69273700264f907b#diff-c5e409b6f1a3f869f4cdd104644d039eR1282

#0  _mesa_error (ctx=0x555555a68e00, error=1282, fmtString=0x7fffefc29645 "%s(no array object bound)") at main/errors.c:274
#1  0x00007fffef5c561c in update_array (ctx=0x555555a68e00, func=0x7fffefc29798 "glVertexAttribPointer", attrib=17, legalTypesMask=32766, sizeMin=1, sizeMax=5, size=4, type=5126,
    stride=40, normalized=0 '\000', integer=0 '\000', doubles=0 '\000', ptr=0x0) at main/varray.c:483
#2  0x00007fffef5c61b4 in _mesa_VertexAttribPointer (index=0, size=4, type=5126, normalized=0 '\000', stride=40, ptr=0x0) at main/varray.c:733
#3  0x00007ffff786d6a4 in osvr::renderkit::RenderManagerOpenGL::UpdateDistortionMeshesInternal (this=0x5555557c4100, type=osvr::renderkit::SQUARE,
    distort=std::vector of length 2, capacity 2 = {...}) at /home/chris/build/osvr-rendermanager-debug/src/osvr-rendermanager/osvr/RenderKit/RenderManagerOpenGL.cpp:971
#4  0x00007ffff786c0c3 in osvr::renderkit::RenderManagerOpenGL::OpenDisplay (this=0x5555557c4100)
    at /home/chris/build/osvr-rendermanager-debug/src/osvr-rendermanager/osvr/RenderKit/RenderManagerOpenGL.cpp:730
#5  0x000055555555b40a in main (argc=1, argv=0x7fffffffdb68) at /home/chris/build/osvr-rendermanager-debug/src/osvr-rendermanager/examples/RenderManagerOpenGLCoreExample.cpp:547

The lines in RenderManagerOpenGL.cpp are off by two because to use the core profile I removed the #ifdef here: https://github.com/sensics/OSVR-RenderManager/blob/bfc88b75bc5f01388703bbd73c90251d8ba57f42/osvr/RenderKit/RenderManagerOpenGL.cpp#L153-L158 (see https://github.com/sensics/OSVR-RenderManager/issues/68).

mesa allows to fake a compatibility profle (without really supporting it) with the environment variables MESA_GL_VERSION_OVERRIDE=3.3COMPAT MESA_GLSL_VERSION_OVERRIDE=330. When they are set, RenderManagerOpenGLCoreExample works. When they are not set, the above error happens.

I'm curious: Does it still work on Mac OS X where only the core profile is supported too? Does it work on other drivers on other operating systems when the #ifdef is removed?

ChristophHaag commented 7 years ago

OpenGL spec says

Client vertex and index arrays - all vertex array attribute and element array index pointers must refer to buffer objects. The default vertex array object (the name zero) is also deprecated. Calling VertexAttribPointer when no buffer object or no vertex array object is bound will generate an INVALID_- OPERATION error, as will calling any array drawing command when no vertex array object is bound.

so it works with the compatibility profile, but has been deprecated so it doesn't work with core.

ChristophHaag commented 7 years ago

Looks like OpenGL isn't so hard after all, just creating a vertex array object and binding it makes everything work without errors. Intentionally badly formatted patch because I have no idea what I'm doing:

diff --git a/osvr/RenderKit/RenderManagerOpenGL.cpp b/osvr/RenderKit/RenderManagerOpenGL.cpp
index 84c3d9c..05bac14 100755
--- a/osvr/RenderKit/RenderManagerOpenGL.cpp
+++ b/osvr/RenderKit/RenderManagerOpenGL.cpp
@@ -150,12 +150,10 @@ public:
     SDL_GL_SetAttribute(SDL_GL_ALPHA_SIZE, p->bitsPerPixel);
     SDL_GL_SetAttribute(SDL_GL_DEPTH_SIZE, 24);
     SDL_GL_SetAttribute(SDL_GL_ACCELERATED_VISUAL, 1);
-#ifdef __APPLE__
     SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 3);
     SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 3);
     SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK,
       SDL_GL_CONTEXT_PROFILE_CORE);
-#endif

     // If we have multiple displays, we need to re-use the
     // same context between them.  In fact, we always want
@@ -967,7 +965,9 @@ namespace renderkit {
             glBufferData(GL_ARRAY_BUFFER,
                 sizeof(DistortionVertex) * meshBuffer.vertices.size(),
                 &meshBuffer.vertices[0], GL_STATIC_DRAW);
-
+GLuint array;
+glGenVertexArrays(1, &array);
+glBindVertexArray(array);
             size_t const stride = sizeof(DistortionVertex);
             glVertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, stride,
                 (void*)offsetof(DistortionVertex,pos));
@@ -1276,7 +1276,9 @@ namespace renderkit {
         }

         auto const & meshBuffer = m_distortionMeshBuffer[params.m_index];
-
+GLuint array;
+glGenVertexArrays(1, &array);
+glBindVertexArray(array);
         glBindBuffer(GL_ARRAY_BUFFER, meshBuffer.vertexBuffer);
         size_t const stride = sizeof(DistortionVertex);
         glVertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, stride,

With that patch RenderManagerOpenGLCoreExample runs with the rendermanager using only the core profile.

Edit: And after looking at the commit that caused this, creating VAOs was exactly what the original commit tried to avoid, right?

russell-taylor commented 7 years ago

Right, because it breaks backwards compatibility with GLES 2.0. Whoever is in charge of OpenGL is really making my life difficult...

russell-taylor commented 7 years ago

This should be fixed in 2d02144f7e6216ba381d16984ccc7228733528d3