remove VertexBuffer abstraction. Rely on ParsedVertexBuffer for input layout and buffer metadata

Author: EmilioLaiso

include/API/Device.h

@@ -19,7 +19,6 @@
 #include "API/Capabilities.h"
 #include "API/CommandBuffer.h"
 #include "API/Texture.h"
-#include "API/VertexBuffer.h"
 #include "Support/Pipeline.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -119,10 +118,6 @@ createRenderTargetFromCPUBuffer(Device &Dev, const CPUBuffer &Buf);
 llvm::Expected<std::shared_ptr<Texture>>
 createDefaultDepthStencilTarget(Device &Dev, uint32_t Width, uint32_t Height);
 
-// Creates a VertexBuffer from a ParsedVertexBuffer.
-llvm::Expected<VertexBuffer> createVertexBuffer(Device &Dev,
-                                                const ParsedVertexBuffer &PVB);
-
 } // namespace offloadtest
 
 #endif // OFFLOADTEST_API_DEVICE_H

include/API/VertexBuffer.h

@@ -16,9 +16,11 @@
 #include "API/Resources.h"
 
 #include "llvm/ADT/SmallVector.h"
+
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/YAMLTraits.h"
+#include <cassert>
 #include <limits>
 #include <memory>
 #include <string>
@@ -372,6 +374,15 @@ struct ParsedVertexBuffer {
     return Stride;
   }
 
+  // Returns the byte offset of the stream at the given index.
+  uint32_t getOffset(uint32_t Index) const {
+    assert(Index < Streams.size() && "Stream index out of bounds");
+    uint32_t Offset = 0;
+    for (uint32_t I = 0; I < Index; ++I)
+      Offset += getFormatSizeInBytes(Streams[I].Fmt);
+    return Offset;
+  }
+
   uint32_t getVertexCount() const {
     uint32_t Stride = getStride();
     if (Stride == 0)

include/Support/Pipeline.h

@@ -34,7 +34,6 @@
 #include "API/Capabilities.h"
 #include "API/Device.h"
 #include "API/FormatConversion.h"
-#include "API/VertexBuffer.h"
 #include "DXFeatures.h"
 #include "Support/Pipeline.h"
 #include "Support/WinError.h"
@@ -486,7 +485,7 @@ class DXDevice : public offloadtest::Device {
     std::shared_ptr<DXTexture> RT;
     std::shared_ptr<DXBuffer> RTReadback;
     std::shared_ptr<DXTexture> DS;
-    std::optional<offloadtest::VertexBuffer> VB;
+    std::shared_ptr<DXBuffer> VB;
 
     llvm::SmallVector<DescriptorTable> DescTables;
     llvm::SmallVector<ResourcePair> RootResources;
@@ -1615,26 +1614,29 @@ class DXDevice : public offloadtest::Device {
           "No vertex buffer bound for graphics pipeline.");
 
     const ParsedVertexBuffer &PVB = *P.Bindings.VertexBufferPtr;
-    auto VBOrErr = offloadtest::createVertexBuffer(*this, PVB);
-    if (!VBOrErr)
-      return VBOrErr.takeError();
-    IS.VB = std::move(*VBOrErr);
+
+    BufferCreateDesc BufDesc = {};
+    BufDesc.Location = MemoryLocation::CpuToGpu;
+    BufDesc.Usage = BufferUsage::VertexBuffer;
+    auto BufOrErr = createBuffer("VertexBuffer", BufDesc, PVB.InterleavedSize);
+    if (!BufOrErr)
+      return BufOrErr.takeError();
+    IS.VB = std::static_pointer_cast<DXBuffer>(*BufOrErr);
 
     // TODO: Currently uses a single CpuToGpu mapped buffer. For optimal GPU
     // performance on discrete GPUs, use a staging buffer + copy to a GpuOnly
     // vertex buffer instead.
-    auto *DXBuf = static_cast<DXBuffer *>(IS.VB->Data.get());
     void *Ptr = nullptr;
-    if (auto Err = HR::toError(DXBuf->Buffer->Map(0, nullptr, &Ptr),
+    if (auto Err = HR::toError(IS.VB->Buffer->Map(0, nullptr, &Ptr),
                                "Failed to map vertex buffer"))
       return Err;
-    memcpy(Ptr, PVB.InterleavedData.get(), IS.VB->Data->getSizeInBytes());
-    DXBuf->Buffer->Unmap(0, nullptr);
+    memcpy(Ptr, PVB.InterleavedData.get(), IS.VB->getSizeInBytes());
+    IS.VB->Buffer->Unmap(0, nullptr);
 
     D3D12_VERTEX_BUFFER_VIEW VBView = {};
-    VBView.BufferLocation = DXBuf->Buffer->GetGPUVirtualAddress();
-    VBView.SizeInBytes = static_cast<UINT>(IS.VB->Data->getSizeInBytes());
-    VBView.StrideInBytes = IS.VB->Desc.getStride();
+    VBView.BufferLocation = IS.VB->Buffer->GetGPUVirtualAddress();
+    VBView.SizeInBytes = static_cast<UINT>(IS.VB->getSizeInBytes());
+    VBView.StrideInBytes = PVB.getStride();
 
     IS.CB->CmdList->IASetPrimitiveTopology(D3D_PRIMITIVE_TOPOLOGY_TRIANGLELIST);
     IS.CB->CmdList->IASetVertexBuffers(0, 1, &VBView);
@@ -1646,13 +1648,13 @@ class DXDevice : public offloadtest::Device {
     if (!IS.VB)
       return llvm::createStringError(std::errc::invalid_argument,
                                      "Vertex buffer not initialized.");
-    // Create the input layout from the vertex buffer description.
+    // Create the input layout from the parsed vertex buffer streams.
+    const ParsedVertexBuffer &PVB = *P.Bindings.VertexBufferPtr;
     std::vector<D3D12_INPUT_ELEMENT_DESC> InputLayout;
-    const VertexBufferDesc &VBDesc = IS.VB->Desc;
-    for (uint32_t I = 0; I < VBDesc.Streams.size(); ++I) {
-      const VertexStream &S = VBDesc.Streams[I];
+    for (uint32_t I = 0; I < PVB.Streams.size(); ++I) {
+      const VertexStreamData &S = PVB.Streams[I];
       InputLayout.push_back({S.Name.c_str(), 0, getDXGIFormat(S.Fmt), 0,
-                             VBDesc.getOffset(I),
+                             PVB.getOffset(I),
                              D3D12_INPUT_CLASSIFICATION_PER_VERTEX_DATA, 0});
     }
 
@@ -1745,7 +1747,8 @@ class DXDevice : public offloadtest::Device {
                                 static_cast<LONG>(VP.Height)};
     IS.CB->CmdList->RSSetScissorRects(1, &Scissor);
 
-    IS.CB->CmdList->DrawInstanced(IS.VB->getVertexCount(), 1, 0, 0);
+    IS.CB->CmdList->DrawInstanced(P.Bindings.VertexBufferPtr->getVertexCount(),
+                                  1, 0, 0);
 
     // Transition the render target to copy source and copy to the readback
     // buffer.

lib/API/DX/Device.cpp

@@ -11,7 +11,6 @@
 
 #include "API/Device.h"
 #include "API/FormatConversion.h"
-#include "API/VertexBuffer.h"
 
 #include "Config.h"
 
@@ -88,26 +87,6 @@ offloadtest::createRenderTargetFromCPUBuffer(Device &Dev,
   return Dev.createTexture("RenderTarget", Desc);
 }
 
-llvm::Expected<VertexBuffer>
-offloadtest::createVertexBuffer(Device &Dev, const ParsedVertexBuffer &PVB) {
-  BufferCreateDesc BufDesc = {};
-  BufDesc.Location = MemoryLocation::CpuToGpu;
-  BufDesc.Usage = BufferUsage::VertexBuffer;
-  auto BufOrErr =
-      Dev.createBuffer("VertexBuffer", BufDesc, PVB.InterleavedSize);
-  if (!BufOrErr)
-    return BufOrErr.takeError();
-
-  VertexBufferDesc VBDesc;
-  for (const auto &S : PVB.Streams)
-    VBDesc.Streams.push_back({S.Name, S.Fmt});
-
-  // TODO: Generalize VB data copy so that we can deduplicate that from each
-  // backend.
-
-  return VertexBuffer{VBDesc, *BufOrErr};
-}
-
 llvm::Expected<std::shared_ptr<Texture>>
 offloadtest::createDefaultDepthStencilTarget(Device &Dev, uint32_t Width,
                                              uint32_t Height) {

lib/API/Device.cpp

@@ -11,7 +11,6 @@
 
 #include "API/Device.h"
 #include "API/FormatConversion.h"
-#include "API/VertexBuffer.h"
 #include "MTLResources.h"
 #include "Support/Pipeline.h"
 
@@ -200,7 +199,7 @@ class MTLDevice : public offloadtest::Device {
     MTL::ComputePipelineState *ComputePipeline = nullptr;
     MTL::RenderPipelineState *RenderPipeline = nullptr;
     MTL::Buffer *ArgBuffer;
-    std::optional<offloadtest::VertexBuffer> VB;
+    std::shared_ptr<MTLBuffer> VB;
     MTL::VertexDescriptor *VertexDescriptor;
     llvm::SmallVector<MTL::Texture *> Textures;
     llvm::SmallVector<MTL::Buffer *> Buffers;
@@ -216,13 +215,13 @@ class MTLDevice : public offloadtest::Device {
     if (!IS.VB)
       return llvm::Error::success();
 
-    const VertexBufferDesc &VBDesc = IS.VB->Desc;
+    const ParsedVertexBuffer &PVB = *P.Bindings.VertexBufferPtr;
 
     NS::Array *FnAttrs = Fn->vertexAttributes();
     if (!FnAttrs)
       return llvm::createStringError(std::errc::invalid_argument,
                                      "Vertex shader has no vertex attributes.");
-    if (FnAttrs->count() != VBDesc.Streams.size())
+    if (FnAttrs->count() != PVB.Streams.size())
       return llvm::createStringError(
           std::errc::invalid_argument,
           "Mismatch between vertex shader attribute count and pipeline "
@@ -242,8 +241,8 @@ class MTLDevice : public offloadtest::Device {
     }
 
     IS.VertexDescriptor = MTL::VertexDescriptor::alloc()->init();
-    for (uint32_t I = 0; I < VBDesc.Streams.size(); ++I) {
-      const VertexStream &S = VBDesc.Streams[I];
+    for (uint32_t I = 0; I < PVB.Streams.size(); ++I) {
+      const VertexStreamData &S = PVB.Streams[I];
       llvm::SmallString<32> AttrName(S.Name);
       llvm::transform(AttrName, AttrName.begin(), tolower);
       // Append a zero since we're only supporting one attribute per name.
@@ -252,15 +251,15 @@ class MTLDevice : public offloadtest::Device {
       MTL::VertexAttributeDescriptor *VADesc =
           MTL::VertexAttributeDescriptor::alloc()->init();
       VADesc->setBufferIndex(0);
-      VADesc->setOffset(VBDesc.getOffset(I));
+      VADesc->setOffset(PVB.getOffset(I));
       VADesc->setFormat(getMetalVertexFormat(S.Fmt));
       IS.VertexDescriptor->attributes()->setObject(VADesc,
                                                    ShaderAttrIndices[AttrName]);
     }
 
     MTL::VertexBufferLayoutDescriptor *LDesc =
         MTL::VertexBufferLayoutDescriptor::alloc()->init();
-    LDesc->setStride(VBDesc.getStride());
+    LDesc->setStride(PVB.getStride());
     LDesc->setStepRate(1);
     LDesc->setStepFunction(MTL::VertexStepFunctionPerVertex);
     IS.VertexDescriptor->layouts()->setObject(LDesc, 0);
@@ -450,19 +449,23 @@ class MTLDevice : public offloadtest::Device {
             "No vertex buffer specified for graphics pipeline.");
 
       const ParsedVertexBuffer &PVB = *P.Bindings.VertexBufferPtr;
-      auto VBOrErr = offloadtest::createVertexBuffer(*this, PVB);
-      if (!VBOrErr)
-        return VBOrErr.takeError();
-      IS.VB = std::move(*VBOrErr);
+
+      BufferCreateDesc BufDesc = {};
+      BufDesc.Location = MemoryLocation::CpuToGpu;
+      BufDesc.Usage = BufferUsage::VertexBuffer;
+      auto BufOrErr =
+          createBuffer("VertexBuffer", BufDesc, PVB.InterleavedSize);
+      if (!BufOrErr)
+        return BufOrErr.takeError();
+      IS.VB = std::static_pointer_cast<MTLBuffer>(*BufOrErr);
 
       // TODO: Currently uses a single CpuToGpu mapped buffer. On discrete GPUs
       // (DX/VK), consider using a staging buffer + copy to a GpuOnly vertex
       // buffer for optimal GPU read performance. On Apple Silicon this is
       // unnecessary due to unified memory.
-      auto *MTLBuf = static_cast<MTLBuffer *>(IS.VB->Data.get());
-      const size_t BufSize = IS.VB->Data->getSizeInBytes();
-      memcpy(MTLBuf->Buf->contents(), PVB.InterleavedData.get(), BufSize);
-      MTLBuf->Buf->didModifyRange(NS::Range::Make(0, BufSize));
+      const size_t BufSize = IS.VB->getSizeInBytes();
+      memcpy(IS.VB->Buf->contents(), PVB.InterleavedData.get(), BufSize);
+      IS.VB->Buf->didModifyRange(NS::Range::Make(0, BufSize));
     }
     return llvm::Error::success();
   }
@@ -645,11 +648,10 @@ class MTLDevice : public offloadtest::Device {
     if (!IS.VB)
       return llvm::createStringError(std::errc::invalid_argument,
                                      "Vertex buffer not initialized.");
-    CmdEncoder->setVertexBuffer(
-        static_cast<MTLBuffer *>(IS.VB->Data.get())->Buf, 0, 0);
+    CmdEncoder->setVertexBuffer(IS.VB->Buf, 0, 0);
 
     CmdEncoder->drawPrimitives(MTL::PrimitiveTypeTriangle, NS::UInteger(0),
-                               IS.VB->getVertexCount());
+                               P.Bindings.VertexBufferPtr->getVertexCount());
 
     CmdEncoder->endEncoding();