Fixed binding offsets for uniform and transform feedback buffers

There was some confusion between buffer mapping and size and offset
buffer binding arguments, which are distinct, but were represented
by the same Buffer class members. Added OffsetBindingPointer to solve
the issue and removed setOffset/setSize from the Buffer class, which
should not have existed in the first place (only the mapRange/unmap
functions should be allowed to modify these values, for now).

Change-Id: Iacecd17cfb90d0a229d9edf62a463c8acf31f07a
Reviewed-on: https://swiftshader-review.googlesource.com/4590
Tested-by: Alexis Hétu <sugoi@google.com>
Reviewed-by: Nicolas Capens <capn@google.com>

diff --git a/src/OpenGL/common/Object.hpp b/src/OpenGL/common/Object.hpp
index 14b2975..0dbe07a 100644 (file)
--- a/src/OpenGL/common/Object.hpp
+++ b/src/OpenGL/common/Object.hpp
@@ -31,7 +31,7 @@ public:
 
     virtual void addRef();
        virtual void release();
-    
+
 private:
     volatile int referenceCount;
 };
@@ -61,7 +61,7 @@ public:
                ASSERT(!object);   // Objects have to be released before the resource manager is destroyed, so they must be explicitly cleaned up. Assign null to all binding pointers to make the reference count go to zero.
        }
 
-    ObjectType *operator=(ObjectType *newObject) 
+    ObjectType *operator=(ObjectType *newObject)
        {
                if(newObject) newObject->addRef();
                if(object) object->release();
@@ -71,7 +71,7 @@ public:
                return object;
        }
 
-       ObjectType *operator=(const BindingPointer<ObjectType> &other) 
+       ObjectType *operator=(const BindingPointer<ObjectType> &other)
        {
                return operator=(other.object);
        }

diff --git a/src/OpenGL/libGLESv2/Buffer.h b/src/OpenGL/libGLESv2/Buffer.h
index 1f509ec..b5e47cb 100644 (file)
--- a/src/OpenGL/libGLESv2/Buffer.h
+++ b/src/OpenGL/libGLESv2/Buffer.h
@@ -44,8 +44,6 @@ class Buffer : public gl::NamedObject
        GLsizeiptr length() const { return mLength; }\r
        GLbitfield access() const { return mAccess; }\r
 \r
-       void setOffset(GLintptr offset) { mOffset = offset; }\r
-       void setSize(size_t size) { mSize = size; }\r
        void* mapRange(GLintptr offset, GLsizeiptr length, GLbitfield access);\r
        bool unmap();\r
        void flushMappedRange(GLintptr offset, GLsizeiptr length) {}\r
@@ -62,6 +60,28 @@ class Buffer : public gl::NamedObject
        GLbitfield mAccess;\r
 };\r
 \r
+class UniformBufferBinding\r
+{\r
+public:\r
+       UniformBufferBinding() : offset(0), size(0) { }\r
+\r
+       void set(Buffer *newUniformBuffer, int newOffset = 0, int newSize = 0)\r
+       {\r
+               uniformBuffer = newUniformBuffer;\r
+               offset = newOffset;\r
+               size = newSize;\r
+       }\r
+\r
+       int getOffset() const { return offset; }\r
+       int getSize() const { return size; }\r
+       const gl::BindingPointer<Buffer>& get() const { return uniformBuffer; }\r
+\r
+private:\r
+       gl::BindingPointer<Buffer> uniformBuffer;\r
+       int offset;\r
+       int size;\r
+};\r
+\r
 }\r
 \r
 #endif   // LIBGLESV2_BUFFER_H_\r

diff --git a/src/OpenGL/libGLESv2/Context.cpp b/src/OpenGL/libGLESv2/Context.cpp
index af69141..04ec7f8 100644 (file)
--- a/src/OpenGL/libGLESv2/Context.cpp
+++ b/src/OpenGL/libGLESv2/Context.cpp
@@ -1293,12 +1293,7 @@ void Context::bindIndexedUniformBuffer(GLuint buffer, GLuint index, GLintptr off
        mResourceManager->checkBufferAllocation(buffer);\r
 \r
        Buffer* bufferObject = getBuffer(buffer);\r
-       if(bufferObject)\r
-       {\r
-               bufferObject->setOffset(offset);\r
-               bufferObject->setSize(size);\r
-       }\r
-       mState.uniformBuffers[index] = bufferObject;\r
+       mState.uniformBuffers[index].set(bufferObject, offset, size);\r
 }\r
 \r
 void Context::bindGenericTransformFeedbackBuffer(GLuint buffer)\r
@@ -1313,12 +1308,7 @@ void Context::bindIndexedTransformFeedbackBuffer(GLuint buffer, GLuint index, GL
        mResourceManager->checkBufferAllocation(buffer);\r
 \r
        Buffer* bufferObject = getBuffer(buffer);\r
-       if(bufferObject)\r
-       {\r
-               bufferObject->setOffset(offset);\r
-               bufferObject->setSize(size);\r
-       }\r
-       getTransformFeedback()->setBuffer(index, bufferObject);\r
+       getTransformFeedback()->setBuffer(index, bufferObject, offset, size);\r
 }\r
 \r
 bool Context::bindTransformFeedback(GLuint id)\r

diff --git a/src/OpenGL/libGLESv2/Context.h b/src/OpenGL/libGLESv2/Context.h
index eb4e810..cf06cfb 100644 (file)
--- a/src/OpenGL/libGLESv2/Context.h
+++ b/src/OpenGL/libGLESv2/Context.h
@@ -44,7 +44,6 @@ struct TranslatedAttribute;
 struct TranslatedIndexData;\r
 \r
 class Device;\r
-class Buffer;\r
 class Shader;\r
 class Program;\r
 class Texture;\r
@@ -378,7 +377,7 @@ struct State
        gl::BindingPointer<Buffer> pixelPackBuffer;\r
        gl::BindingPointer<Buffer> pixelUnpackBuffer;\r
        gl::BindingPointer<Buffer> genericUniformBuffer;\r
-       gl::BindingPointer<Buffer> uniformBuffers[MAX_UNIFORM_BUFFER_BINDINGS];\r
+       UniformBufferBinding uniformBuffers[MAX_UNIFORM_BUFFER_BINDINGS];\r
 \r
     GLuint readFramebuffer;\r
     GLuint drawFramebuffer;\r

diff --git a/src/OpenGL/libGLESv2/Program.cpp b/src/OpenGL/libGLESv2/Program.cpp
index 28b5571..17719a6 100644 (file)
--- a/src/OpenGL/libGLESv2/Program.cpp
+++ b/src/OpenGL/libGLESv2/Program.cpp
@@ -284,7 +284,7 @@ namespace es2
        int Program::getAttributeStream(int attributeIndex)\r
        {\r
                ASSERT(attributeIndex >= 0 && attributeIndex < MAX_VERTEX_ATTRIBS);\r
-    \r
+\r
                return attributeStream[attributeIndex];\r
        }\r
 \r
@@ -485,7 +485,7 @@ namespace es2
                {\r
                        return false;   // Attempting to write an array to a non-array uniform is an INVALID_OPERATION\r
                }\r
-       \r
+\r
                count = std::min(size - (int)uniformIndex[location].element, count);\r
 \r
                int index = numElements - 1;\r
@@ -672,7 +672,7 @@ namespace es2
                {\r
                        return false;   // Attempting to write an array to a non-array uniform is an INVALID_OPERATION\r
                }\r
-       \r
+\r
                count = std::min(size - (int)uniformIndex[location].element, count);\r
 \r
                if(targetUniform->type == GL_INT || targetUniform->type == GL_UNSIGNED_INT || IsSamplerUniform(targetUniform->type))\r
@@ -729,7 +729,7 @@ namespace es2
                {\r
                        return false;   // Attempting to write an array to a non-array uniform is an INVALID_OPERATION\r
                }\r
-       \r
+\r
                count = std::min(size - (int)uniformIndex[location].element, count);\r
 \r
                int index = numElements - 1;\r
@@ -791,7 +791,7 @@ namespace es2
                {\r
                        return false;   // Attempting to write an array to a non-array uniform is an INVALID_OPERATION\r
                }\r
-       \r
+\r
                count = std::min(size - (int)uniformIndex[location].element, count);\r
 \r
                if(targetUniform->type == GL_INT || targetUniform->type == GL_UNSIGNED_INT || IsSamplerUniform(targetUniform->type))\r
@@ -848,7 +848,7 @@ namespace es2
                {\r
                        return false;   // Attempting to write an array to a non-array uniform is an INVALID_OPERATION\r
                }\r
-       \r
+\r
                count = std::min(size - (int)uniformIndex[location].element, count);\r
 \r
                int index = numElements - 1;\r
@@ -1132,7 +1132,7 @@ namespace es2
                }\r
        }\r
 \r
-       void Program::applyUniformBuffers(gl::BindingPointer<Buffer>* uniformBuffers)\r
+       void Program::applyUniformBuffers(UniformBufferBinding* uniformBuffers)\r
        {\r
                GLint vertexUniformBuffers[IMPLEMENTATION_MAX_UNIFORM_BUFFER_BINDINGS];\r
                GLint fragmentUniformBuffers[IMPLEMENTATION_MAX_UNIFORM_BUFFER_BINDINGS];\r
@@ -1175,9 +1175,9 @@ namespace es2
                for(unsigned int bufferBindingIndex = 0; bufferBindingIndex < IMPLEMENTATION_MAX_UNIFORM_BUFFER_BINDINGS; ++bufferBindingIndex)\r
                {\r
                        int index = vertexUniformBuffers[bufferBindingIndex];\r
-                       device->VertexProcessor::setUniformBuffer(bufferBindingIndex, (index != -1) ? uniformBuffers[index]->getResource() : nullptr, 0);\r
+                       device->VertexProcessor::setUniformBuffer(bufferBindingIndex, (index != -1) ? uniformBuffers[index].get()->getResource() : nullptr, (index != -1) ? uniformBuffers[index].getOffset() : 0);\r
                        index = fragmentUniformBuffers[bufferBindingIndex];\r
-                       device->PixelProcessor::setUniformBuffer(bufferBindingIndex, (index != -1) ? uniformBuffers[index]->getResource() : nullptr, 0);\r
+                       device->PixelProcessor::setUniformBuffer(bufferBindingIndex, (index != -1) ? uniformBuffers[index].get()->getResource() : nullptr, (index != -1) ? uniformBuffers[index].getOffset() : 0);\r
                }\r
        }\r
 \r
@@ -1256,7 +1256,7 @@ namespace es2
                                                        if(components >= 1) pixelBinary->semantic[in + i][0] = sw::Shader::Semantic();\r
                                                        if(components >= 2) pixelBinary->semantic[in + i][1] = sw::Shader::Semantic();\r
                                                        if(components >= 3) pixelBinary->semantic[in + i][2] = sw::Shader::Semantic();\r
-                                                       if(components >= 4) pixelBinary->semantic[in + i][3] = sw::Shader::Semantic();                                  \r
+                                                       if(components >= 4) pixelBinary->semantic[in + i][3] = sw::Shader::Semantic();\r
                                                }\r
                                        }\r
 \r
@@ -1316,7 +1316,7 @@ namespace es2
 \r
                vertexBinary = new sw::VertexShader(vertexShader->getVertexShader());\r
                pixelBinary = new sw::PixelShader(fragmentShader->getPixelShader());\r
-                       \r
+\r
                if(!linkVaryings())\r
                {\r
                        return;\r
@@ -1467,7 +1467,7 @@ namespace es2
                if(IsSamplerUniform(type))\r
            {\r
                        int index = registerIndex;\r
-                       \r
+\r
                        do\r
                        {\r
                                if(shader == GL_VERTEX_SHADER)\r
@@ -1510,7 +1510,7 @@ namespace es2
                                        if(index < MAX_TEXTURE_IMAGE_UNITS)\r
                                        {\r
                                                samplersPS[index].active = true;\r
-                                               \r
+\r
                                                switch(type)\r
                                                {\r
                                                default:                      UNREACHABLE(type);\r
@@ -2307,7 +2307,7 @@ namespace es2
                return linked;\r
        }\r
 \r
-       bool Program::isValidated() const \r
+       bool Program::isValidated() const\r
        {\r
                return validated;\r
        }\r
@@ -2685,7 +2685,7 @@ namespace es2
        {\r
                resetInfoLog();\r
 \r
-               if(!isLinked()) \r
+               if(!isLinked())\r
                {\r
                        appendToInfoLog("Program has not been successfully linked.");\r
                        validated = false;\r
@@ -2722,7 +2722,7 @@ namespace es2
                        if(samplersPS[i].active)\r
                        {\r
                                unsigned int unit = samplersPS[i].logicalTextureUnit;\r
-            \r
+\r
                                if(unit >= MAX_COMBINED_TEXTURE_IMAGE_UNITS)\r
                                {\r
                                        if(logErrors)\r
@@ -2757,7 +2757,7 @@ namespace es2
                        if(samplersVS[i].active)\r
                        {\r
                                unsigned int unit = samplersVS[i].logicalTextureUnit;\r
-            \r
+\r
                                if(unit >= MAX_COMBINED_TEXTURE_IMAGE_UNITS)\r
                                {\r
                                        if(logErrors)\r

diff --git a/src/OpenGL/libGLESv2/Program.h b/src/OpenGL/libGLESv2/Program.h
index 86a861e..641b415 100644 (file)
--- a/src/OpenGL/libGLESv2/Program.h
+++ b/src/OpenGL/libGLESv2/Program.h
@@ -167,7 +167,7 @@ namespace es2
 \r
                void dirtyAllUniforms();\r
                void applyUniforms();\r
-               void applyUniformBuffers(gl::BindingPointer<Buffer>* uniformBuffers);\r
+               void applyUniformBuffers(UniformBufferBinding* uniformBuffers);\r
 \r
                void link();\r
                bool isLinked() const;\r

diff --git a/src/OpenGL/libGLESv2/TransformFeedback.cpp b/src/OpenGL/libGLESv2/TransformFeedback.cpp
index e69f9ad..d71c748 100644 (file)
--- a/src/OpenGL/libGLESv2/TransformFeedback.cpp
+++ b/src/OpenGL/libGLESv2/TransformFeedback.cpp
@@ -13,8 +13,6 @@
 \r
 #include "TransformFeedback.h"\r
 \r
-#include "Buffer.h"\r
-\r
 namespace es2\r
 {\r
 \r
@@ -28,7 +26,7 @@ TransformFeedback::~TransformFeedback()
        mGenericBuffer = NULL;\r
        for(int i = 0; i < MAX_TRANSFORM_FEEDBACK_SEPARATE_ATTRIBS; ++i)\r
        {\r
-               mBuffer[i] = NULL;\r
+               mBuffer[i].set(nullptr);\r
        }\r
 }\r
 \r
@@ -39,7 +37,7 @@ Buffer* TransformFeedback::getGenericBuffer() const
 \r
 Buffer* TransformFeedback::getBuffer(GLuint index) const\r
 {\r
-       return mBuffer[index];\r
+       return mBuffer[index].get();\r
 }\r
 \r
 bool TransformFeedback::isActive() const\r
@@ -80,16 +78,12 @@ void TransformFeedback::setGenericBuffer(Buffer* buffer)
 \r
 void TransformFeedback::setBuffer(GLuint index, Buffer* buffer)\r
 {\r
-       mBuffer[index] = buffer;\r
+       mBuffer[index].set(buffer);\r
 }\r
 \r
 void TransformFeedback::setBuffer(GLuint index, Buffer* buffer, GLintptr offset, GLsizeiptr size)\r
 {\r
-       mBuffer[index] = buffer;\r
-       if(buffer)\r
-       {\r
-               buffer->mapRange(offset, size, buffer->access());\r
-       }\r
+       mBuffer[index].set(buffer, offset, size);\r
 }\r
 \r
 void TransformFeedback::detachBuffer(GLuint buffer)\r
@@ -101,9 +95,9 @@ void TransformFeedback::detachBuffer(GLuint buffer)
 \r
        for(int i = 0; i < MAX_TRANSFORM_FEEDBACK_SEPARATE_ATTRIBS; ++i)\r
        {\r
-               if(mBuffer[i].name() == buffer)\r
+               if(mBuffer[i].get().name() == buffer)\r
                {\r
-                       mBuffer[i] = NULL;\r
+                       mBuffer[i].set(nullptr);\r
                }\r
        }\r
 }\r

diff --git a/src/OpenGL/libGLESv2/TransformFeedback.h b/src/OpenGL/libGLESv2/TransformFeedback.h
index ab86604..631f17f 100644 (file)
--- a/src/OpenGL/libGLESv2/TransformFeedback.h
+++ b/src/OpenGL/libGLESv2/TransformFeedback.h
@@ -14,6 +14,7 @@
 #ifndef LIBGLESV2_TRANSFORM_FEEDBACK_H_\r
 #define LIBGLESV2_TRANSFORM_FEEDBACK_H_\r
 \r
+#include "Buffer.h"\r
 #include "common/Object.hpp"\r
 #include "Renderer/Renderer.hpp"\r
 \r
@@ -21,7 +22,6 @@
 \r
 namespace es2\r
 {\r
-class Buffer;\r
 \r
 class TransformFeedback : public gl::NamedObject\r
 {\r
@@ -46,7 +46,7 @@ public:
 \r
 private:\r
        gl::BindingPointer<Buffer> mGenericBuffer;\r
-       gl::BindingPointer<Buffer> mBuffer[MAX_TRANSFORM_FEEDBACK_SEPARATE_ATTRIBS];\r
+       UniformBufferBinding mBuffer[MAX_TRANSFORM_FEEDBACK_SEPARATE_ATTRIBS];\r
 \r
        bool mActive;\r
        bool mPaused;\r