Fix ResourceManager create-on-bind reallocations.

src/libANGLE/HandleAllocator.cpp

@@ -9,13 +9,29 @@
 
 #include "libANGLE/HandleAllocator.h"
 
+#include <algorithm>
+
 #include "common/debug.h"
 
 namespace gl
 {
 
+struct HandleAllocator::HandleRangeComparator
+{
+    bool operator()(const HandleRange &range, GLuint handle) const
+    {
+        return (handle < range.begin);
+    }
+};
+
 HandleAllocator::HandleAllocator() : mBaseValue(1), mNextValue(1)
 {
+    mUnallocatedList.push_back(HandleRange(1, std::numeric_limits<GLuint>::max() - 1));
+}
+
+HandleAllocator::HandleAllocator(GLuint maximumHandleValue) : mBaseValue(1), mNextValue(1)
+{
+    mUnallocatedList.push_back(HandleRange(1, maximumHandleValue));
 }
 
 HandleAllocator::~HandleAllocator()
@@ -31,32 +47,86 @@ void HandleAllocator::setBaseHandle(GLuint value)
 
 GLuint HandleAllocator::allocate()
 {
-    if (mFreeValues.size())
+    ASSERT(!mUnallocatedList.empty() || !mReleasedList.empty());
+
+    // Allocate from released list, constant time.
+    if (!mReleasedList.empty())
+    {
+        GLuint reusedHandle = mReleasedList.back();
+        mReleasedList.pop_back();
+        return reusedHandle;
+    }
+
+    // Allocate from unallocated list, constant time.
+    auto listIt = mUnallocatedList.begin();
+
+    GLuint freeListHandle = listIt->begin;
+    ASSERT(freeListHandle > 0);
+
+    listIt->begin++;
+    if (listIt->begin == listIt->end)
     {
-        GLuint handle = mFreeValues.back();
-        mFreeValues.pop_back();
-        return handle;
+        mUnallocatedList.erase(listIt);
     }
-    return mNextValue++;
+
+    return freeListHandle;
 }
 
 void HandleAllocator::release(GLuint handle)
 {
-    if (handle == mNextValue - 1)
+    // Add to released list, constant time.
+    mReleasedList.push_back(handle);
+}
+
+void HandleAllocator::reserve(GLuint handle)
+{
+    // Clear from released list -- might be a slow operation.
+    if (!mReleasedList.empty())
     {
-        // Don't drop below base value
-        if(mNextValue > mBaseValue)
+        auto releasedIt = std::find(mReleasedList.begin(), mReleasedList.end(), handle);
+        if (releasedIt != mReleasedList.end())
         {
-            mNextValue--;
+            mReleasedList.erase(releasedIt);
+            return;
         }
     }
-    else
+
+    // Not in released list, reserve in the unallocated list.
+    auto boundIt = std::lower_bound(mUnallocatedList.begin(), mUnallocatedList.end(), handle, HandleRangeComparator());
+
+    ASSERT(boundIt != mUnallocatedList.end());
+
+    GLuint begin = boundIt->begin;
+    GLuint end = boundIt->end;
+
+    if (handle == begin || handle == end)
     {
-        // Only free handles that we own - don't drop below the base value
-        if (handle >= mBaseValue)
+        if (begin + 1 == end)
+        {
+            mUnallocatedList.erase(boundIt);
+        }
+        else if (handle == begin)
         {
-            mFreeValues.push_back(handle);
+            boundIt->begin++;
         }
+        else
+        {
+            ASSERT(handle == end);
+            boundIt->end--;
+        }
+        return;
+    }
+
+    // need to split the range
+    auto placementIt = mUnallocatedList.erase(boundIt);
+
+    if (begin != handle)
+    {
+        placementIt = mUnallocatedList.insert(placementIt, HandleRange(begin, handle));
+    }
+    if (handle + 1 != end)
+    {
+        mUnallocatedList.insert(placementIt, HandleRange(handle + 1, end));
     }
 }
 

src/libANGLE/HandleAllocator.h

@@ -14,21 +14,26 @@
 
 #include "angle_gl.h"
 
-#include <vector>
+#include <stack>
 
 namespace gl
 {
 
-class HandleAllocator
+class HandleAllocator final
 {
   public:
+    // Maximum handle = MAX_UINT-1
     HandleAllocator();
-    virtual ~HandleAllocator();
+    // Specify maximum handle value
+    HandleAllocator(GLuint maximumHandleValue);
+
+    ~HandleAllocator();
 
     void setBaseHandle(GLuint value);
 
     GLuint allocate();
     void release(GLuint handle);
+    void reserve(GLuint handle);
 
   private:
     DISALLOW_COPY_AND_ASSIGN(HandleAllocator);
@@ -37,6 +42,22 @@ class HandleAllocator
     GLuint mNextValue;
     typedef std::vector<GLuint> HandleList;
     HandleList mFreeValues;
+
+    struct HandleRange
+    {
+        HandleRange(GLuint beginIn, GLuint endIn) : begin(beginIn), end(endIn) {}
+
+        GLuint begin;
+        GLuint end;
+    };
+
+    struct HandleRangeComparator;
+
+    // The freelist consists of never-allocated handles, stored
+    // as ranges, and handles that were previously allocated and
+    // released, stored in a stack.
+    std::vector<HandleRange> mUnallocatedList;
+    std::vector<GLuint> mReleasedList;
 };
 
 }

src/libANGLE/HandleAllocator_unittest.cpp

+//
+// Copyright 2015 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Unit tests for HandleAllocator.
+//
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "libANGLE/HandleAllocator.h"
+
+namespace
+{
+
+TEST(HandleAllocatorTest, ReservationsWithGaps)
+{
+    gl::HandleAllocator allocator;
+
+    std::set<GLuint> allocationList;
+    for (GLuint id = 2; id < 50; id += 2)
+    {
+        allocationList.insert(id);
+    }
+
+    for (GLuint id : allocationList)
+    {
+        allocator.reserve(id);
+    }
+
+    std::set<GLuint> allocatedList;
+    for (size_t allocationNum = 0; allocationNum < allocationList.size() * 2; ++allocationNum)
+    {
+        GLuint handle = allocator.allocate();
+        EXPECT_EQ(0, allocationList.count(handle));
+        EXPECT_EQ(0, allocatedList.count(handle));
+        allocatedList.insert(handle);
+    }
+}
+
+TEST(HandleAllocatorTest, Random)
+{
+    gl::HandleAllocator allocator;
+
+    std::set<GLuint> allocationList;
+    for (size_t iterationCount = 0; iterationCount < 40; ++iterationCount)
+    {
+        for (size_t randomCount = 0; randomCount < 40; ++randomCount)
+        {
+            GLuint randomHandle = (rand() % 1000) + 1;
+            if (allocationList.count(randomHandle) == 0)
+            {
+                allocator.reserve(randomHandle);
+                allocationList.insert(randomHandle);
+            }
+        }
+
+        for (size_t normalCount = 0; normalCount < 40; ++normalCount)
+        {
+            GLuint normalHandle = allocator.allocate();
+            EXPECT_EQ(0, allocationList.count(normalHandle));
+            allocationList.insert(normalHandle);
+        }
+    }
+}
+
+TEST(HandleAllocatorTest, Reallocation)
+{
+    // Note: no current test for overflow
+    gl::HandleAllocator limitedAllocator(10);
+
+    for (GLuint count = 1; count < 10; count++)
+    {
+        GLuint result = limitedAllocator.allocate();
+        EXPECT_EQ(count, result);
+    }
+
+    for (GLuint count = 1; count < 10; count++)
+    {
+        limitedAllocator.release(count);
+    }
+
+    for (GLuint count = 2; count < 10; count++)
+    {
+        limitedAllocator.reserve(count);
+    }
+
+    GLint finalResult = limitedAllocator.allocate();
+    EXPECT_EQ(finalResult, 1);
+}
+
+}

src/libANGLE/Renderbuffer.cpp

@@ -27,7 +27,6 @@ Renderbuffer::Renderbuffer(rx::RenderbufferImpl *impl, GLuint id)
     mInternalFormat(GL_RGBA4),
     mSamples(0)
 {
-    ASSERT(mRenderbuffer);
 }
 
 Renderbuffer::~Renderbuffer()

src/libANGLE/ResourceManager.cpp

@@ -356,33 +356,84 @@ void ResourceManager::setRenderbuffer(GLuint handle, Renderbuffer *buffer)
     mRenderbufferMap[handle] = buffer;
 }
 
-void ResourceManager::checkBufferAllocation(unsigned int buffer)
+void ResourceManager::checkBufferAllocation(GLuint handle)
 {
-    if (buffer != 0 && !getBuffer(buffer))
+    if (handle != 0)
     {
-        Buffer *bufferObject = new Buffer(mFactory->createBuffer(), buffer);
-        mBufferMap[buffer] = bufferObject;
-        bufferObject->addRef();
+        auto bufferMapIt = mBufferMap.find(handle);
+        bool handleAllocated = (bufferMapIt != mBufferMap.end());
+
+        if (handleAllocated && bufferMapIt->second != nullptr)
+        {
+            return;
+        }
+
+        Buffer *buffer = new Buffer(mFactory->createBuffer(), handle);
+        buffer->addRef();
+
+        if (handleAllocated)
+        {
+            bufferMapIt->second = buffer;
+        }
+        else
+        {
+            mBufferHandleAllocator.reserve(handle);
+            mBufferMap[handle] = buffer;
+        }
     }
 }
 
-void ResourceManager::checkTextureAllocation(GLuint texture, GLenum type)
+void ResourceManager::checkTextureAllocation(GLuint handle, GLenum type)
 {
-    if (!getTexture(texture) && texture != 0)
+    if (handle != 0)
     {
-        Texture *textureObject = new Texture(mFactory->createTexture(type), texture, type);
-        mTextureMap[texture] = textureObject;
-        textureObject->addRef();
+        auto textureMapIt = mTextureMap.find(handle);
+        bool handleAllocated = (textureMapIt != mTextureMap.end());
+
+        if (handleAllocated && textureMapIt->second != nullptr)
+        {
+            return;
+        }
+
+        Texture *texture = new Texture(mFactory->createTexture(type), handle, type);
+        texture->addRef();
+
+        if (handleAllocated)
+        {
+            textureMapIt->second = texture;
+        }
+        else
+        {
+            mTextureHandleAllocator.reserve(handle);
+            mTextureMap[handle] = texture;
+        }
     }
 }
 
-void ResourceManager::checkRenderbufferAllocation(GLuint renderbuffer)
+void ResourceManager::checkRenderbufferAllocation(GLuint handle)
 {
-    if (renderbuffer != 0 && !getRenderbuffer(renderbuffer))
+    if (handle != 0)
     {
-        Renderbuffer *renderbufferObject = new Renderbuffer(mFactory->createRenderbuffer(), renderbuffer);
-        mRenderbufferMap[renderbuffer] = renderbufferObject;
-        renderbufferObject->addRef();
+        auto renderbufferMapIt = mRenderbufferMap.find(handle);
+        bool handleAllocated = (renderbufferMapIt != mRenderbufferMap.end());
+
+        if (handleAllocated && renderbufferMapIt->second != nullptr)
+        {
+            return;
+        }
+
+        Renderbuffer *renderbuffer = new Renderbuffer(mFactory->createRenderbuffer(), handle);
+        renderbuffer->addRef();
+
+        if (handleAllocated)
+        {
+            renderbufferMapIt->second = renderbuffer;
+        }
+        else
+        {
+            mRenderbufferHandleAllocator.reserve(handle);
+            mRenderbufferMap[handle] = renderbuffer;
+        }
     }
 }
 
@@ -393,6 +444,7 @@ void ResourceManager::checkSamplerAllocation(GLuint sampler)
         Sampler *samplerObject = new Sampler(sampler);
         mSamplerMap[sampler] = samplerObject;
         samplerObject->addRef();
+        // Samplers cannot be created via Bind
     }
 }
 

src/libANGLE/ResourceManager.h

@@ -68,9 +68,9 @@ class ResourceManager
 
     void setRenderbuffer(GLuint handle, Renderbuffer *renderbuffer);
 
-    void checkBufferAllocation(unsigned int buffer);
-    void checkTextureAllocation(GLuint texture, GLenum type);
-    void checkRenderbufferAllocation(GLuint renderbuffer);
+    void checkBufferAllocation(GLuint handle);
+    void checkTextureAllocation(GLuint handle, GLenum type);
+    void checkRenderbufferAllocation(GLuint handle);
     void checkSamplerAllocation(GLuint sampler);
 
     bool isSampler(GLuint sampler);
@@ -78,6 +78,8 @@ class ResourceManager
   private:
     DISALLOW_COPY_AND_ASSIGN(ResourceManager);
 
+    void createTextureInternal(GLuint handle);
+
     rx::ImplFactory *mFactory;
     std::size_t mRefCount;
 

src/libANGLE/ResourceManager_unittest.cpp

+//
+// Copyright 2015 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Unit tests for ResourceManager.
+//
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "angle_unittests_utils.h"
+#include "libANGLE/ResourceManager.h"
+
+using namespace rx;
+using namespace gl;
+
+namespace
+{
+
+class MockFactory : public NullFactory
+{
+  public:
+    MOCK_METHOD0(createBuffer, BufferImpl*());
+    MOCK_METHOD1(createTexture, TextureImpl*(GLenum));
+    MOCK_METHOD0(createRenderbuffer, RenderbufferImpl*());
+};
+
+class ResourceManagerTest : public testing::Test
+{
+  protected:
+    void SetUp() override
+    {
+        mResourceManager = new ResourceManager(&mMockFactory);
+    }
+
+    void TearDown() override
+    {
+        SafeDelete(mResourceManager);
+    }
+
+    MockFactory mMockFactory;
+    ResourceManager *mResourceManager;
+};
+
+TEST_F(ResourceManagerTest, ReallocateBoundTexture)
+{
+    EXPECT_CALL(mMockFactory, createTexture(GL_TEXTURE_2D)).Times(1).RetiresOnSaturation();
+
+    mResourceManager->checkTextureAllocation(1, GL_TEXTURE_2D);
+    GLuint newTexture = mResourceManager->createTexture();
+    EXPECT_NE(1u, newTexture);
+}
+
+TEST_F(ResourceManagerTest, ReallocateBoundBuffer)
+{
+    EXPECT_CALL(mMockFactory, createBuffer()).Times(1).RetiresOnSaturation();
+
+    mResourceManager->checkBufferAllocation(1);
+    GLuint newBuffer = mResourceManager->createBuffer();
+    EXPECT_NE(1u, newBuffer);
+}
+
+TEST_F(ResourceManagerTest, ReallocateBoundRenderbuffer)
+{
+    EXPECT_CALL(mMockFactory, createRenderbuffer()).Times(1).RetiresOnSaturation();
+
+    mResourceManager->checkRenderbufferAllocation(1);
+    GLuint newRenderbuffer = mResourceManager->createRenderbuffer();
+    EXPECT_NE(1u, newRenderbuffer);
+}
+
+}

src/tests/angle_unittests.gypi

@@ -18,9 +18,12 @@
             '<(angle_path)/src/common/Optional_unittest.cpp',
             '<(angle_path)/src/libANGLE/Config_unittest.cpp',
             '<(angle_path)/src/libANGLE/Fence_unittest.cpp',
+            '<(angle_path)/src/libANGLE/HandleAllocator_unittest.cpp',
             '<(angle_path)/src/libANGLE/ImageIndexIterator_unittest.cpp',
+            '<(angle_path)/src/libANGLE/ResourceManager_unittest.cpp',
             '<(angle_path)/src/libANGLE/Surface_unittest.cpp',
             '<(angle_path)/src/libANGLE/TransformFeedback_unittest.cpp',
+            '<(angle_path)/src/tests/angle_unittests_utils.h',
             '<(angle_path)/src/tests/compiler_tests/API_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/CollectVariables_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/ConstantFolding_test.cpp',
@@ -64,6 +67,7 @@
         '<(angle_path)/include',
         '<(angle_path)/src',
         '<(angle_path)/src/compiler/preprocessor',
+        '<(angle_path)/src/tests',
     ],
     'sources':
     [

src/tests/angle_unittests_utils.h

+//
+// Copyright 2015 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// angle_unittests_utils.h:
+//   Helpers for mocking and unit testing.
+
+#ifndef TESTS_ANGLE_UNITTESTS_UTILS_H_
+#define TESTS_ANGLE_UNITTESTS_UTILS_H_
+
+#include "libANGLE/renderer/ImplFactory.h"
+
+namespace rx
+{
+
+// Useful when mocking a part of the ImplFactory class
+class NullFactory : public ImplFactory
+{
+  public:
+    NullFactory() {}
+
+    // Shader creation
+    CompilerImpl *createCompiler(const gl::Data &data) override { return nullptr; }
+    ShaderImpl *createShader(GLenum type) override { return nullptr; }
+    ProgramImpl *createProgram() override { return nullptr; }
+
+    // Framebuffer creation
+    DefaultAttachmentImpl *createDefaultAttachment(GLenum type, egl::Surface *surface) override { return nullptr; }
+    FramebufferImpl *createFramebuffer(const gl::Framebuffer::Data &data) override { return nullptr; }
+
+    // Texture creation
+    TextureImpl *createTexture(GLenum target) override { return nullptr; }
+
+    // Renderbuffer creation
+    RenderbufferImpl *createRenderbuffer() override { return nullptr; }
+
+    // Buffer creation
+    BufferImpl *createBuffer() override { return nullptr; }
+
+    // Vertex Array creation
+    VertexArrayImpl *createVertexArray() override { return nullptr; }
+
+    // Query and Fence creation
+    QueryImpl *createQuery(GLenum type) override { return nullptr; }
+    FenceNVImpl *createFenceNV() override { return nullptr; }
+    FenceSyncImpl *createFenceSync() override { return nullptr; }
+
+    // Transform Feedback creation
+    TransformFeedbackImpl *createTransformFeedback() override { return nullptr; }
+
+    DISALLOW_COPY_AND_ASSIGN(NullFactory);
+};
+
+}
+
+#endif // TESTS_ANGLE_UNITTESTS_UTILS_H_