Merge pull request #2002 from ReinUsesLisp/dsa-vao-buffer

gl_rasterizer: Use DSA for VAOs and buffers

src/video_core/renderer_opengl/gl_buffer_cache.cpp

@@ -14,7 +14,7 @@
 namespace OpenGL {
 
 OGLBufferCache::OGLBufferCache(RasterizerOpenGL& rasterizer, std::size_t size)
-    : RasterizerCache{rasterizer}, stream_buffer(GL_ARRAY_BUFFER, size) {}
+    : RasterizerCache{rasterizer}, stream_buffer(size, true) {}
 
 GLintptr OGLBufferCache::UploadMemory(Tegra::GPUVAddr gpu_addr, std::size_t size,
                                       std::size_t alignment, bool cache) {

src/video_core/renderer_opengl/gl_rasterizer.cpp

@@ -135,27 +135,31 @@ void RasterizerOpenGL::CheckExtensions() {
     }
 }
 
-void RasterizerOpenGL::SetupVertexFormat() {
+GLuint RasterizerOpenGL::SetupVertexFormat() {
     auto& gpu = Core::System::GetInstance().GPU().Maxwell3D();
     const auto& regs = gpu.regs;
 
-    if (!gpu.dirty_flags.vertex_attrib_format)
+    if (!gpu.dirty_flags.vertex_attrib_format) {
-        return;
+        return state.draw.vertex_array;
+    }
     gpu.dirty_flags.vertex_attrib_format = false;
 
     MICROPROFILE_SCOPE(OpenGL_VAO);
 
     auto [iter, is_cache_miss] = vertex_array_cache.try_emplace(regs.vertex_attrib_format);
-    auto& VAO = iter->second;
+    auto& vao_entry = iter->second;
 
     if (is_cache_miss) {
-        VAO.Create();
+        vao_entry.Create();
-        state.draw.vertex_array = VAO.handle;
+        const GLuint vao = vao_entry.handle;
-        state.ApplyVertexBufferState();
 
-        // The index buffer binding is stored within the VAO. Stupid OpenGL, but easy to work
+        // Eventhough we are using DSA to create this vertex array, there is a bug on Intel's blob
-        // around.
+        // that fails to properly create the vertex array if it's not bound even after creating it
-        glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, buffer_cache.GetHandle());
+        // with glCreateVertexArrays
+        state.draw.vertex_array = vao;
+        state.ApplyVertexArrayState();
+
+        glVertexArrayElementBuffer(vao, buffer_cache.GetHandle());
 
         // Use the vertex array as-is, assumes that the data is formatted correctly for OpenGL.
         // Enables the first 16 vertex attributes always, as we don't know which ones are actually
@@ -163,7 +167,7 @@ void RasterizerOpenGL::SetupVertexFormat() {
         // for now to avoid OpenGL errors.
         // TODO(Subv): Analyze the shader to identify which attributes are actually used and don't
         // assume every shader uses them all.
-        for (unsigned index = 0; index < 16; ++index) {
+        for (u32 index = 0; index < 16; ++index) {
             const auto& attrib = regs.vertex_attrib_format[index];
 
             // Ignore invalid attributes.
@@ -178,28 +182,29 @@ void RasterizerOpenGL::SetupVertexFormat() {
 
             ASSERT(buffer.IsEnabled());
 
-            glEnableVertexAttribArray(index);
+            glEnableVertexArrayAttrib(vao, index);
             if (attrib.type == Tegra::Engines::Maxwell3D::Regs::VertexAttribute::Type::SignedInt ||
                 attrib.type ==
                     Tegra::Engines::Maxwell3D::Regs::VertexAttribute::Type::UnsignedInt) {
-                glVertexAttribIFormat(index, attrib.ComponentCount(),
+                glVertexArrayAttribIFormat(vao, index, attrib.ComponentCount(),
-                                      MaxwellToGL::VertexType(attrib), attrib.offset);
+                                           MaxwellToGL::VertexType(attrib), attrib.offset);
             } else {
-                glVertexAttribFormat(index, attrib.ComponentCount(),
+                glVertexArrayAttribFormat(
-                                     MaxwellToGL::VertexType(attrib),
+                    vao, index, attrib.ComponentCount(), MaxwellToGL::VertexType(attrib),
-                                     attrib.IsNormalized() ? GL_TRUE : GL_FALSE, attrib.offset);
+                    attrib.IsNormalized() ? GL_TRUE : GL_FALSE, attrib.offset);
             }
-            glVertexAttribBinding(index, attrib.buffer);
+            glVertexArrayAttribBinding(vao, index, attrib.buffer);
         }
     }
-    state.draw.vertex_array = VAO.handle;
-    state.ApplyVertexBufferState();
 
     // Rebinding the VAO invalidates the vertex buffer bindings.
     gpu.dirty_flags.vertex_array = 0xFFFFFFFF;
+
+    state.draw.vertex_array = vao_entry.handle;
+    return vao_entry.handle;
 }
 
-void RasterizerOpenGL::SetupVertexBuffer() {
+void RasterizerOpenGL::SetupVertexBuffer(GLuint vao) {
     auto& gpu = Core::System::GetInstance().GPU().Maxwell3D();
     const auto& regs = gpu.regs;
 
@@ -217,7 +222,7 @@ void RasterizerOpenGL::SetupVertexBuffer() {
         if (!vertex_array.IsEnabled())
             continue;
 
-        Tegra::GPUVAddr start = vertex_array.StartAddress();
+        const Tegra::GPUVAddr start = vertex_array.StartAddress();
         const Tegra::GPUVAddr end = regs.vertex_array_limit[index].LimitAddress();
 
         ASSERT(end > start);
@@ -225,21 +230,18 @@ void RasterizerOpenGL::SetupVertexBuffer() {
         const GLintptr vertex_buffer_offset = buffer_cache.UploadMemory(start, size);
 
         // Bind the vertex array to the buffer at the current offset.
-        glBindVertexBuffer(index, buffer_cache.GetHandle(), vertex_buffer_offset,
+        glVertexArrayVertexBuffer(vao, index, buffer_cache.GetHandle(), vertex_buffer_offset,
-                           vertex_array.stride);
+                                  vertex_array.stride);
 
         if (regs.instanced_arrays.IsInstancingEnabled(index) && vertex_array.divisor != 0) {
             // Enable vertex buffer instancing with the specified divisor.
-            glVertexBindingDivisor(index, vertex_array.divisor);
+            glVertexArrayBindingDivisor(vao, index, vertex_array.divisor);
         } else {
             // Disable the vertex buffer instancing.
-            glVertexBindingDivisor(index, 0);
+            glVertexArrayBindingDivisor(vao, index, 0);
         }
     }
 
-    // Implicit set by glBindVertexBuffer. Stupid glstate handling...
-    state.draw.vertex_buffer = buffer_cache.GetHandle();
-
     gpu.dirty_flags.vertex_array = 0;
 }
 
@@ -691,9 +693,6 @@ void RasterizerOpenGL::DrawArrays() {
     // Draw the vertex batch
     const bool is_indexed = accelerate_draw == AccelDraw::Indexed;
 
-    state.draw.vertex_buffer = buffer_cache.GetHandle();
-    state.ApplyVertexBufferState();
-
     std::size_t buffer_size = CalculateVertexArraysSize();
 
     // Add space for index buffer (keeping in mind non-core primitives)
@@ -723,8 +722,9 @@ void RasterizerOpenGL::DrawArrays() {
         gpu.dirty_flags.vertex_array = 0xFFFFFFFF;
     }
 
-    SetupVertexFormat();
+    const GLuint vao = SetupVertexFormat();
-    SetupVertexBuffer();
+    SetupVertexBuffer(vao);
+
     DrawParameters params = SetupDraw();
     SetupShaders(params.primitive_mode);
 

src/video_core/renderer_opengl/gl_rasterizer.h

@@ -215,8 +215,10 @@ private:
 
     std::size_t CalculateIndexBufferSize() const;
 
-    void SetupVertexFormat();
+    /// Updates and returns a vertex array object representing current vertex format
-    void SetupVertexBuffer();
+    GLuint SetupVertexFormat();
+
+    void SetupVertexBuffer(GLuint vao);
 
     DrawParameters SetupDraw();
 

src/video_core/renderer_opengl/gl_resource_manager.cpp

@@ -117,7 +117,7 @@ void OGLBuffer::Create() {
         return;
 
     MICROPROFILE_SCOPE(OpenGL_ResourceCreation);
-    glGenBuffers(1, &handle);
+    glCreateBuffers(1, &handle);
 }
 
 void OGLBuffer::Release() {
@@ -126,7 +126,6 @@ void OGLBuffer::Release() {
 
     MICROPROFILE_SCOPE(OpenGL_ResourceDeletion);
     glDeleteBuffers(1, &handle);
-    OpenGLState::GetCurState().ResetBuffer(handle).Apply();
     handle = 0;
 }
 
@@ -152,7 +151,7 @@ void OGLVertexArray::Create() {
         return;
 
     MICROPROFILE_SCOPE(OpenGL_ResourceCreation);
-    glGenVertexArrays(1, &handle);
+    glCreateVertexArrays(1, &handle);
 }
 
 void OGLVertexArray::Release() {

src/video_core/renderer_opengl/gl_state.cpp

@@ -83,8 +83,6 @@ OpenGLState::OpenGLState() {
     draw.read_framebuffer = 0;
     draw.draw_framebuffer = 0;
     draw.vertex_array = 0;
-    draw.vertex_buffer = 0;
-    draw.uniform_buffer = 0;
     draw.shader_program = 0;
     draw.program_pipeline = 0;
 
@@ -505,7 +503,6 @@ void OpenGLState::ApplySamplers() const {
 }
 
 void OpenGLState::ApplyFramebufferState() const {
-    // Framebuffer
     if (draw.read_framebuffer != cur_state.draw.read_framebuffer) {
         glBindFramebuffer(GL_READ_FRAMEBUFFER, draw.read_framebuffer);
     }
@@ -514,16 +511,10 @@ void OpenGLState::ApplyFramebufferState() const {
     }
 }
 
-void OpenGLState::ApplyVertexBufferState() const {
+void OpenGLState::ApplyVertexArrayState() const {
-    // Vertex array
     if (draw.vertex_array != cur_state.draw.vertex_array) {
         glBindVertexArray(draw.vertex_array);
     }
-
-    // Vertex buffer
-    if (draw.vertex_buffer != cur_state.draw.vertex_buffer) {
-        glBindBuffer(GL_ARRAY_BUFFER, draw.vertex_buffer);
-    }
 }
 
 void OpenGLState::ApplyDepthClamp() const {
@@ -543,11 +534,7 @@ void OpenGLState::ApplyDepthClamp() const {
 
 void OpenGLState::Apply() const {
     ApplyFramebufferState();
-    ApplyVertexBufferState();
+    ApplyVertexArrayState();
-    // Uniform buffer
-    if (draw.uniform_buffer != cur_state.draw.uniform_buffer) {
-        glBindBuffer(GL_UNIFORM_BUFFER, draw.uniform_buffer);
-    }
 
     // Shader program
     if (draw.shader_program != cur_state.draw.shader_program) {
@@ -638,16 +625,6 @@ OpenGLState& OpenGLState::ResetPipeline(GLuint handle) {
     return *this;
 }
 
-OpenGLState& OpenGLState::ResetBuffer(GLuint handle) {
-    if (draw.vertex_buffer == handle) {
-        draw.vertex_buffer = 0;
-    }
-    if (draw.uniform_buffer == handle) {
-        draw.uniform_buffer = 0;
-    }
-    return *this;
-}
-
 OpenGLState& OpenGLState::ResetVertexArray(GLuint handle) {
     if (draw.vertex_array == handle) {
         draw.vertex_array = 0;

src/video_core/renderer_opengl/gl_state.h

@@ -154,8 +154,6 @@ public:
         GLuint read_framebuffer; // GL_READ_FRAMEBUFFER_BINDING
         GLuint draw_framebuffer; // GL_DRAW_FRAMEBUFFER_BINDING
         GLuint vertex_array;     // GL_VERTEX_ARRAY_BINDING
-        GLuint vertex_buffer;    // GL_ARRAY_BUFFER_BINDING
-        GLuint uniform_buffer;   // GL_UNIFORM_BUFFER_BINDING
         GLuint shader_program;   // GL_CURRENT_PROGRAM
         GLuint program_pipeline; // GL_PROGRAM_PIPELINE_BINDING
     } draw;
@@ -206,10 +204,10 @@ public:
     }
     /// Apply this state as the current OpenGL state
     void Apply() const;
-    /// Apply only the state afecting the framebuffer
+    /// Apply only the state affecting the framebuffer
     void ApplyFramebufferState() const;
-    /// Apply only the state afecting the vertex buffer
+    /// Apply only the state affecting the vertex array
-    void ApplyVertexBufferState() const;
+    void ApplyVertexArrayState() const;
     /// Set the initial OpenGL state
     static void ApplyDefaultState();
     /// Resets any references to the given resource
@@ -217,7 +215,6 @@ public:
     OpenGLState& ResetSampler(GLuint handle);
     OpenGLState& ResetProgram(GLuint handle);
     OpenGLState& ResetPipeline(GLuint handle);
-    OpenGLState& ResetBuffer(GLuint handle);
     OpenGLState& ResetVertexArray(GLuint handle);
     OpenGLState& ResetFramebuffer(GLuint handle);
     void EmulateViewportWithScissor();

src/video_core/renderer_opengl/gl_stream_buffer.cpp

@@ -15,13 +15,12 @@ MICROPROFILE_DEFINE(OpenGL_StreamBuffer, "OpenGL", "Stream Buffer Orphaning",
 
 namespace OpenGL {
 
-OGLStreamBuffer::OGLStreamBuffer(GLenum target, GLsizeiptr size, bool prefer_coherent)
+OGLStreamBuffer::OGLStreamBuffer(GLsizeiptr size, bool vertex_data_usage, bool prefer_coherent)
-    : gl_target(target), buffer_size(size) {
+    : buffer_size(size) {
     gl_buffer.Create();
-    glBindBuffer(gl_target, gl_buffer.handle);
 
     GLsizeiptr allocate_size = size;
-    if (target == GL_ARRAY_BUFFER) {
+    if (vertex_data_usage) {
         // On AMD GPU there is a strange crash in indexed drawing. The crash happens when the buffer
         // read position is near the end and is an out-of-bound access to the vertex buffer. This is
         // probably a bug in the driver and is related to the usage of vec3<byte> attributes in the
@@ -35,18 +34,17 @@ OGLStreamBuffer::OGLStreamBuffer(GLenum target, GLsizeiptr size, bool prefer_coh
         coherent = prefer_coherent;
         const GLbitfield flags =
             GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | (coherent ? GL_MAP_COHERENT_BIT : 0);
-        glBufferStorage(gl_target, allocate_size, nullptr, flags);
+        glNamedBufferStorage(gl_buffer.handle, allocate_size, nullptr, flags);
-        mapped_ptr = static_cast<u8*>(glMapBufferRange(
+        mapped_ptr = static_cast<u8*>(glMapNamedBufferRange(
-            gl_target, 0, buffer_size, flags | (coherent ? 0 : GL_MAP_FLUSH_EXPLICIT_BIT)));
+            gl_buffer.handle, 0, buffer_size, flags | (coherent ? 0 : GL_MAP_FLUSH_EXPLICIT_BIT)));
     } else {
-        glBufferData(gl_target, allocate_size, nullptr, GL_STREAM_DRAW);
+        glNamedBufferData(gl_buffer.handle, allocate_size, nullptr, GL_STREAM_DRAW);
     }
 }
 
 OGLStreamBuffer::~OGLStreamBuffer() {
     if (persistent) {
-        glBindBuffer(gl_target, gl_buffer.handle);
+        glUnmapNamedBuffer(gl_buffer.handle);
-        glUnmapBuffer(gl_target);
     }
     gl_buffer.Release();
 }
@@ -74,7 +72,7 @@ std::tuple<u8*, GLintptr, bool> OGLStreamBuffer::Map(GLsizeiptr size, GLintptr a
         invalidate = true;
 
         if (persistent) {
-            glUnmapBuffer(gl_target);
+            glUnmapNamedBuffer(gl_buffer.handle);
         }
     }
 
@@ -84,7 +82,7 @@ std::tuple<u8*, GLintptr, bool> OGLStreamBuffer::Map(GLsizeiptr size, GLintptr a
                            (coherent ? GL_MAP_COHERENT_BIT : GL_MAP_FLUSH_EXPLICIT_BIT) |
                            (invalidate ? GL_MAP_INVALIDATE_BUFFER_BIT : GL_MAP_UNSYNCHRONIZED_BIT);
         mapped_ptr = static_cast<u8*>(
-            glMapBufferRange(gl_target, buffer_pos, buffer_size - buffer_pos, flags));
+            glMapNamedBufferRange(gl_buffer.handle, buffer_pos, buffer_size - buffer_pos, flags));
         mapped_offset = buffer_pos;
     }
 
@@ -95,11 +93,11 @@ void OGLStreamBuffer::Unmap(GLsizeiptr size) {
     ASSERT(size <= mapped_size);
 
     if (!coherent && size > 0) {
-        glFlushMappedBufferRange(gl_target, buffer_pos - mapped_offset, size);
+        glFlushMappedNamedBufferRange(gl_buffer.handle, buffer_pos - mapped_offset, size);
     }
 
     if (!persistent) {
-        glUnmapBuffer(gl_target);
+        glUnmapNamedBuffer(gl_buffer.handle);
     }
 
     buffer_pos += size;

src/video_core/renderer_opengl/gl_stream_buffer.h

@@ -13,7 +13,7 @@ namespace OpenGL {
 
 class OGLStreamBuffer : private NonCopyable {
 public:
-    explicit OGLStreamBuffer(GLenum target, GLsizeiptr size, bool prefer_coherent = false);
+    explicit OGLStreamBuffer(GLsizeiptr size, bool vertex_data_usage, bool prefer_coherent = false);
     ~OGLStreamBuffer();
 
     GLuint GetHandle() const;
@@ -33,7 +33,6 @@ public:
 
 private:
     OGLBuffer gl_buffer;
-    GLenum gl_target;
 
     bool coherent = false;
     bool persistent = false;

src/video_core/renderer_opengl/renderer_opengl.cpp

@@ -245,20 +245,20 @@ void RendererOpenGL::InitOpenGLObjects() {
 
     // Generate VAO
     vertex_array.Create();
-
     state.draw.vertex_array = vertex_array.handle;
-    state.draw.vertex_buffer = vertex_buffer.handle;
-    state.draw.uniform_buffer = 0;
-    state.Apply();
 
     // Attach vertex data to VAO
-    glBufferData(GL_ARRAY_BUFFER, sizeof(ScreenRectVertex) * 4, nullptr, GL_STREAM_DRAW);
+    glNamedBufferData(vertex_buffer.handle, sizeof(ScreenRectVertex) * 4, nullptr, GL_STREAM_DRAW);
-    glVertexAttribPointer(attrib_position, 2, GL_FLOAT, GL_FALSE, sizeof(ScreenRectVertex),
+    glVertexArrayAttribFormat(vertex_array.handle, attrib_position, 2, GL_FLOAT, GL_FALSE,
-                          (GLvoid*)offsetof(ScreenRectVertex, position));
+                              offsetof(ScreenRectVertex, position));
-    glVertexAttribPointer(attrib_tex_coord, 2, GL_FLOAT, GL_FALSE, sizeof(ScreenRectVertex),
+    glVertexArrayAttribFormat(vertex_array.handle, attrib_tex_coord, 2, GL_FLOAT, GL_FALSE,
-                          (GLvoid*)offsetof(ScreenRectVertex, tex_coord));
+                              offsetof(ScreenRectVertex, tex_coord));
-    glEnableVertexAttribArray(attrib_position);
+    glVertexArrayAttribBinding(vertex_array.handle, attrib_position, 0);
-    glEnableVertexAttribArray(attrib_tex_coord);
+    glVertexArrayAttribBinding(vertex_array.handle, attrib_tex_coord, 0);
+    glEnableVertexArrayAttrib(vertex_array.handle, attrib_position);
+    glEnableVertexArrayAttrib(vertex_array.handle, attrib_tex_coord);
+    glVertexArrayVertexBuffer(vertex_array.handle, 0, vertex_buffer.handle, 0,
+                              sizeof(ScreenRectVertex));
 
     // Allocate textures for the screen
     screen_info.texture.resource.Create();
@@ -370,14 +370,12 @@ void RendererOpenGL::DrawScreenTriangles(const ScreenInfo& screen_info, float x,
     state.texture_units[0].texture = screen_info.display_texture;
     state.texture_units[0].swizzle = {GL_RED, GL_GREEN, GL_BLUE, GL_ALPHA};
     // Workaround brigthness problems in SMO by enabling sRGB in the final output
-    // if it has been used in the frame
+    // if it has been used in the frame. Needed because of this bug in QT: QTBUG-50987
-    // Needed because of this bug in QT
-    // QTBUG-50987
     state.framebuffer_srgb.enabled = OpenGLState::GetsRGBUsed();
     state.Apply();
-    glBufferSubData(GL_ARRAY_BUFFER, 0, sizeof(vertices), vertices.data());
+    glNamedBufferSubData(vertex_buffer.handle, 0, sizeof(vertices), vertices.data());
     glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
-    // restore default state
+    // Restore default state
     state.framebuffer_srgb.enabled = false;
     state.texture_units[0].texture = 0;
     state.Apply();