[Impeller] various StC fixes for GPU -> CPU readback plus BufferBindingGLES error

impeller/core/device_buffer.cc

@@ -12,6 +12,8 @@ DeviceBuffer::~DeviceBuffer() = default;
 
 void DeviceBuffer::Flush(std::optional<Range> range) const {}
 
+void DeviceBuffer::Invalidate(std::optional<Range> range) const {}
+
 // static
 BufferView DeviceBuffer::AsBufferView(std::shared_ptr<DeviceBuffer> buffer) {
   BufferView view;

impeller/core/device_buffer.h

@@ -49,6 +49,8 @@ class DeviceBuffer {
   /// If the range is not provided, the entire buffer is flushed.
   virtual void Flush(std::optional<Range> range = std::nullopt) const;
 
+  virtual void Invalidate(std::optional<Range> range = std::nullopt) const;
+
  protected:
   const DeviceBufferDescriptor desc_;
 

impeller/core/device_buffer_descriptor.h

@@ -14,6 +14,9 @@ namespace impeller {
 struct DeviceBufferDescriptor {
   StorageMode storage_mode = StorageMode::kDeviceTransient;
   size_t size = 0u;
+  // Perhaps we could combine this with storage mode and create appropriate
+  // host-write and host-read flags.
+  bool readback = false;
 };
 
 }  // namespace impeller

impeller/golden_tests/vulkan_screenshotter.mm

@@ -29,6 +29,7 @@
   buffer_desc.storage_mode = StorageMode::kHostVisible;
   buffer_desc.size =
       texture->GetTextureDescriptor().GetByteSizeOfBaseMipLevel();
+  buffer_desc.readback = true;
   std::shared_ptr<DeviceBuffer> device_buffer =
       surface_context->GetResourceAllocator()->CreateBuffer(buffer_desc);
   FML_CHECK(device_buffer);
@@ -52,6 +53,7 @@
           .ok();
   FML_CHECK(success);
   latch.Wait();
+  device_buffer->Invalidate();
 
   // TODO(gaaclarke): Replace CoreImage requirement with something
   // crossplatform.

impeller/renderer/backend/gles/buffer_bindings_gles.cc

@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "impeller/base/validation.h"
+#include "impeller/core/shader_types.h"
 #include "impeller/renderer/backend/gles/device_buffer_gles.h"
 #include "impeller/renderer/backend/gles/formats_gles.h"
 #include "impeller/renderer/backend/gles/sampler_gles.h"
@@ -269,7 +270,7 @@ bool BufferBindingsGLES::BindUniformBuffer(const ProcTableGLES& gl,
     const auto& member = metadata->members[i];
     auto location = locations[i];
     // Void type or inactive uniform.
-    if (location == -1) {
+    if (location == -1 || member.type == ShaderType::kVoid) {
       continue;
     }
 
@@ -351,7 +352,7 @@ bool BufferBindingsGLES::BindUniformBuffer(const ProcTableGLES& gl,
       case ShaderType::kSampledImage:
       case ShaderType::kSampler:
         VALIDATION_LOG << "Could not bind uniform buffer data for key: "
-                       << member.name;
+                       << member.name << " : " << static_cast<int>(member.type);
         return false;
     }
   }

impeller/renderer/backend/vulkan/allocator_vk.cc

@@ -30,16 +30,23 @@ ToVKBufferMemoryPropertyFlags(StorageMode mode) {
 }
 
 static VmaAllocationCreateFlags ToVmaAllocationBufferCreateFlags(
-    StorageMode mode) {
+    StorageMode mode,
+    bool readback) {
   VmaAllocationCreateFlags flags = 0;
   switch (mode) {
     case StorageMode::kHostVisible:
-      flags |= VMA_ALLOCATION_CREATE_HOST_ACCESS_SEQUENTIAL_WRITE_BIT;
+      if (!readback) {
+        flags |= VMA_ALLOCATION_CREATE_HOST_ACCESS_SEQUENTIAL_WRITE_BIT;
+      } else {
+        flags |= VMA_ALLOCATION_CREATE_HOST_ACCESS_RANDOM_BIT;
+      }
       flags |= VMA_ALLOCATION_CREATE_MAPPED_BIT;
       return flags;
     case StorageMode::kDevicePrivate:
+      FML_DCHECK(!readback);
       return flags;
     case StorageMode::kDeviceTransient:
+      FML_DCHECK(!readback);
       return flags;
   }
   FML_UNREACHABLE();
@@ -62,8 +69,8 @@ static PoolVMA CreateBufferPool(VmaAllocator allocator) {
   allocation_info.usage = VMA_MEMORY_USAGE_AUTO;
   allocation_info.preferredFlags = static_cast<VkMemoryPropertyFlags>(
       ToVKBufferMemoryPropertyFlags(StorageMode::kHostVisible));
-  allocation_info.flags =
-      ToVmaAllocationBufferCreateFlags(StorageMode::kHostVisible);
+  allocation_info.flags = ToVmaAllocationBufferCreateFlags(
+      StorageMode::kHostVisible, /*readback=*/false);
 
   uint32_t memTypeIndex;
   auto result = vk::Result{vmaFindMemoryTypeIndexForBufferInfo(
@@ -455,8 +462,10 @@ std::shared_ptr<DeviceBuffer> AllocatorVK::OnCreateBuffer(
   allocation_info.usage = ToVMAMemoryUsage();
   allocation_info.preferredFlags = static_cast<VkMemoryPropertyFlags>(
       ToVKBufferMemoryPropertyFlags(desc.storage_mode));
-  allocation_info.flags = ToVmaAllocationBufferCreateFlags(desc.storage_mode);
-  if (created_buffer_pool_ && desc.storage_mode == StorageMode::kHostVisible) {
+  allocation_info.flags =
+      ToVmaAllocationBufferCreateFlags(desc.storage_mode, desc.readback);
+  if (created_buffer_pool_ && desc.storage_mode == StorageMode::kHostVisible &&
+      !desc.readback) {
     allocation_info.pool = staging_buffer_pool_.get().pool;
   }
 

impeller/renderer/backend/vulkan/blit_command_vk.cc

@@ -159,6 +159,18 @@ bool BlitCopyTextureToBufferCommandVK::Encode(CommandEncoderVK& encoder) const {
                                image_copy           //
   );
 
+  // If the buffer is used for readback, then apply a transfer -> host memory
+  // barrier.
+  if (destination->GetDeviceBufferDescriptor().readback) {
+    vk::MemoryBarrier barrier;
+    barrier.srcAccessMask = vk::AccessFlagBits::eTransferWrite;
+    barrier.dstAccessMask = vk::AccessFlagBits::eHostRead;
+
+    cmd_buffer.pipelineBarrier(vk::PipelineStageFlagBits::eTransfer,
+                               vk::PipelineStageFlagBits::eHost, {}, 1,
+                               &barrier, 0, {}, 0, {});
+  }
+
   return true;
 }
 

impeller/renderer/backend/vulkan/device_buffer_vk.cc

@@ -5,6 +5,7 @@
 #include "impeller/renderer/backend/vulkan/device_buffer_vk.h"
 
 #include "flutter/fml/trace_event.h"
+#include "flutter_vma/flutter_vma.h"
 #include "impeller/renderer/backend/vulkan/context_vk.h"
 #include "vulkan/vulkan_core.h"
 
@@ -70,6 +71,13 @@ void DeviceBufferVK::Flush(std::optional<Range> range) const {
                        flush_range.length);
 }
 
+void DeviceBufferVK::Invalidate(std::optional<Range> range) const {
+  auto flush_range = range.value_or(Range{0, GetDeviceBufferDescriptor().size});
+  ::vmaInvalidateAllocation(resource_->buffer.get().allocator,
+                            resource_->buffer.get().allocation,
+                            flush_range.offset, flush_range.length);
+}
+
 bool DeviceBufferVK::SetLabel(const std::string& label, Range range) {
   // We do not have the ability to name ranges. Just name the whole thing.
   return SetLabel(label);

impeller/renderer/backend/vulkan/device_buffer_vk.h

@@ -69,6 +69,9 @@ class DeviceBufferVK final : public DeviceBuffer,
   // |DeviceBuffer|
   void Flush(std::optional<Range> range) const override;
 
+  // |DeviceBuffer|
+  void Invalidate(std::optional<Range> range) const override;
+
   DeviceBufferVK(const DeviceBufferVK&) = delete;
 
   DeviceBufferVK& operator=(const DeviceBufferVK&) = delete;

lib/ui/painting/image_encoding_impeller.cc

@@ -34,11 +34,8 @@ sk_sp<SkImage> ConvertBufferToSkImage(
     const std::shared_ptr<impeller::DeviceBuffer>& buffer,
     SkColorType color_type,
     SkISize dimensions) {
-  auto buffer_view = impeller::DeviceBuffer::AsBufferView(buffer);
-
   SkImageInfo image_info = SkImageInfo::Make(dimensions, color_type,
                                              SkAlphaType::kPremul_SkAlphaType);
-
   SkBitmap bitmap;
   auto func = [](void* addr, void* context) {
     auto buffer =
@@ -157,6 +154,7 @@ void ImageEncodingImpeller::ConvertDlImageToSkImage(
 
   impeller::DeviceBufferDescriptor buffer_desc;
   buffer_desc.storage_mode = impeller::StorageMode::kHostVisible;
+  buffer_desc.readback = true;  // set to false for testing.
   buffer_desc.size =
       texture->GetTextureDescriptor().GetByteSizeOfBaseMipLevel();
   auto buffer =
@@ -174,6 +172,7 @@ void ImageEncodingImpeller::ConvertDlImageToSkImage(
       encode_task(fml::Status(fml::StatusCode::kUnknown, ""));
       return;
     }
+    buffer->Invalidate();
     auto sk_image = ConvertBufferToSkImage(buffer, color_type, dimensions);
     encode_task(sk_image);
   };