D3D: Refactor VertexBuffer::getSpaceRequired.

src/libANGLE/renderer/d3d/RendererD3D.h

@@ -88,6 +88,10 @@ class BufferFactoryD3D
     // TODO(jmadill): add VertexFormatCaps
     virtual VertexConversionType getVertexConversionType(gl::VertexFormatType vertexFormatType) const = 0;
     virtual GLenum getVertexComponentType(gl::VertexFormatType vertexFormatType) const = 0;
+    virtual gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(
+        const gl::VertexAttribute &attrib,
+        GLsizei count,
+        GLsizei instances) const = 0;
 };
 
 class RendererD3D : public Renderer, public BufferFactoryD3D

src/libANGLE/renderer/d3d/VertexBuffer.cpp

@@ -98,15 +98,14 @@ gl::Error VertexBufferInterface::storeVertexAttributes(const gl::VertexAttribute
                                                        unsigned int *outStreamOffset,
                                                        const uint8_t *sourceData)
 {
-    gl::Error error(GL_NO_ERROR);
-
-    unsigned int spaceRequired;
-    error = mVertexBuffer->getSpaceRequired(attrib, count, instances, &spaceRequired);
-    if (error.isError())
+    auto errorOrSpaceRequired = mFactory->getVertexSpaceRequired(attrib, count, instances);
+    if (errorOrSpaceRequired.isError())
     {
-        return error;
+        return errorOrSpaceRequired.getError();
     }
 
+    unsigned int spaceRequired = errorOrSpaceRequired.getResult();
+
     // Align to 16-byte boundary
     unsigned int alignedSpaceRequired = roundUp(spaceRequired, 16u);
 
@@ -117,7 +116,7 @@ gl::Error VertexBufferInterface::storeVertexAttributes(const gl::VertexAttribute
         return gl::Error(GL_OUT_OF_MEMORY, "Internal error, new vertex buffer write position would overflow.");
     }
 
-    error = reserveSpace(mReservedSpace);
+    gl::Error error = reserveSpace(mReservedSpace);
     if (error.isError())
     {
         return error;
@@ -142,15 +141,14 @@ gl::Error VertexBufferInterface::storeVertexAttributes(const gl::VertexAttribute
 
 gl::Error VertexBufferInterface::reserveVertexSpace(const gl::VertexAttribute &attrib, GLsizei count, GLsizei instances)
 {
-    gl::Error error(GL_NO_ERROR);
-
-    unsigned int requiredSpace;
-    error = mVertexBuffer->getSpaceRequired(attrib, count, instances, &requiredSpace);
-    if (error.isError())
+    auto errorOrRequiredSpace = mFactory->getVertexSpaceRequired(attrib, count, instances);
+    if (errorOrRequiredSpace.isError())
     {
-        return error;
+        return errorOrRequiredSpace.getError();
     }
 
+    unsigned int requiredSpace = errorOrRequiredSpace.getResult();
+
     // Align to 16-byte boundary
     unsigned int alignedRequiredSpace = roundUp(requiredSpace, 16u);
 

src/libANGLE/renderer/d3d/VertexBuffer.h

@@ -44,8 +44,6 @@ class VertexBuffer : angle::NonCopyable
                                             GLsizei instances,
                                             unsigned int offset,
                                             const uint8_t *sourceData) = 0;
-    virtual gl::Error getSpaceRequired(const gl::VertexAttribute &attrib, GLsizei count, GLsizei instances,
-                                       unsigned int *outSpaceRequired) const = 0;
 
     virtual unsigned int getBufferSize() const = 0;
     virtual gl::Error setBufferSize(unsigned int size) = 0;

src/libANGLE/renderer/d3d/VertexDataManager.cpp

@@ -68,37 +68,36 @@ bool DirectStoragePossible(const gl::VertexAttribute &attrib)
         return false;
     }
 
-    StaticVertexBufferInterface *staticBuffer =
-        bufferD3D->getStaticVertexBuffer(attrib, D3D_BUFFER_CREATE_IF_NECESSARY);
-    // Dynamic buffers can not be stored directly.
-    if (!staticBuffer)
-    {
-        return false;
-    }
-
     // Alignment restrictions: In D3D, vertex data must be aligned to the format stride, or to a
     // 4-byte boundary, whichever is smaller. (Undocumented, and experimentally confirmed)
-    size_t alignment        = 4;
-    bool requiresConversion = false;
+    size_t alignment = 4;
 
     if (attrib.type != GL_FLOAT)
     {
         gl::VertexFormatType vertexFormatType = gl::GetVertexFormatType(attrib);
 
-        unsigned int outputElementSize;
-        staticBuffer->getVertexBuffer()->getSpaceRequired(attrib, 1, 0, &outputElementSize);
-        alignment = std::min<size_t>(outputElementSize, 4);
-
         // TODO(jmadill): add VertexFormatCaps
         BufferFactoryD3D *factory = bufferD3D->getFactory();
-        requiresConversion =
-            (factory->getVertexConversionType(vertexFormatType) & VERTEX_CONVERT_CPU) != 0;
-    }
 
-    bool isAligned = (static_cast<size_t>(ComputeVertexAttributeStride(attrib)) % alignment == 0) &&
-                     (static_cast<size_t>(attrib.offset) % alignment == 0);
+        auto errorOrElementSize = factory->getVertexSpaceRequired(attrib, 1, 0);
+        if (errorOrElementSize.isError())
+        {
+            ERR("Unlogged error in DirectStoragePossible.");
+            return false;
+        }
+
+        alignment = std::min<size_t>(errorOrElementSize.getResult(), 4);
 
-    return !requiresConversion && isAligned;
+        // CPU-converted vertex data must be converted (naturally).
+        if ((factory->getVertexConversionType(vertexFormatType) & VERTEX_CONVERT_CPU) != 0)
+        {
+            return false;
+        }
+    }
+
+    // Final alignment check - unaligned data must be converted.
+    return (static_cast<size_t>(ComputeVertexAttributeStride(attrib)) % alignment == 0) &&
+           (static_cast<size_t>(attrib.offset) % alignment == 0);
 }
 }  // anonymous namespace
 
@@ -380,16 +379,21 @@ gl::Error VertexDataManager::storeAttribute(TranslatedAttribute *translated,
     }
 
     unsigned int streamOffset = 0;
-    unsigned int outputElementSize = 0;
 
-    if (staticBuffer)
+    auto errorOrOutputElementSize = mFactory->getVertexSpaceRequired(attrib, 1, 0);
+    if (errorOrOutputElementSize.isError())
     {
-        gl::Error error = staticBuffer->getVertexBuffer()->getSpaceRequired(attrib, 1, 0, &outputElementSize);
-        if (error.isError())
-        {
-            return error;
-        }
+        return errorOrOutputElementSize.getError();
+    }
+
+    translated->storage = nullptr;
+    translated->stride  = errorOrOutputElementSize.getResult();
+    translated->serial  = vertexBuffer->getSerial();
+
+    gl::Error error(GL_NO_ERROR);
 
+    if (staticBuffer)
+    {
         if (!staticBuffer->lookupAttribute(attrib, &streamOffset))
         {
             // Convert the entire buffer
@@ -414,9 +418,9 @@ gl::Error VertexDataManager::storeAttribute(TranslatedAttribute *translated,
         unsigned int firstElementOffset =
             (static_cast<unsigned int>(attrib.offset) /
              static_cast<unsigned int>(ComputeVertexAttributeStride(attrib))) *
-            outputElementSize;
+            translated->stride;
         ASSERT(attrib.divisor == 0 || firstVertexIndex == 0);
-        unsigned int startOffset = firstVertexIndex * outputElementSize;
+        unsigned int startOffset = firstVertexIndex * translated->stride;
         if (streamOffset + firstElementOffset + startOffset < streamOffset)
         {
             return gl::Error(GL_OUT_OF_MEMORY);
@@ -427,11 +431,6 @@ gl::Error VertexDataManager::storeAttribute(TranslatedAttribute *translated,
     else
     {
         size_t totalCount = ComputeVertexAttributeElementCount(attrib, count, instances);
-        gl::Error error = mStreamingBuffer->getVertexBuffer()->getSpaceRequired(attrib, 1, 0, &outputElementSize);
-        if (error.isError())
-        {
-            return error;
-        }
 
         error = mStreamingBuffer->storeVertexAttributes(
             attrib, translated->currentValueType, firstVertexIndex,
@@ -442,9 +441,6 @@ gl::Error VertexDataManager::storeAttribute(TranslatedAttribute *translated,
         }
     }
 
-    translated->storage = nullptr;
-    translated->serial = vertexBuffer->getSerial();
-    translated->stride = outputElementSize;
     translated->offset = streamOffset;
 
     return gl::Error(GL_NO_ERROR);

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

@@ -4185,6 +4185,42 @@ GLenum Renderer11::getVertexComponentType(gl::VertexFormatType vertexFormatType)
     return d3d11::GetDXGIFormatInfo(d3d11::GetVertexFormatInfo(vertexFormatType, mRenderer11DeviceCaps.featureLevel).nativeFormat).componentType;
 }
 
+gl::ErrorOrResult<unsigned int> Renderer11::getVertexSpaceRequired(
+    const gl::VertexAttribute &attrib,
+    GLsizei count,
+    GLsizei instances) const
+{
+    if (!attrib.enabled)
+    {
+        return 16u;
+    }
+
+    unsigned int elementCount = 0;
+    if (instances == 0 || attrib.divisor == 0)
+    {
+        elementCount = count;
+    }
+    else
+    {
+        // Round up to divisor, if possible
+        elementCount = UnsignedCeilDivide(static_cast<unsigned int>(instances), attrib.divisor);
+    }
+
+    gl::VertexFormatType formatType      = gl::GetVertexFormatType(attrib);
+    const D3D_FEATURE_LEVEL featureLevel = mRenderer11DeviceCaps.featureLevel;
+    const d3d11::VertexFormat &vertexFormatInfo =
+        d3d11::GetVertexFormatInfo(formatType, featureLevel);
+    const d3d11::DXGIFormatSize &dxgiFormatInfo =
+        d3d11::GetDXGIFormatSizeInfo(vertexFormatInfo.nativeFormat);
+    unsigned int elementSize = dxgiFormatInfo.pixelBytes;
+    if (elementSize > std::numeric_limits<unsigned int>::max() / elementCount)
+    {
+        return gl::Error(GL_OUT_OF_MEMORY, "New vertex buffer size would result in an overflow.");
+    }
+
+    return elementSize * elementCount;
+}
+
 void Renderer11::generateCaps(gl::Caps *outCaps, gl::TextureCapsMap *outTextureCaps,
                               gl::Extensions *outExtensions, gl::Limitations *outLimitations) const
 {

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

@@ -266,6 +266,9 @@ class Renderer11 : public RendererD3D
     bool getLUID(LUID *adapterLuid) const override;
     VertexConversionType getVertexConversionType(gl::VertexFormatType vertexFormatType) const override;
     GLenum getVertexComponentType(gl::VertexFormatType vertexFormatType) const override;
+    gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(const gl::VertexAttribute &attrib,
+                                                           GLsizei count,
+                                                           GLsizei instances) const override;
 
     gl::Error readFromAttachment(const gl::FramebufferAttachment &srcAttachment,
                                  const gl::Rectangle &sourceArea,

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

@@ -144,52 +144,6 @@ gl::Error VertexBuffer11::storeVertexAttributes(const gl::VertexAttribute &attri
     return gl::Error(GL_NO_ERROR);
 }
 
-gl::Error VertexBuffer11::getSpaceRequired(const gl::VertexAttribute &attrib, GLsizei count,
-                                           GLsizei instances, unsigned int *outSpaceRequired) const
-{
-    unsigned int elementCount = 0;
-    if (attrib.enabled)
-    {
-        if (instances == 0 || attrib.divisor == 0)
-        {
-            elementCount = count;
-        }
-        else
-        {
-            // Round up to divisor, if possible
-            elementCount = UnsignedCeilDivide(static_cast<unsigned int>(instances), attrib.divisor);
-        }
-
-        gl::VertexFormatType formatType = gl::GetVertexFormatType(attrib);
-        const D3D_FEATURE_LEVEL featureLevel = mRenderer->getRenderer11DeviceCaps().featureLevel;
-        const d3d11::VertexFormat &vertexFormatInfo = d3d11::GetVertexFormatInfo(formatType, featureLevel);
-        const d3d11::DXGIFormatSize &dxgiFormatInfo =
-            d3d11::GetDXGIFormatSizeInfo(vertexFormatInfo.nativeFormat);
-        unsigned int elementSize = dxgiFormatInfo.pixelBytes;
-        if (elementSize <= std::numeric_limits<unsigned int>::max() / elementCount)
-        {
-            if (outSpaceRequired)
-            {
-                *outSpaceRequired = elementSize * elementCount;
-            }
-            return gl::Error(GL_NO_ERROR);
-        }
-        else
-        {
-            return gl::Error(GL_OUT_OF_MEMORY, "New vertex buffer size would result in an overflow.");
-        }
-    }
-    else
-    {
-        const unsigned int elementSize = 4;
-        if (outSpaceRequired)
-        {
-            *outSpaceRequired = elementSize * 4;
-        }
-        return gl::Error(GL_NO_ERROR);
-    }
-}
-
 unsigned int VertexBuffer11::getBufferSize() const
 {
     return mBufferSize;
@@ -233,4 +187,4 @@ ID3D11Buffer *VertexBuffer11::getBuffer() const
     return mBuffer;
 }
 
-}
+}  // namespace rx

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

@@ -21,9 +21,9 @@ class VertexBuffer11 : public VertexBuffer
 {
   public:
     explicit VertexBuffer11(Renderer11 *const renderer);
-    virtual ~VertexBuffer11();
+    ~VertexBuffer11() override;
 
-    virtual gl::Error initialize(unsigned int size, bool dynamicUsage);
+    gl::Error initialize(unsigned int size, bool dynamicUsage) override;
 
     gl::Error storeVertexAttributes(const gl::VertexAttribute &attrib,
                                     GLenum currentValueType,
@@ -33,14 +33,11 @@ class VertexBuffer11 : public VertexBuffer
                                     unsigned int offset,
                                     const uint8_t *sourceData) override;
 
-    virtual gl::Error getSpaceRequired(const gl::VertexAttribute &attrib, GLsizei count, GLsizei instances,
-                                       unsigned int *outSpaceRequired) const;
+    unsigned int getBufferSize() const override;
+    gl::Error setBufferSize(unsigned int size) override;
+    gl::Error discard() override;
 
-    virtual unsigned int getBufferSize() const;
-    virtual gl::Error setBufferSize(unsigned int size);
-    virtual gl::Error discard();
-
-    virtual void hintUnmapResource();
+    void hintUnmapResource() override;
 
     ID3D11Buffer *getBuffer() const;
 
@@ -56,6 +53,6 @@ class VertexBuffer11 : public VertexBuffer
     uint8_t *mMappedResourceData;
 };
 
-}
+}  // namespace rx
 
 #endif // LIBANGLE_RENDERER_D3D_D3D11_VERTEXBUFFER11_H_

src/libANGLE/renderer/d3d/d3d9/Renderer9.cpp

@@ -2752,6 +2752,38 @@ GLenum Renderer9::getVertexComponentType(gl::VertexFormatType vertexFormatType)
     return d3d9::GetVertexFormatInfo(getCapsDeclTypes(), vertexFormatType).componentType;
 }
 
+gl::ErrorOrResult<unsigned int> Renderer9::getVertexSpaceRequired(const gl::VertexAttribute &attrib,
+                                                                  GLsizei count,
+                                                                  GLsizei instances) const
+{
+    gl::VertexFormatType vertexFormatType = gl::GetVertexFormatType(attrib, GL_FLOAT);
+    const d3d9::VertexFormat &d3d9VertexInfo =
+        d3d9::GetVertexFormatInfo(getCapsDeclTypes(), vertexFormatType);
+
+    if (!attrib.enabled)
+    {
+        return 16u;
+    }
+
+    unsigned int elementCount = 0;
+    if (instances == 0 || attrib.divisor == 0)
+    {
+        elementCount = static_cast<unsigned int>(count);
+    }
+    else
+    {
+        // Round up to divisor, if possible
+        elementCount = UnsignedCeilDivide(static_cast<unsigned int>(instances), attrib.divisor);
+    }
+
+    if (d3d9VertexInfo.outputElementSize > std::numeric_limits<unsigned int>::max() / elementCount)
+    {
+        return gl::Error(GL_OUT_OF_MEMORY, "New vertex buffer size would result in an overflow.");
+    }
+
+    return static_cast<unsigned int>(d3d9VertexInfo.outputElementSize) * elementCount;
+}
+
 void Renderer9::generateCaps(gl::Caps *outCaps,
                              gl::TextureCapsMap *outTextureCaps,
                              gl::Extensions *outExtensions,

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

@@ -252,6 +252,9 @@ class Renderer9 : public RendererD3D
     bool getLUID(LUID *adapterLuid) const override;
     VertexConversionType getVertexConversionType(gl::VertexFormatType vertexFormatType) const override;
     GLenum getVertexComponentType(gl::VertexFormatType vertexFormatType) const override;
+    gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(const gl::VertexAttribute &attrib,
+                                                           GLsizei count,
+                                                           GLsizei instances) const override;
 
     gl::Error copyToRenderTarget(IDirect3DSurface9 *dest, IDirect3DSurface9 *source, bool fromManaged);
 

src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.cpp

@@ -74,15 +74,16 @@ gl::Error VertexBuffer9::storeVertexAttributes(const gl::VertexAttribute &attrib
 
     DWORD lockFlags = mDynamicUsage ? D3DLOCK_NOOVERWRITE : 0;
 
-    uint8_t *mapPtr = NULL;
+    uint8_t *mapPtr = nullptr;
 
-    unsigned int mapSize;
-    gl::Error error = spaceRequired(attrib, count, instances, &mapSize);
-    if (error.isError())
+    auto errorOrMapSize = mRenderer->getVertexSpaceRequired(attrib, count, instances);
+    if (errorOrMapSize.isError())
     {
-        return error;
+        return errorOrMapSize.getError();
     }
 
+    unsigned int mapSize = errorOrMapSize.getResult();
+
     HRESULT result = mVertexBuffer->Lock(offset, mapSize, reinterpret_cast<void**>(&mapPtr), lockFlags);
     if (FAILED(result))
     {
@@ -115,12 +116,6 @@ gl::Error VertexBuffer9::storeVertexAttributes(const gl::VertexAttribute &attrib
     return gl::Error(GL_NO_ERROR);
 }
 
-gl::Error VertexBuffer9::getSpaceRequired(const gl::VertexAttribute &attrib, GLsizei count, GLsizei instances,
-                                          unsigned int *outSpaceRequired) const
-{
-    return spaceRequired(attrib, count, instances, outSpaceRequired);
-}
-
 unsigned int VertexBuffer9::getBufferSize() const
 {
     return mBufferSize;
@@ -167,49 +162,4 @@ IDirect3DVertexBuffer9 * VertexBuffer9::getBuffer() const
 {
     return mVertexBuffer;
 }
-
-gl::Error VertexBuffer9::spaceRequired(const gl::VertexAttribute &attrib, std::size_t count, GLsizei instances,
-                                       unsigned int *outSpaceRequired) const
-{
-    gl::VertexFormatType vertexFormatType = gl::GetVertexFormatType(attrib, GL_FLOAT);
-    const d3d9::VertexFormat &d3d9VertexInfo = d3d9::GetVertexFormatInfo(mRenderer->getCapsDeclTypes(), vertexFormatType);
-
-    if (attrib.enabled)
-    {
-        unsigned int elementCount = 0;
-        if (instances == 0 || attrib.divisor == 0)
-        {
-            elementCount = static_cast<unsigned int>(count);
-        }
-        else
-        {
-            // Round up to divisor, if possible
-            elementCount = UnsignedCeilDivide(static_cast<unsigned int>(instances), attrib.divisor);
-        }
-
-        if (d3d9VertexInfo.outputElementSize <= std::numeric_limits<unsigned int>::max() / elementCount)
-        {
-            if (outSpaceRequired)
-            {
-                *outSpaceRequired =
-                    static_cast<unsigned int>(d3d9VertexInfo.outputElementSize) * elementCount;
-            }
-            return gl::Error(GL_NO_ERROR);
-        }
-        else
-        {
-            return gl::Error(GL_OUT_OF_MEMORY, "New vertex buffer size would result in an overflow.");
-        }
-    }
-    else
-    {
-        const unsigned int elementSize = 4;
-        if (outSpaceRequired)
-        {
-            *outSpaceRequired = elementSize * 4;
-        }
-        return gl::Error(GL_NO_ERROR);
-    }
-}
-
-}
+}  // namespace rx

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

@@ -19,9 +19,9 @@ class VertexBuffer9 : public VertexBuffer
 {
   public:
     explicit VertexBuffer9(Renderer9 *renderer);
-    virtual ~VertexBuffer9();
+    ~VertexBuffer9() override;
 
-    virtual gl::Error initialize(unsigned int size, bool dynamicUsage);
+    gl::Error initialize(unsigned int size, bool dynamicUsage) override;
 
     gl::Error storeVertexAttributes(const gl::VertexAttribute &attrib,
                                     GLenum currentValueType,
@@ -31,11 +31,9 @@ class VertexBuffer9 : public VertexBuffer
                                     unsigned int offset,
                                     const uint8_t *sourceData) override;
 
-    virtual gl::Error getSpaceRequired(const gl::VertexAttribute &attrib, GLsizei count, GLsizei instances, unsigned int *outSpaceRequired) const;
-
-    virtual unsigned int getBufferSize() const;
-    virtual gl::Error setBufferSize(unsigned int size);
-    virtual gl::Error discard();
+    unsigned int getBufferSize() const override;
+    gl::Error setBufferSize(unsigned int size) override;
+    gl::Error discard() override;
 
     IDirect3DVertexBuffer9 *getBuffer() const;
 
@@ -45,9 +43,6 @@ class VertexBuffer9 : public VertexBuffer
     IDirect3DVertexBuffer9 *mVertexBuffer;
     unsigned int mBufferSize;
     bool mDynamicUsage;
-
-    gl::Error spaceRequired(const gl::VertexAttribute &attrib, std::size_t count, GLsizei instances,
-                            unsigned int *outSpaceRequired) const;
 };
 
 }

src/tests/perf_tests/IndexDataManagerTest.cpp

@@ -56,6 +56,10 @@ class MockBufferFactoryD3D : public rx::BufferFactoryD3D
     MOCK_METHOD0(createVertexBuffer, rx::VertexBuffer*());
     MOCK_CONST_METHOD1(getVertexConversionType, rx::VertexConversionType(gl::VertexFormatType));
     MOCK_CONST_METHOD1(getVertexComponentType, GLenum(gl::VertexFormatType));
+    MOCK_CONST_METHOD3(getVertexSpaceRequired,
+                       gl::ErrorOrResult<unsigned int>(const gl::VertexAttribute &,
+                                                       GLsizei,
+                                                       GLsizei));
 
     // Dependency injection
     rx::IndexBuffer* createIndexBuffer() override