diff --git a/impeller/core/device_buffer.cc b/impeller/core/device_buffer.cc index d7262e72b9466..3eae7e0a5f5b0 100644 --- a/impeller/core/device_buffer.cc +++ b/impeller/core/device_buffer.cc @@ -12,6 +12,8 @@ DeviceBuffer::~DeviceBuffer() = default; void DeviceBuffer::Flush(std::optional range) const {} +void DeviceBuffer::Invalidate(std::optional range) const {} + // static BufferView DeviceBuffer::AsBufferView(std::shared_ptr buffer) { BufferView view; diff --git a/impeller/core/device_buffer.h b/impeller/core/device_buffer.h index 619069ddc7179..0306fad46e595 100644 --- a/impeller/core/device_buffer.h +++ b/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 = std::nullopt) const; + virtual void Invalidate(std::optional range = std::nullopt) const; + protected: const DeviceBufferDescriptor desc_; diff --git a/impeller/core/device_buffer_descriptor.h b/impeller/core/device_buffer_descriptor.h index d1bf458942273..6c7c0ca1d29fc 100644 --- a/impeller/core/device_buffer_descriptor.h +++ b/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 diff --git a/impeller/golden_tests/vulkan_screenshotter.mm b/impeller/golden_tests/vulkan_screenshotter.mm index c413da89990b3..a896c72f6975d 100644 --- a/impeller/golden_tests/vulkan_screenshotter.mm +++ b/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 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. diff --git a/impeller/renderer/backend/gles/buffer_bindings_gles.cc b/impeller/renderer/backend/gles/buffer_bindings_gles.cc index 9d719b3b2cf48..4067498173b9e 100644 --- a/impeller/renderer/backend/gles/buffer_bindings_gles.cc +++ b/impeller/renderer/backend/gles/buffer_bindings_gles.cc @@ -8,6 +8,7 @@ #include #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(member.type); return false; } } diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 2f90cd768dc4e..7147e3d9f16e6 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/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( 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 AllocatorVK::OnCreateBuffer( allocation_info.usage = ToVMAMemoryUsage(); allocation_info.preferredFlags = static_cast( 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; } diff --git a/impeller/renderer/backend/vulkan/blit_command_vk.cc b/impeller/renderer/backend/vulkan/blit_command_vk.cc index 467cada841be0..c3c2ebcf88bf1 100644 --- a/impeller/renderer/backend/vulkan/blit_command_vk.cc +++ b/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; } diff --git a/impeller/renderer/backend/vulkan/device_buffer_vk.cc b/impeller/renderer/backend/vulkan/device_buffer_vk.cc index 1019aa363ea70..b31b28af20d0f 100644 --- a/impeller/renderer/backend/vulkan/device_buffer_vk.cc +++ b/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) const { flush_range.length); } +void DeviceBufferVK::Invalidate(std::optional 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); diff --git a/impeller/renderer/backend/vulkan/device_buffer_vk.h b/impeller/renderer/backend/vulkan/device_buffer_vk.h index e5603c9b9051b..744453dc2ac33 100644 --- a/impeller/renderer/backend/vulkan/device_buffer_vk.h +++ b/impeller/renderer/backend/vulkan/device_buffer_vk.h @@ -69,6 +69,9 @@ class DeviceBufferVK final : public DeviceBuffer, // |DeviceBuffer| void Flush(std::optional range) const override; + // |DeviceBuffer| + void Invalidate(std::optional range) const override; + DeviceBufferVK(const DeviceBufferVK&) = delete; DeviceBufferVK& operator=(const DeviceBufferVK&) = delete; diff --git a/lib/ui/painting/image_encoding_impeller.cc b/lib/ui/painting/image_encoding_impeller.cc index e2e7cf9e9642d..6eb3c201d2944 100644 --- a/lib/ui/painting/image_encoding_impeller.cc +++ b/lib/ui/painting/image_encoding_impeller.cc @@ -34,11 +34,8 @@ sk_sp ConvertBufferToSkImage( const std::shared_ptr& 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); };