Vulkan: Implement Buffer updates.

src/libANGLE/renderer/vulkan/BufferVk.cpp

@@ -28,7 +28,7 @@ BufferVk::~BufferVk()
 
 void BufferVk::destroy(const gl::Context *context)
 {
-    ContextVk *contextVk = GetImplAs<ContextVk>(context);
+    ContextVk *contextVk = vk::GetImpl(context);
     RendererVk *renderer = contextVk->getRenderer();
 
     release(renderer);
@@ -46,7 +46,7 @@ gl::Error BufferVk::setData(const gl::Context *context,
                             size_t size,
                             gl::BufferUsage usage)
 {
-    ContextVk *contextVk = GetImplAs<ContextVk>(context);
+    ContextVk *contextVk = vk::GetImpl(context);
     auto device          = contextVk->getDevice();
 
     if (size > mCurrentRequiredSize)
@@ -61,7 +61,7 @@ gl::Error BufferVk::setData(const gl::Context *context,
         createInfo.pNext                 = nullptr;
         createInfo.flags                 = 0;
         createInfo.size                  = size;
-        createInfo.usage                 = VK_BUFFER_USAGE_VERTEX_BUFFER_BIT;
+        createInfo.usage = (VK_BUFFER_USAGE_VERTEX_BUFFER_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT);
         createInfo.sharingMode           = VK_SHARING_MODE_EXCLUSIVE;
         createInfo.queueFamilyIndexCount = 0;
         createInfo.pQueueFamilyIndices   = nullptr;
@@ -73,7 +73,7 @@ gl::Error BufferVk::setData(const gl::Context *context,
 
     if (data)
     {
-        ANGLE_TRY(setDataImpl(device, static_cast<const uint8_t *>(data), size, 0));
+        ANGLE_TRY(setDataImpl(contextVk, static_cast<const uint8_t *>(data), size, 0));
     }
 
     return gl::NoError();
@@ -88,9 +88,8 @@ gl::Error BufferVk::setSubData(const gl::Context *context,
     ASSERT(mBuffer.getHandle() != VK_NULL_HANDLE);
     ASSERT(mBufferMemory.getHandle() != VK_NULL_HANDLE);
 
-    VkDevice device = GetImplAs<ContextVk>(context)->getDevice();
-
-    ANGLE_TRY(setDataImpl(device, static_cast<const uint8_t *>(data), size, offset));
+    ContextVk *contextVk = vk::GetImpl(context);
+    ANGLE_TRY(setDataImpl(contextVk, static_cast<const uint8_t *>(data), size, offset));
 
     return gl::NoError();
 }
@@ -110,7 +109,7 @@ gl::Error BufferVk::map(const gl::Context *context, GLenum access, void **mapPtr
     ASSERT(mBuffer.getHandle() != VK_NULL_HANDLE);
     ASSERT(mBufferMemory.getHandle() != VK_NULL_HANDLE);
 
-    VkDevice device = GetImplAs<ContextVk>(context)->getDevice();
+    VkDevice device = vk::GetImpl(context)->getDevice();
 
     ANGLE_TRY(
         mBufferMemory.map(device, 0, mState.getSize(), 0, reinterpret_cast<uint8_t **>(mapPtr)));
@@ -127,7 +126,7 @@ gl::Error BufferVk::mapRange(const gl::Context *context,
     ASSERT(mBuffer.getHandle() != VK_NULL_HANDLE);
     ASSERT(mBufferMemory.getHandle() != VK_NULL_HANDLE);
 
-    VkDevice device = GetImplAs<ContextVk>(context)->getDevice();
+    VkDevice device = vk::GetImpl(context)->getDevice();
 
     ANGLE_TRY(mBufferMemory.map(device, offset, length, 0, reinterpret_cast<uint8_t **>(mapPtr)));
 
@@ -139,7 +138,7 @@ gl::Error BufferVk::unmap(const gl::Context *context, GLboolean *result)
     ASSERT(mBuffer.getHandle() != VK_NULL_HANDLE);
     ASSERT(mBufferMemory.getHandle() != VK_NULL_HANDLE);
 
-    VkDevice device = GetImplAs<ContextVk>(context)->getDevice();
+    VkDevice device = vk::GetImpl(context)->getDevice();
 
     mBufferMemory.unmap(device);
 
@@ -153,7 +152,7 @@ gl::Error BufferVk::getIndexRange(const gl::Context *context,
                                   bool primitiveRestartEnabled,
                                   gl::IndexRange *outRange)
 {
-    VkDevice device = GetImplAs<ContextVk>(context)->getDevice();
+    VkDevice device = vk::GetImpl(context)->getDevice();
 
     // TODO(jmadill): Consider keeping a shadow system memory copy in some cases.
     ASSERT(mBuffer.valid());
@@ -168,15 +167,67 @@ gl::Error BufferVk::getIndexRange(const gl::Context *context,
     return gl::NoError();
 }
 
-vk::Error BufferVk::setDataImpl(VkDevice device, const uint8_t *data, size_t size, size_t offset)
+vk::Error BufferVk::setDataImpl(ContextVk *contextVk,
+                                const uint8_t *data,
+                                size_t size,
+                                size_t offset)
 {
-    uint8_t *mapPointer = nullptr;
-    ANGLE_TRY(mBufferMemory.map(device, offset, size, 0, &mapPointer));
-    ASSERT(mapPointer);
+    RendererVk *renderer = contextVk->getRenderer();
+    VkDevice device      = contextVk->getDevice();
 
-    memcpy(mapPointer, data, size);
+    // Use map when available.
+    if (renderer->isSerialInUse(getQueueSerial()))
+    {
+        vk::StagingBuffer stagingBuffer;
+        ANGLE_TRY(stagingBuffer.init(contextVk, static_cast<VkDeviceSize>(size),
+                                     vk::StagingUsage::Write));
+
+        uint8_t *mapPointer = nullptr;
+        ANGLE_TRY(stagingBuffer.getDeviceMemory().map(device, 0, size, 0, &mapPointer));
+        ASSERT(mapPointer);
+
+        memcpy(mapPointer, data, size);
+        stagingBuffer.getDeviceMemory().unmap(device);
+
+        // Enqueue a copy command on the GPU.
+        // TODO(jmadill): Command re-ordering for render passes.
+        renderer->endRenderPass();
+
+        vk::CommandBuffer *commandBuffer = nullptr;
+        ANGLE_TRY(renderer->getStartedCommandBuffer(&commandBuffer));
+
+        // Insert a barrier to ensure reads from the buffer are complete.
+        // TODO(jmadill): Insert minimal barriers.
+        VkBufferMemoryBarrier bufferBarrier;
+        bufferBarrier.sType               = VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER;
+        bufferBarrier.pNext               = nullptr;
+        bufferBarrier.srcAccessMask       = VK_ACCESS_MEMORY_READ_BIT;
+        bufferBarrier.dstAccessMask       = VK_ACCESS_TRANSFER_WRITE_BIT;
+        bufferBarrier.srcQueueFamilyIndex = 0;
+        bufferBarrier.dstQueueFamilyIndex = 0;
+        bufferBarrier.buffer              = mBuffer.getHandle();
+        bufferBarrier.offset              = offset;
+        bufferBarrier.size                = static_cast<VkDeviceSize>(size);
+
+        commandBuffer->singleBufferBarrier(VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
+                                           VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT, 0, bufferBarrier);
+
+        VkBufferCopy copyRegion = {offset, 0, size};
+        commandBuffer->copyBuffer(stagingBuffer.getBuffer(), mBuffer, 1, &copyRegion);
+
+        setQueueSerial(renderer->getCurrentQueueSerial());
+        renderer->releaseObject(getQueueSerial(), &stagingBuffer);
+    }
+    else
+    {
+        uint8_t *mapPointer = nullptr;
+        ANGLE_TRY(mBufferMemory.map(device, offset, size, 0, &mapPointer));
+        ASSERT(mapPointer);
 
-    mBufferMemory.unmap(device);
+        memcpy(mapPointer, data, size);
+
+        mBufferMemory.unmap(device);
+    }
 
     return vk::NoError();
 }

src/libANGLE/renderer/vulkan/BufferVk.h

@@ -57,7 +57,7 @@ class BufferVk : public BufferImpl, public ResourceVk
     const vk::Buffer &getVkBuffer() const;
 
   private:
-    vk::Error setDataImpl(VkDevice device, const uint8_t *data, size_t size, size_t offset);
+    vk::Error setDataImpl(ContextVk *contextVk, const uint8_t *data, size_t size, size_t offset);
     void release(RendererVk *renderer);
 
     vk::Buffer mBuffer;

src/libANGLE/renderer/vulkan/RendererVk.cpp

@@ -824,4 +824,14 @@ void RendererVk::onReleaseRenderPass(const FramebufferVk *framebufferVk)
     }
 }
 
+bool RendererVk::isResourceInUse(const ResourceVk &resource)
+{
+    return isSerialInUse(resource.getQueueSerial());
+}
+
+bool RendererVk::isSerialInUse(Serial serial)
+{
+    return serial > mLastCompletedQueueSerial;
+}
+
 }  // namespace rx

src/libANGLE/renderer/vulkan/RendererVk.h

@@ -75,6 +75,9 @@ class RendererVk : angle::NonCopyable
 
     Serial getCurrentQueueSerial() const;
 
+    bool isResourceInUse(const ResourceVk &resource);
+    bool isSerialInUse(Serial serial);
+
     template <typename T>
     void releaseResource(const ResourceVk &resource, T *object)
     {
@@ -85,7 +88,7 @@ class RendererVk : angle::NonCopyable
     template <typename T>
     void releaseObject(Serial resourceSerial, T *object)
     {
-        if (resourceSerial <= mLastCompletedQueueSerial)
+        if (!isSerialInUse(resourceSerial))
         {
             object->destroy(mDevice);
         }

src/libANGLE/renderer/vulkan/renderervk_utils.cpp

@@ -72,7 +72,7 @@ VkAccessFlags GetBasicLayoutAccessFlags(VkImageLayout layout)
     }
 }
 
-VkImageUsageFlags GetImageUsageFlags(vk::StagingUsage usage)
+VkImageUsageFlags GetStagingImageUsageFlags(vk::StagingUsage usage)
 {
     switch (usage)
     {
@@ -88,6 +88,22 @@ VkImageUsageFlags GetImageUsageFlags(vk::StagingUsage usage)
     }
 }
 
+VkImageUsageFlags GetStagingBufferUsageFlags(vk::StagingUsage usage)
+{
+    switch (usage)
+    {
+        case vk::StagingUsage::Read:
+            return VK_BUFFER_USAGE_TRANSFER_DST_BIT;
+        case vk::StagingUsage::Write:
+            return VK_BUFFER_USAGE_TRANSFER_SRC_BIT;
+        case vk::StagingUsage::Both:
+            return (VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_TRANSFER_SRC_BIT);
+        default:
+            UNREACHABLE();
+            return 0;
+    }
+}
+
 }  // anonymous namespace
 
 // Mirrors std_validation_str in loader.h
@@ -331,6 +347,16 @@ void CommandBuffer::singleImageBarrier(VkPipelineStageFlags srcStageMask,
                          nullptr, 1, &imageMemoryBarrier);
 }
 
+void CommandBuffer::singleBufferBarrier(VkPipelineStageFlags srcStageMask,
+                                        VkPipelineStageFlags dstStageMask,
+                                        VkDependencyFlags dependencyFlags,
+                                        const VkBufferMemoryBarrier &bufferBarrier)
+{
+    ASSERT(valid());
+    vkCmdPipelineBarrier(mHandle, srcStageMask, dstStageMask, dependencyFlags, 0, nullptr, 1,
+                         &bufferBarrier, 0, nullptr);
+}
+
 void CommandBuffer::destroy(VkDevice device)
 {
     if (valid())
@@ -341,6 +367,16 @@ void CommandBuffer::destroy(VkDevice device)
     }
 }
 
+void CommandBuffer::copyBuffer(const vk::Buffer &srcBuffer,
+                               const vk::Buffer &destBuffer,
+                               uint32_t regionCount,
+                               const VkBufferCopy *regions)
+{
+    ASSERT(valid());
+    ASSERT(srcBuffer.valid() && destBuffer.valid());
+    vkCmdCopyBuffer(mHandle, srcBuffer.getHandle(), destBuffer.getHandle(), regionCount, regions);
+}
+
 void CommandBuffer::clearSingleColorImage(const vk::Image &image, const VkClearColorValue &color)
 {
     ASSERT(valid());
@@ -763,7 +799,7 @@ Error StagingImage::init(VkDevice device,
     createInfo.arrayLayers   = 1;
     createInfo.samples       = VK_SAMPLE_COUNT_1_BIT;
     createInfo.tiling        = VK_IMAGE_TILING_LINEAR;
-    createInfo.usage                 = GetImageUsageFlags(usage);
+    createInfo.usage                 = GetStagingImageUsageFlags(usage);
     createInfo.sharingMode   = VK_SHARING_MODE_EXCLUSIVE;
     createInfo.queueFamilyIndexCount = 1;
     createInfo.pQueueFamilyIndices   = &queueFamilyIndex;
@@ -1024,6 +1060,43 @@ uint32_t MemoryProperties::findCompatibleMemoryIndex(uint32_t bitMask, uint32_t
     return std::numeric_limits<uint32_t>::max();
 }
 
+// StagingBuffer implementation.
+StagingBuffer::StagingBuffer() : mSize(0)
+{
+}
+
+void StagingBuffer::destroy(VkDevice device)
+{
+    mBuffer.destroy(device);
+    mDeviceMemory.destroy(device);
+    mSize = 0;
+}
+
+vk::Error StagingBuffer::init(ContextVk *contextVk, VkDeviceSize size, StagingUsage usage)
+{
+    VkBufferCreateInfo createInfo;
+    createInfo.sType                 = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO;
+    createInfo.pNext                 = nullptr;
+    createInfo.flags                 = 0;
+    createInfo.size                  = size;
+    createInfo.usage                 = GetStagingBufferUsageFlags(usage);
+    createInfo.sharingMode           = VK_SHARING_MODE_EXCLUSIVE;
+    createInfo.queueFamilyIndexCount = 0;
+    createInfo.pQueueFamilyIndices   = nullptr;
+
+    ANGLE_TRY(mBuffer.init(contextVk->getDevice(), createInfo));
+    ANGLE_TRY(AllocateBufferMemory(contextVk, static_cast<size_t>(size), &mBuffer, &mDeviceMemory,
+                                   &mSize));
+
+    return vk::NoError();
+}
+
+void StagingBuffer::dumpResources(Serial serial, std::vector<vk::GarbageObject> *garbageQueue)
+{
+    mBuffer.dumpResources(serial, garbageQueue);
+    mDeviceMemory.dumpResources(serial, garbageQueue);
+}
+
 Optional<uint32_t> FindMemoryType(const VkPhysicalDeviceMemoryProperties &memoryProps,
                                   const VkMemoryRequirements &requirements,
                                   uint32_t propertyFlagMask)
@@ -1041,11 +1114,11 @@ Optional<uint32_t> FindMemoryType(const VkPhysicalDeviceMemoryProperties &memory
     return Optional<uint32_t>::Invalid();
 }
 
-gl::Error AllocateBufferMemory(ContextVk *contextVk,
-                               size_t size,
-                               vk::Buffer *buffer,
-                               vk::DeviceMemory *deviceMemoryOut,
-                               size_t *requiredSizeOut)
+Error AllocateBufferMemory(ContextVk *contextVk,
+                           size_t size,
+                           Buffer *buffer,
+                           DeviceMemory *deviceMemoryOut,
+                           size_t *requiredSizeOut)
 {
     VkDevice device = contextVk->getDevice();
 
@@ -1077,7 +1150,7 @@ gl::Error AllocateBufferMemory(ContextVk *contextVk,
     ANGLE_TRY(deviceMemoryOut->allocate(device, allocInfo));
     ANGLE_TRY(buffer->bindMemory(device, *deviceMemoryOut));
 
-    return gl::NoError();
+    return NoError();
 }
 
 // GarbageObject implementation.

src/libANGLE/renderer/vulkan/renderervk_utils.h

@@ -324,8 +324,18 @@ class CommandBuffer final : public WrappedObject<CommandBuffer, VkCommandBuffer>
                             VkDependencyFlags dependencyFlags,
                             const VkImageMemoryBarrier &imageMemoryBarrier);
 
+    void singleBufferBarrier(VkPipelineStageFlags srcStageMask,
+                             VkPipelineStageFlags dstStageMask,
+                             VkDependencyFlags dependencyFlags,
+                             const VkBufferMemoryBarrier &bufferBarrier);
+
     void clearSingleColorImage(const vk::Image &image, const VkClearColorValue &color);
 
+    void copyBuffer(const vk::Buffer &srcBuffer,
+                    const vk::Buffer &destBuffer,
+                    uint32_t regionCount,
+                    const VkBufferCopy *regions);
+
     void copySingleImage(const vk::Image &srcImage,
                          const vk::Image &destImage,
                          const gl::Box &copyRegion,
@@ -466,35 +476,6 @@ enum class StagingUsage
     Both,
 };
 
-class StagingImage final : angle::NonCopyable
-{
-  public:
-    StagingImage();
-    StagingImage(StagingImage &&other);
-    void destroy(VkDevice device);
-
-    vk::Error init(VkDevice device,
-                   uint32_t queueFamilyIndex,
-                   const MemoryProperties &memoryProperties,
-                   TextureDimension dimension,
-                   VkFormat format,
-                   const gl::Extents &extent,
-                   StagingUsage usage);
-
-    Image &getImage() { return mImage; }
-    const Image &getImage() const { return mImage; }
-    DeviceMemory &getDeviceMemory() { return mDeviceMemory; }
-    const DeviceMemory &getDeviceMemory() const { return mDeviceMemory; }
-    VkDeviceSize getSize() const { return mSize; }
-
-    void dumpResources(Serial serial, std::vector<vk::GarbageObject> *mGarbage);
-
-  private:
-    Image mImage;
-    DeviceMemory mDeviceMemory;
-    VkDeviceSize mSize;
-};
-
 class Buffer final : public WrappedObject<Buffer, VkBuffer>
 {
   public:
@@ -573,6 +554,59 @@ class Fence final : public WrappedObject<Fence, VkFence>
     VkResult getStatus(VkDevice device) const;
 };
 
+// Helper class for managing a CPU/GPU transfer Image.
+class StagingImage final : angle::NonCopyable
+{
+  public:
+    StagingImage();
+    StagingImage(StagingImage &&other);
+    void destroy(VkDevice device);
+
+    vk::Error init(VkDevice device,
+                   uint32_t queueFamilyIndex,
+                   const MemoryProperties &memoryProperties,
+                   TextureDimension dimension,
+                   VkFormat format,
+                   const gl::Extents &extent,
+                   StagingUsage usage);
+
+    Image &getImage() { return mImage; }
+    const Image &getImage() const { return mImage; }
+    DeviceMemory &getDeviceMemory() { return mDeviceMemory; }
+    const DeviceMemory &getDeviceMemory() const { return mDeviceMemory; }
+    VkDeviceSize getSize() const { return mSize; }
+
+    void dumpResources(Serial serial, std::vector<vk::GarbageObject> *garbageQueue);
+
+  private:
+    Image mImage;
+    DeviceMemory mDeviceMemory;
+    VkDeviceSize mSize;
+};
+
+// Similar to StagingImage, for Buffers.
+class StagingBuffer final : angle::NonCopyable
+{
+  public:
+    StagingBuffer();
+    void destroy(VkDevice device);
+
+    vk::Error init(ContextVk *contextVk, VkDeviceSize size, StagingUsage usage);
+
+    Buffer &getBuffer() { return mBuffer; }
+    const Buffer &getBuffer() const { return mBuffer; }
+    DeviceMemory &getDeviceMemory() { return mDeviceMemory; }
+    const DeviceMemory &getDeviceMemory() const { return mDeviceMemory; }
+    size_t getSize() const { return mSize; }
+
+    void dumpResources(Serial serial, std::vector<vk::GarbageObject> *garbageQueue);
+
+  private:
+    Buffer mBuffer;
+    DeviceMemory mDeviceMemory;
+    size_t mSize;
+};
+
 template <typename ObjT>
 class ObjectAndSerial final : angle::NonCopyable
 {
@@ -611,11 +645,11 @@ Optional<uint32_t> FindMemoryType(const VkPhysicalDeviceMemoryProperties &memory
                                   const VkMemoryRequirements &requirements,
                                   uint32_t propertyFlagMask);
 
-gl::Error AllocateBufferMemory(ContextVk *contextVk,
-                               size_t size,
-                               vk::Buffer *buffer,
-                               vk::DeviceMemory *deviceMemoryOut,
-                               size_t *requiredSizeOut);
+Error AllocateBufferMemory(ContextVk *contextVk,
+                           size_t size,
+                           Buffer *buffer,
+                           DeviceMemory *deviceMemoryOut,
+                           size_t *requiredSizeOut);
 
 struct BufferAndMemory final : private angle::NonCopyable
 {

src/tests/gl_tests/StateChangeTest.cpp

@@ -845,6 +845,32 @@ TEST_P(SimpleStateChangeTest, RedefineBufferInUse)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Tests updating a buffer's contents while in use, without redefining it.
+TEST_P(SimpleStateChangeTest, UpdateBufferInUse)
+{
+    std::vector<GLColor> redColorData(6, GLColor::red);
+
+    GLBuffer buffer;
+    glBindBuffer(GL_ARRAY_BUFFER, buffer);
+    glBufferData(GL_ARRAY_BUFFER, sizeof(GLColor) * redColorData.size(), redColorData.data(),
+                 GL_STATIC_DRAW);
+
+    // Trigger a pull from the buffer.
+    simpleDrawWithBuffer(&buffer);
+
+    // Update the buffer that's in-flight.
+    std::vector<GLColor> greenColorData(6, GLColor::green);
+    glBufferSubData(GL_ARRAY_BUFFER, 0, sizeof(GLColor) * greenColorData.size(),
+                    greenColorData.data());
+
+    // Trigger the flush and verify the first draw worked.
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+
+    // Draw again and verify the new data is correct.
+    simpleDrawWithBuffer(&buffer);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
 // Tests that deleting an in-flight Texture does not immediately delete the resource.
 TEST_P(SimpleStateChangeTest, DeleteTextureInUse)
 {