Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -4207,6 +4207,7 @@ FILE: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/limits_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc
Expand Down
2 changes: 0 additions & 2 deletions impeller/core/device_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ std::shared_ptr<Texture> DeviceBuffer::AsTexture(
return texture;
}

void DeviceBuffer::Flush() {}

const DeviceBufferDescriptor& DeviceBuffer::GetDeviceBufferDescriptor() const {
return desc_;
}
Expand Down
2 changes: 0 additions & 2 deletions impeller/core/device_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ class DeviceBuffer : public Buffer,

BufferView AsBufferView() const;

virtual void Flush();

virtual std::shared_ptr<Texture> AsTexture(
Allocator& allocator,
const TextureDescriptor& descriptor,
Expand Down
1 change: 1 addition & 0 deletions impeller/renderer/backend/vulkan/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ impeller_component("vulkan") {
"fence_waiter_vk.h",
"formats_vk.cc",
"formats_vk.h",
"limits_vk.h",
"pass_bindings_cache.cc",
"pass_bindings_cache.h",
"pipeline_cache_vk.cc",
Expand Down
29 changes: 21 additions & 8 deletions impeller/renderer/backend/vulkan/allocator_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "impeller/core/formats.h"
#include "impeller/renderer/backend/vulkan/device_buffer_vk.h"
#include "impeller/renderer/backend/vulkan/formats_vk.h"
#include "impeller/renderer/backend/vulkan/limits_vk.h"
#include "impeller/renderer/backend/vulkan/texture_vk.h"

namespace impeller {
Expand Down Expand Up @@ -198,9 +199,9 @@ static constexpr VkMemoryPropertyFlags ToVKBufferMemoryPropertyFlags(
StorageMode mode) {
switch (mode) {
case StorageMode::kHostVisible:
// See https://github.com/flutter/flutter/issues/128556 . Some devices do
// not have support for coherent host memory so we don't request it here.
return VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT;
return VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT |
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The memory we want actually has all three flags here.

VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT |
VK_MEMORY_PROPERTY_HOST_COHERENT_BIT;
case StorageMode::kDevicePrivate:
return VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
case StorageMode::kDeviceTransient:
Expand All @@ -210,18 +211,27 @@ static constexpr VkMemoryPropertyFlags ToVKBufferMemoryPropertyFlags(
}

static VmaAllocationCreateFlags ToVmaAllocationCreateFlags(StorageMode mode,
bool is_texture) {
bool is_texture,
size_t size) {
VmaAllocationCreateFlags flags = 0;
switch (mode) {
case StorageMode::kHostVisible:
if (is_texture) {
flags |= {};
if (size >= kImageSizeThresholdForDedicatedMemoryAllocation) {
flags |= VMA_ALLOCATION_CREATE_DEDICATED_MEMORY_BIT;
} else {
flags |= {};
}
} else {
flags |= VMA_ALLOCATION_CREATE_HOST_ACCESS_RANDOM_BIT;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The random access bit was the one causing problems. We shouldn't need this since the memory is coherent for reading in the GPU and we're not using it to readback data on the CPU

flags |= VMA_ALLOCATION_CREATE_HOST_ACCESS_SEQUENTIAL_WRITE_BIT;
flags |= VMA_ALLOCATION_CREATE_MAPPED_BIT;
}
return flags;
case StorageMode::kDevicePrivate:
if (is_texture &&
size >= kImageSizeThresholdForDedicatedMemoryAllocation) {
flags |= VMA_ALLOCATION_CREATE_DEDICATED_MEMORY_BIT;
}
return flags;
case StorageMode::kDeviceTransient:
return flags;
Expand Down Expand Up @@ -261,7 +271,9 @@ class AllocatedTextureSourceVK final : public TextureSourceVK {
alloc_nfo.usage = ToVMAMemoryUsage();
alloc_nfo.preferredFlags = ToVKTextureMemoryPropertyFlags(
desc.storage_mode, supports_memoryless_textures);
alloc_nfo.flags = ToVmaAllocationCreateFlags(desc.storage_mode, true);
alloc_nfo.flags =
ToVmaAllocationCreateFlags(desc.storage_mode, /*is_texture=*/true,
desc.GetByteSizeOfBaseMipLevel());

auto create_info_native =
static_cast<vk::ImageCreateInfo::NativeType>(image_info);
Expand Down Expand Up @@ -396,7 +408,8 @@ std::shared_ptr<DeviceBuffer> AllocatorVK::OnCreateBuffer(
allocation_info.usage = ToVMAMemoryUsage();
allocation_info.preferredFlags =
ToVKBufferMemoryPropertyFlags(desc.storage_mode);
allocation_info.flags = ToVmaAllocationCreateFlags(desc.storage_mode, false);
allocation_info.flags = ToVmaAllocationCreateFlags(
desc.storage_mode, /*is_texture=*/false, desc.size);

VkBuffer buffer = {};
VmaAllocation buffer_allocation = {};
Expand Down
9 changes: 0 additions & 9 deletions impeller/renderer/backend/vulkan/device_buffer_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,9 @@ bool DeviceBufferVK::OnCopyHostBuffer(const uint8_t* source,
::memmove(dest + offset, source + source_range.offset, source_range.length);
}

// Some devices do not have support for coherent host memory and require an
// explicit flush.
::vmaFlushAllocation(allocator_, allocation_, offset, source_range.length);

return true;
}

void DeviceBufferVK::Flush() {
TRACE_EVENT0("impeller", "FlushDeviceBuffer");
::vmaFlushAllocation(allocator_, allocation_, 0, VK_WHOLE_SIZE);
}

bool DeviceBufferVK::SetLabel(const std::string& label) {
auto context = context_.lock();
if (!context || !buffer_) {
Expand Down
5 changes: 0 additions & 5 deletions impeller/renderer/backend/vulkan/device_buffer_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ class DeviceBufferVK final : public DeviceBuffer,

vk::Buffer GetBuffer() const;

// If the contents of this buffer have been written to with
// `OnGetContents`, then calling flush may be necessary if the memory is
// non-coherent.
void Flush() override;

private:
friend class AllocatorVK;

Expand Down
14 changes: 14 additions & 0 deletions impeller/renderer/backend/vulkan/limits_vk.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#pragma once

#include <stdint.h>

namespace impeller {

// Maximum size to use VMA image suballocation. Any allocation greater than or
// equal to this value will use a dedicated VkDeviceMemory.
//
// This value was taken from ANGLE.
constexpr size_t kImageSizeThresholdForDedicatedMemoryAllocation =
4 * 1024 * 1024;

} // namespace impeller
3 changes: 0 additions & 3 deletions lib/ui/painting/image_decoder_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -510,9 +510,6 @@ ImpellerAllocator::ImpellerAllocator(

std::shared_ptr<impeller::DeviceBuffer> ImpellerAllocator::GetDeviceBuffer()
const {
if (buffer_) {
buffer_->Flush();
}
return buffer_;
}

Expand Down