Add a Buffer::getIndexRange function.

Instead of exposing the index range cache, add a more generic method that
allows the buffers to handle their own caching of index ranges.
BufferImpl::getData can be hard to implement for BufferGL because there
isn't a way to tell the buffer to unmap and glGetBufferSubData can be very
expensive, requiring an extra copy of the data.

BUG=angleproject:881

Change-Id: Idec645219056132e0d72a410fbe7b971fa02c9e9
Reviewed-on: https://chromium-review.googlesource.com/261892
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Olli Etuaho <oetuaho@nvidia.com>
Tested-by: Geoff Lang <geofflang@chromium.org>

src/libANGLE/Buffer.cpp

@@ -93,8 +93,7 @@ Error Buffer::map(GLenum access)
     mMapLength = mSize;
     mAccess = access;
     mAccessFlags = GL_MAP_WRITE_BIT;
-
-    mIndexRangeCache.invalidateRange(0, static_cast<unsigned int>(mMapLength));
+    mIndexRangeCache.clear();
 
     return error;
 }
@@ -151,4 +150,32 @@ Error Buffer::unmap(GLboolean *result)
     return error;
 }
 
+void Buffer::onTransformFeedback()
+{
+    mIndexRangeCache.clear();
+}
+
+void Buffer::onPixelUnpack()
+{
+    mIndexRangeCache.clear();
+}
+
+Error Buffer::getIndexRange(GLenum type, size_t offset, size_t count, gl::RangeUI *outRange) const
+{
+    if (mIndexRangeCache.findRange(type, offset, count, outRange))
+    {
+        return gl::Error(GL_NO_ERROR);
+    }
+
+    Error error = mBuffer->getIndexRange(type, offset, count, outRange);
+    if (error.isError())
+    {
+        return error;
+    }
+
+    mIndexRangeCache.addRange(type, offset, count, *outRange);
+
+    return Error(GL_NO_ERROR);
+}
+
 }

src/libANGLE/Buffer.h

@@ -38,6 +38,11 @@ class Buffer : public RefCountObject
     Error mapRange(GLintptr offset, GLsizeiptr length, GLbitfield access);
     Error unmap(GLboolean *result);
 
+    void onTransformFeedback();
+    void onPixelUnpack();
+
+    Error getIndexRange(GLenum type, size_t offset, size_t count, RangeUI *outRange) const;
+
     GLenum getUsage() const { return mUsage; }
     GLbitfield getAccessFlags() const { return mAccessFlags; }
     GLenum getAccess() const { return mAccess; }
@@ -49,9 +54,6 @@ class Buffer : public RefCountObject
 
     rx::BufferImpl *getImplementation() const { return mBuffer; }
 
-    IndexRangeCache *getIndexRangeCache() { return &mIndexRangeCache; }
-    const IndexRangeCache *getIndexRangeCache() const { return &mIndexRangeCache; }
-
   private:
     rx::BufferImpl *mBuffer;
 
@@ -64,7 +66,7 @@ class Buffer : public RefCountObject
     GLint64 mMapOffset;
     GLint64 mMapLength;
 
-    IndexRangeCache mIndexRangeCache;
+    mutable IndexRangeCache mIndexRangeCache;
 };
 
 }

src/libANGLE/Context.cpp

@@ -1219,7 +1219,26 @@ bool Context::getIndexedQueryParameterInfo(GLenum target, GLenum *type, unsigned
 
 Error Context::drawArrays(GLenum mode, GLint first, GLsizei count, GLsizei instances)
 {
-    return mRenderer->drawArrays(getData(), mode, first, count, instances);
+    Error error = mRenderer->drawArrays(getData(), mode, first, count, instances);
+    if (error.isError())
+    {
+        return error;
+    }
+
+    TransformFeedback *transformFeedback = mState.getCurrentTransformFeedback();
+    if (transformFeedback->isActive() && !transformFeedback->isPaused())
+    {
+        for (size_t tfBufferIndex = 0; tfBufferIndex < transformFeedback->getIndexedBufferCount(); tfBufferIndex++)
+        {
+            const OffsetBindingPointer<Buffer> &buffer = transformFeedback->getIndexedBuffer(tfBufferIndex);
+            if (buffer.get() != nullptr)
+            {
+                buffer->onTransformFeedback();
+            }
+        }
+    }
+
+    return Error(GL_NO_ERROR);
 }
 
 Error Context::drawElements(GLenum mode, GLsizei count, GLenum type,

src/libANGLE/Framebuffer.cpp

@@ -551,7 +551,19 @@ GLenum Framebuffer::getImplementationColorReadType() const
 
 Error Framebuffer::readPixels(const gl::State &state, const gl::Rectangle &area, GLenum format, GLenum type, GLvoid *pixels) const
 {
-    return mImpl->readPixels(state, area, format, type, pixels);
+    Error error = mImpl->readPixels(state, area, format, type, pixels);
+    if (error.isError())
+    {
+        return error;
+    }
+
+    Buffer *unpackBuffer = state.getUnpackState().pixelBuffer.get();
+    if (unpackBuffer)
+    {
+        unpackBuffer->onPixelUnpack();
+    }
+
+    return Error(GL_NO_ERROR);
 }
 
 Error Framebuffer::blit(const gl::State &state, const gl::Rectangle &sourceArea, const gl::Rectangle &destArea,

src/libANGLE/TransformFeedback_unittest.cpp

@@ -6,6 +6,7 @@
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "libANGLE/Buffer.h"
 #include "libANGLE/Caps.h"
 #include "libANGLE/TransformFeedback.h"
 #include "libANGLE/renderer/BufferImpl_mock.h"

src/libANGLE/renderer/BufferImpl.h

@@ -10,7 +10,8 @@
 #define LIBANGLE_RENDERER_BUFFERIMPL_H_
 
 #include "common/angleutils.h"
-#include "libANGLE/Buffer.h"
+#include "common/mathutil.h"
+#include "libANGLE/Error.h"
 
 #include <stdint.h>
 
@@ -29,9 +30,7 @@ class BufferImpl : angle::NonCopyable
     virtual gl::Error mapRange(size_t offset, size_t length, GLbitfield access, GLvoid **mapPtr) = 0;
     virtual gl::Error unmap(GLboolean *result) = 0;
 
-    // This method may not have a corresponding GL-backed function. It is necessary
-    // for validation, for certain indexed draw calls.
-    virtual gl::Error getData(const uint8_t **outData) = 0;
+    virtual gl::Error getIndexRange(GLenum type, size_t offset, size_t count, gl::RangeUI *outRange) = 0;
 };
 
 }

src/libANGLE/renderer/BufferImpl_mock.h

@@ -28,7 +28,7 @@ class MockBufferImpl : public BufferImpl
     MOCK_METHOD4(mapRange, gl::Error(size_t, size_t, GLbitfield, GLvoid **));
     MOCK_METHOD1(unmap, gl::Error(GLboolean *result));
 
-    MOCK_METHOD1(getData, gl::Error(const uint8_t **));
+    MOCK_METHOD4(getIndexRange, gl::Error(GLenum, size_t, size_t, gl::RangeUI *));
 
     MOCK_METHOD0(destructor, void());
 };

src/libANGLE/renderer/d3d/BufferD3D.cpp

@@ -8,6 +8,7 @@
 
 #include "libANGLE/renderer/d3d/BufferD3D.h"
 
+#include "common/utilities.h"
 #include "libANGLE/renderer/d3d/IndexBuffer.h"
 #include "libANGLE/renderer/d3d/VertexBuffer.h"
 
@@ -77,4 +78,17 @@ void BufferD3D::promoteStaticUsage(int dataSize)
     }
 }
 
-}
+gl::Error BufferD3D::getIndexRange(GLenum type, size_t offset, size_t count, gl::RangeUI *outRange)
+{
+    const uint8_t *data = nullptr;
+    gl::Error error = getData(&data);
+    if (error.isError())
+    {
+        return error;
+    }
+
+    *outRange = gl::ComputeIndexRange(type, data + offset, count);
+    return gl::Error(GL_NO_ERROR);
+}
+
+}

src/libANGLE/renderer/d3d/BufferD3D.h

@@ -9,8 +9,8 @@
 #ifndef LIBANGLE_RENDERER_D3D_BUFFERD3D_H_
 #define LIBANGLE_RENDERER_D3D_BUFFERD3D_H_
 
-#include "libANGLE/renderer/BufferImpl.h"
 #include "libANGLE/angletypes.h"
+#include "libANGLE/renderer/BufferImpl.h"
 
 #include <stdint.h>
 
@@ -31,6 +31,7 @@ class BufferD3D : public BufferImpl
     virtual size_t getSize() const = 0;
     virtual bool supportsDirectBinding() const = 0;
     virtual void markTransformFeedbackUsage() = 0;
+    virtual gl::Error getData(const uint8_t **outData) = 0;
 
     StaticVertexBufferInterface *getStaticVertexBuffer() { return mStaticVertexBuffer; }
     StaticIndexBufferInterface *getStaticIndexBuffer() { return mStaticIndexBuffer; }
@@ -39,6 +40,8 @@ class BufferD3D : public BufferImpl
     void invalidateStaticData();
     void promoteStaticUsage(int dataSize);
 
+    gl::Error getIndexRange(GLenum type, size_t offset, size_t count, gl::RangeUI *outRange) override;
+
   protected:
     void updateSerial();
 

src/libANGLE/renderer/d3d/d3d11/Buffer11.h

@@ -9,6 +9,8 @@
 #ifndef LIBANGLE_RENDERER_D3D_D3D11_BUFFER11_H_
 #define LIBANGLE_RENDERER_D3D_D3D11_BUFFER11_H_
 
+#include <map>
+
 #include "libANGLE/angletypes.h"
 #include "libANGLE/renderer/d3d/BufferD3D.h"
 
@@ -59,10 +61,10 @@ class Buffer11 : public BufferD3D
     // BufferD3D implementation
     virtual size_t getSize() const { return mSize; }
     virtual bool supportsDirectBinding() const;
+    gl::Error getData(const uint8_t **outData) override;
 
     // BufferImpl implementation
     virtual gl::Error setData(const void* data, size_t size, GLenum usage);
-    gl::Error getData(const uint8_t **outData) override;
     virtual gl::Error setSubData(const void* data, size_t size, size_t offset);
     virtual gl::Error copySubData(BufferImpl* source, GLintptr sourceOffset, GLintptr destOffset, GLsizeiptr size);
     virtual gl::Error map(GLenum access, GLvoid **mapPtr);

src/libANGLE/renderer/d3d/d3d11/Framebuffer11.cpp

@@ -161,8 +161,6 @@ gl::Error Framebuffer11::readPixels(const gl::Rectangle &area, GLenum format, GL
             SafeRelease(colorBufferTexture);
             return error;
         }
-
-        packBuffer->getIndexRangeCache()->clear();
     }
     else
     {

src/libANGLE/renderer/d3d/d3d9/Buffer9.h

@@ -26,10 +26,10 @@ class Buffer9 : public BufferD3D
     // BufferD3D implementation
     virtual size_t getSize() const { return mSize; }
     virtual bool supportsDirectBinding() const { return false; }
+    gl::Error getData(const uint8_t **outData) override;
 
     // BufferImpl implementation
     virtual gl::Error setData(const void* data, size_t size, GLenum usage);
-    gl::Error getData(const uint8_t **outData) override;
     virtual gl::Error setSubData(const void* data, size_t size, size_t offset);
     virtual gl::Error copySubData(BufferImpl* source, GLintptr sourceOffset, GLintptr destOffset, GLsizeiptr size);
     virtual gl::Error map(GLenum access, GLvoid **mapPtr);

src/libANGLE/renderer/gl/BufferGL.cpp

@@ -9,6 +9,7 @@
 #include "libANGLE/renderer/gl/BufferGL.h"
 
 #include "common/debug.h"
+#include "common/utilities.h"
 #include "libANGLE/angletypes.h"
 #include "libANGLE/renderer/gl/FunctionsGL.h"
 #include "libANGLE/renderer/gl/StateManagerGL.h"
@@ -28,6 +29,7 @@ static const GLenum DestBufferOperationTarget = GL_ARRAY_BUFFER;
 
 BufferGL::BufferGL(const FunctionsGL *functions, StateManagerGL *stateManager)
     : BufferImpl(),
+      mIsMapped(false),
       mFunctions(functions),
       mStateManager(stateManager),
       mBufferID(0)
@@ -77,6 +79,8 @@ gl::Error BufferGL::map(GLenum access, GLvoid **mapPtr)
 {
     mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
     *mapPtr = mFunctions->mapBuffer(DestBufferOperationTarget, access);
+
+    mIsMapped = true;
     return gl::Error(GL_NO_ERROR);
 }
 
@@ -84,6 +88,8 @@ gl::Error BufferGL::mapRange(size_t offset, size_t length, GLbitfield access, GL
 {
     mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
     *mapPtr = mFunctions->mapBufferRange(DestBufferOperationTarget, offset, length, access);
+
+    mIsMapped = true;
     return gl::Error(GL_NO_ERROR);
 }
 
@@ -93,13 +99,21 @@ gl::Error BufferGL::unmap(GLboolean *result)
 
     mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
     *result = mFunctions->unmapBuffer(DestBufferOperationTarget);
+
+    mIsMapped = false;
     return gl::Error(GL_NO_ERROR);
 }
 
-gl::Error BufferGL::getData(const uint8_t **outData)
+gl::Error BufferGL::getIndexRange(GLenum type, size_t offset, size_t count, gl::RangeUI *outRange)
 {
-    UNIMPLEMENTED();
-    return gl::Error(GL_INVALID_OPERATION);
+    ASSERT(!mIsMapped);
+
+    mStateManager->bindBuffer(DestBufferOperationTarget, mBufferID);
+    const uint8_t *bufferData = reinterpret_cast<uint8_t*>(mFunctions->mapBuffer(DestBufferOperationTarget, GL_READ_ONLY));
+    *outRange = gl::ComputeIndexRange(type, bufferData + offset, count);
+    mFunctions->unmapBuffer(DestBufferOperationTarget);
+
+    return gl::Error(GL_NO_ERROR);
 }
 
 GLuint BufferGL::getBufferID() const

src/libANGLE/renderer/gl/BufferGL.h

@@ -30,13 +30,13 @@ class BufferGL : public BufferImpl
     gl::Error mapRange(size_t offset, size_t length, GLbitfield access, GLvoid **mapPtr) override;
     gl::Error unmap(GLboolean *result) override;
 
-    // This method may not have a corresponding GL-backed function. It is necessary
-    // for validation, for certain indexed draw calls.
-    gl::Error getData(const uint8_t **outData) override;
+    gl::Error getIndexRange(GLenum type, size_t offset, size_t count, gl::RangeUI *outRange) override;
 
     GLuint getBufferID() const;
 
   private:
+    bool mIsMapped;
+
     const FunctionsGL *mFunctions;
     StateManagerGL *mStateManager;
 

src/libANGLE/renderer/gl/VertexArrayGL.cpp

@@ -261,25 +261,10 @@ gl::Error VertexArrayGL::syncIndexData(GLsizei count, GLenum type, const GLvoid
         if (attributesNeedStreaming)
         {
             ptrdiff_t elementArrayBufferOffset = reinterpret_cast<ptrdiff_t>(indices);
-
-            // Find the index range in the buffer
-            const gl::IndexRangeCache *rangeCache = mElementArrayBuffer.get()->getIndexRangeCache();
-
-            if (!rangeCache->findRange(type, static_cast<unsigned int>(elementArrayBufferOffset), count, outIndexRange))
+            gl::Error error = mElementArrayBuffer->getIndexRange(type, static_cast<size_t>(elementArrayBufferOffset), count, outIndexRange);
+            if (error.isError())
             {
-                // Need to compute the index range.
-                mStateManager->bindBuffer(GL_ELEMENT_ARRAY_BUFFER, elementArrayBufferID);
-                uint8_t *elementArrayBufferPointer = reinterpret_cast<uint8_t*>(mFunctions->mapBuffer(GL_ELEMENT_ARRAY_BUFFER, GL_READ_ONLY));
-
-                *outIndexRange = gl::ComputeIndexRange(type, elementArrayBufferPointer + elementArrayBufferOffset, count);
-
-                // TODO: Store the range cache at the impl level since the gl::Buffer object is supposed to remain constant
-                const_cast<gl::IndexRangeCache*>(rangeCache)->addRange(type, static_cast<unsigned int>(elementArrayBufferOffset), count, *outIndexRange);
-
-                if (!mFunctions->unmapBuffer(GL_ELEMENT_ARRAY_BUFFER))
-                {
-                    return gl::Error(GL_OUT_OF_MEMORY);
-                }
+                return error;
             }
         }
 

src/libANGLE/validationES.cpp

@@ -19,7 +19,6 @@
 #include "libANGLE/Uniform.h"
 #include "libANGLE/TransformFeedback.h"
 #include "libANGLE/VertexArray.h"
-#include "libANGLE/renderer/BufferImpl.h"
 
 #include "common/mathutil.h"
 #include "common/utilities.h"
@@ -1644,20 +1643,11 @@ bool ValidateDrawElements(Context *context, GLenum mode, GLsizei count, GLenum t
     if (elementArrayBuffer)
     {
         uintptr_t offset = reinterpret_cast<uintptr_t>(indices);
-        if (!elementArrayBuffer->getIndexRangeCache()->findRange(type, static_cast<unsigned int>(offset), count, indexRangeOut))
+        Error error = elementArrayBuffer->getIndexRange(type, static_cast<size_t>(offset), count, indexRangeOut);
+        if (error.isError())
         {
-            rx::BufferImpl *bufferImpl = elementArrayBuffer->getImplementation();
-            const uint8_t *dataPointer = NULL;
-            Error error = bufferImpl->getData(&dataPointer);
-            if (error.isError())
-            {
-                context->recordError(error);
-                return false;
-            }
-
-            const uint8_t *offsetPointer = dataPointer + offset;
-            *indexRangeOut = ComputeIndexRange(type, offsetPointer, count);
-            elementArrayBuffer->getIndexRangeCache()->addRange(type, static_cast<unsigned int>(offset), count, *indexRangeOut);
+            context->recordError(error);
+            return false;
         }
     }
     else

src/libANGLE/validationES.h

@@ -62,9 +62,9 @@ bool ValidateUniformMatrix(Context *context, GLenum matrixType, GLint location,
 
 bool ValidateStateQuery(Context *context, GLenum pname, GLenum *nativeType, unsigned int *numParams);
 
-bool ValidateCopyTexImageParametersBase(Context* context, GLenum target, GLint level, GLenum internalformat, bool isSubImage,
-                                                     GLint xoffset, GLint yoffset, GLint zoffset, GLint x, GLint y, GLsizei width, GLsizei height,
-                                                     GLint border, GLenum *textureInternalFormatOut);
+bool ValidateCopyTexImageParametersBase(Context *context, GLenum target, GLint level, GLenum internalformat, bool isSubImage,
+                                        GLint xoffset, GLint yoffset, GLint zoffset, GLint x, GLint y, GLsizei width, GLsizei height,
+                                        GLint border, GLenum *textureInternalFormatOut);
 
 bool ValidateDrawArrays(Context *context, GLenum mode, GLint first, GLsizei count, GLsizei primcount);
 bool ValidateDrawArraysInstanced(Context *context, GLenum mode, GLint first, GLsizei count, GLsizei primcount);

src/tests/perf_tests/IndexDataManagerTest.cpp

@@ -11,7 +11,6 @@
 
 #include <gmock/gmock.h>
 
-#include "common/utilities.h"
 #include "libANGLE/renderer/d3d/BufferD3D.h"
 #include "libANGLE/renderer/d3d/IndexBuffer.h"
 #include "libANGLE/renderer/d3d/IndexDataManager.h"
@@ -144,12 +143,7 @@ void IndexDataManagerPerfTest::step(float dt, double totalTime)
 
     for (unsigned int iteration = 0; iteration < 100; ++iteration)
     {
-        if (!mIndexBuffer.getIndexRangeCache()->findRange(GL_UNSIGNED_SHORT, 0, mIndexCount, &translatedIndexData.indexRange))
-        {
-            translatedIndexData.indexRange = gl::ComputeIndexRange(GL_UNSIGNED_SHORT, &mIndexData[0], mIndexCount);
-            mIndexBuffer.getIndexRangeCache()->addRange(GL_UNSIGNED_SHORT, 0, mIndexCount, translatedIndexData.indexRange);
-        }
-
+        mIndexBuffer.getIndexRange(GL_UNSIGNED_SHORT, 0, mIndexCount, &translatedIndexData.indexRange);
         mIndexDataManager.prepareIndexData(GL_UNSIGNED_SHORT, mIndexCount, &mIndexBuffer, nullptr, &translatedIndexData);
     }