From e3e37f2beeb718cb968e73270f9a6be77e9c74b8 Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Wed, 6 Mar 2024 23:52:25 +0000 Subject: [PATCH] Reverts "[Impeller] fold memory check into allocator_vk (#51187)" (flutter/engine#51243) Reverts: flutter/engine#51187 Initiated by: jonahwilliams Reason for reverting: unexpected benchmark regression https://flutter-flutter-perf.skia.org/e/?queries=device_type%3DPixel_7_Pro%26sub_result%3D90th_percentile_frame_rasterizer_time_millis%26sub_result%3D99th_percentile_frame_rasterizer_time_millis%26sub_result%3Daverage_frame_rasterizer_time_millis%26sub_result%3Dworst_frame_rasterizer_time_millis%26test%3Dnew_gallery_impeller__transition_perf&selected=commit%3D396 Original PR Author: jonahwilliams Reviewed By: {matanlurey} This change reverts the following previous change: Various cleanups to Vulkan allocator implementation: 1. Fixes https://github.com/flutter/flutter/issues/137454 2. Fold device transient cap check into allocator. 3. adds debug tracking for total memory usage in MB (a followup change needs to be made to driver to plumb it through) 4. Small cleanups to mock vulkan so an allocator can be created from it. 5. depth/stencil shouldn't be input attachments. Part of https://github.com/flutter/flutter/issues/144617 --- .../flutter/ci/licenses_golden/excluded_files | 1 - engine/src/flutter/impeller/core/allocator.h | 6 - .../backend/gles/capabilities_gles.cc | 4 + .../renderer/backend/gles/capabilities_gles.h | 3 + .../gles/test/capabilities_unittests.cc | 1 + .../renderer/backend/metal/context_mtl.mm | 1 + .../impeller/renderer/backend/vulkan/BUILD.gn | 1 - .../renderer/backend/vulkan/allocator_vk.cc | 246 ++++++++---------- .../renderer/backend/vulkan/allocator_vk.h | 28 +- .../backend/vulkan/allocator_vk_unittests.cc | 100 ------- .../backend/vulkan/capabilities_vk.cc | 20 ++ .../renderer/backend/vulkan/capabilities_vk.h | 3 + .../backend/vulkan/device_buffer_vk.cc | 21 +- .../backend/vulkan/device_buffer_vk.h | 4 +- .../backend/vulkan/surface_context_vk.cc | 1 - .../backend/vulkan/test/mock_vulkan.cc | 55 +--- .../flutter/impeller/renderer/capabilities.cc | 15 ++ .../flutter/impeller/renderer/capabilities.h | 11 + .../renderer/capabilities_unittests.cc | 1 + .../flutter/impeller/renderer/testing/mocks.h | 1 + 20 files changed, 190 insertions(+), 333 deletions(-) delete mode 100644 engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc diff --git a/engine/src/flutter/ci/licenses_golden/excluded_files b/engine/src/flutter/ci/licenses_golden/excluded_files index 80b6e69d622..b0ac79b7071 100644 --- a/engine/src/flutter/ci/licenses_golden/excluded_files +++ b/engine/src/flutter/ci/licenses_golden/excluded_files @@ -168,7 +168,6 @@ ../../../flutter/impeller/playground ../../../flutter/impeller/renderer/backend/gles/test ../../../flutter/impeller/renderer/backend/metal/texture_mtl_unittests.mm -../../../flutter/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc ../../../flutter/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc ../../../flutter/impeller/renderer/backend/vulkan/command_encoder_vk_unittests.cc ../../../flutter/impeller/renderer/backend/vulkan/command_pool_vk_unittests.cc diff --git a/engine/src/flutter/impeller/core/allocator.h b/engine/src/flutter/impeller/core/allocator.h index 38ac054d364..794de1656eb 100644 --- a/engine/src/flutter/impeller/core/allocator.h +++ b/engine/src/flutter/impeller/core/allocator.h @@ -44,12 +44,6 @@ class Allocator { virtual ISize GetMaxTextureSizeSupported() const = 0; - /// @brief Write debug memory usage information to the dart timeline in debug - /// and profile modes. - /// - /// This is only supported on the Vulkan backend. - virtual void DebugTraceMemoryStatistics() const {}; - protected: Allocator(); diff --git a/engine/src/flutter/impeller/renderer/backend/gles/capabilities_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/capabilities_gles.cc index 4a96d6e913e..a67486bd370 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/capabilities_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/capabilities_gles.cc @@ -181,6 +181,10 @@ bool CapabilitiesGLES::SupportsDecalSamplerAddressMode() const { return supports_decal_sampler_address_mode_; } +bool CapabilitiesGLES::SupportsDeviceTransientTextures() const { + return false; +} + PixelFormat CapabilitiesGLES::GetDefaultColorFormat() const { return PixelFormat::kR8G8B8A8UNormInt; } diff --git a/engine/src/flutter/impeller/renderer/backend/gles/capabilities_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/capabilities_gles.h index deb5bab6313..afcc71b418f 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/capabilities_gles.h +++ b/engine/src/flutter/impeller/renderer/backend/gles/capabilities_gles.h @@ -107,6 +107,9 @@ class CapabilitiesGLES final // |Capabilities| bool SupportsDecalSamplerAddressMode() const override; + // |Capabilities| + bool SupportsDeviceTransientTextures() const override; + // |Capabilities| PixelFormat GetDefaultColorFormat() const override; diff --git a/engine/src/flutter/impeller/renderer/backend/gles/test/capabilities_unittests.cc b/engine/src/flutter/impeller/renderer/backend/gles/test/capabilities_unittests.cc index f0c2c5aa93a..2ad20916a72 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/test/capabilities_unittests.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/test/capabilities_unittests.cc @@ -24,6 +24,7 @@ TEST(CapabilitiesGLES, CanInitializeWithDefaults) { EXPECT_FALSE(capabilities->SupportsComputeSubgroups()); EXPECT_FALSE(capabilities->SupportsReadFromResolve()); EXPECT_FALSE(capabilities->SupportsDecalSamplerAddressMode()); + EXPECT_FALSE(capabilities->SupportsDeviceTransientTextures()); EXPECT_EQ(capabilities->GetDefaultColorFormat(), PixelFormat::kR8G8B8A8UNormInt); diff --git a/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm index 1cfdc4a49da..623f3c8335f 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm @@ -66,6 +66,7 @@ static std::unique_ptr InferMetalCapabilities( .SetSupportsCompute(true) .SetSupportsComputeSubgroups(DeviceSupportsComputeSubgroups(device)) .SetSupportsReadFromResolve(true) + .SetSupportsDeviceTransientTextures(true) .SetDefaultGlyphAtlasFormat(PixelFormat::kA8UNormInt) .Build(); } diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn b/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn index 20ee2f02dad..952ae0cb26b 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn @@ -8,7 +8,6 @@ import("../../../tools/impeller.gni") impeller_component("vulkan_unittests") { testonly = true sources = [ - "allocator_vk_unittests.cc", "blit_command_vk_unittests.cc", "command_encoder_vk_unittests.cc", "command_pool_vk_unittests.cc", diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc index 38eb4b5d09c..7147e3d9f16 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -6,18 +6,18 @@ #include -#include "flutter_vma/flutter_vma.h" +#include "flutter/fml/memory/ref_ptr.h" +#include "flutter/fml/trace_event.h" #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/texture_vk.h" #include "vulkan/vulkan_enums.hpp" -#include "vulkan/vulkan_to_string.hpp" namespace impeller { -vk::Flags -AllocatorVK::ToVKBufferMemoryPropertyFlags(StorageMode mode) { +static constexpr vk::Flags +ToVKBufferMemoryPropertyFlags(StorageMode mode) { switch (mode) { case StorageMode::kHostVisible: return vk::MemoryPropertyFlagBits::eHostVisible; @@ -29,7 +29,7 @@ AllocatorVK::ToVKBufferMemoryPropertyFlags(StorageMode mode) { FML_UNREACHABLE(); } -VmaAllocationCreateFlags AllocatorVK::ToVmaAllocationBufferCreateFlags( +static VmaAllocationCreateFlags ToVmaAllocationBufferCreateFlags( StorageMode mode, bool readback) { VmaAllocationCreateFlags flags = 0; @@ -52,84 +52,6 @@ VmaAllocationCreateFlags AllocatorVK::ToVmaAllocationBufferCreateFlags( FML_UNREACHABLE(); } -vk::ImageUsageFlags AllocatorVK::ToVKImageUsageFlags( - PixelFormat format, - TextureUsageMask usage, - StorageMode mode, - bool supports_memoryless_textures) { - vk::ImageUsageFlags vk_usage; - - switch (mode) { - case StorageMode::kHostVisible: - case StorageMode::kDevicePrivate: - break; - case StorageMode::kDeviceTransient: - if (supports_memoryless_textures) { - vk_usage |= vk::ImageUsageFlagBits::eTransientAttachment; - } - break; - } - - if (usage & static_cast(TextureUsage::kRenderTarget)) { - if (PixelFormatIsDepthStencil(format)) { - vk_usage |= vk::ImageUsageFlagBits::eDepthStencilAttachment; - } else { - vk_usage |= vk::ImageUsageFlagBits::eColorAttachment; - vk_usage |= vk::ImageUsageFlagBits::eInputAttachment; - } - } - - if (usage & static_cast(TextureUsage::kShaderRead)) { - vk_usage |= vk::ImageUsageFlagBits::eSampled; - // Device transient images can only be used as attachments. The caller - // specified incorrect usage flags and is attempting to read a device - // transient image in a shader. Unset the transient attachment flag. See: - // https://github.com/flutter/flutter/issues/121633 - if (mode == StorageMode::kDeviceTransient) { - vk_usage &= ~vk::ImageUsageFlagBits::eTransientAttachment; - } - } - - if (usage & static_cast(TextureUsage::kShaderWrite)) { - vk_usage |= vk::ImageUsageFlagBits::eStorage; - // Device transient images can only be used as attachments. The caller - // specified incorrect usage flags and is attempting to read a device - // transient image in a shader. Unset the transient attachment flag. See: - // https://github.com/flutter/flutter/issues/121633 - if (mode == StorageMode::kDeviceTransient) { - vk_usage &= ~vk::ImageUsageFlagBits::eTransientAttachment; - } - } - - if (mode != StorageMode::kDeviceTransient) { - // Add transfer usage flags to support blit passes only if image isn't - // device transient. - vk_usage |= vk::ImageUsageFlagBits::eTransferSrc | - vk::ImageUsageFlagBits::eTransferDst; - } - - return vk_usage; -} - -vk::Flags -AllocatorVK::ToVKTextureMemoryPropertyFlags(StorageMode mode, - bool supports_memoryless_textures) { - switch (mode) { - case StorageMode::kHostVisible: - return vk::MemoryPropertyFlagBits::eHostVisible | - vk::MemoryPropertyFlagBits::eDeviceLocal; - case StorageMode::kDevicePrivate: - return vk::MemoryPropertyFlagBits::eDeviceLocal; - case StorageMode::kDeviceTransient: - if (supports_memoryless_textures) { - return vk::MemoryPropertyFlagBits::eLazilyAllocated | - vk::MemoryPropertyFlagBits::eDeviceLocal; - } - return vk::MemoryPropertyFlagBits::eDeviceLocal; - } - FML_UNREACHABLE(); -} - static PoolVMA CreateBufferPool(VmaAllocator allocator) { vk::BufferCreateInfo buffer_info; buffer_info.usage = vk::BufferUsageFlagBits::eVertexBuffer | @@ -146,8 +68,8 @@ static PoolVMA CreateBufferPool(VmaAllocator allocator) { VmaAllocationCreateInfo allocation_info = {}; allocation_info.usage = VMA_MEMORY_USAGE_AUTO; allocation_info.preferredFlags = static_cast( - AllocatorVK::ToVKBufferMemoryPropertyFlags(StorageMode::kHostVisible)); - allocation_info.flags = AllocatorVK::ToVmaAllocationBufferCreateFlags( + ToVKBufferMemoryPropertyFlags(StorageMode::kHostVisible)); + allocation_info.flags = ToVmaAllocationBufferCreateFlags( StorageMode::kHostVisible, /*readback=*/false); uint32_t memTypeIndex; @@ -179,7 +101,6 @@ AllocatorVK::AllocatorVK(std::weak_ptr context, auto limits = physical_device.getProperties().limits; max_texture_size_.width = max_texture_size_.height = limits.maxImageDimension2D; - physical_device.getMemoryProperties(&memory_properties_); VmaVulkanFunctions proc_table = {}; @@ -228,24 +149,11 @@ AllocatorVK::AllocatorVK(std::weak_ptr context, VALIDATION_LOG << "Could not create memory allocator"; return; } - - { - // Query texture support. - // TODO(jonahwilliams): - // https://github.com/flutter/flutter/issues/129784 - for (auto i = 0u; i < memory_properties_.memoryTypeCount; i++) { - if (memory_properties_.memoryTypes[i].propertyFlags & - vk::MemoryPropertyFlagBits::eLazilyAllocated) { - supports_memoryless_textures_ = true; - break; - } - } - } - staging_buffer_pool_.reset(CreateBufferPool(allocator)); created_buffer_pool_ &= staging_buffer_pool_.is_valid(); allocator_.reset(allocator); - supports_memoryless_textures_ = false; + supports_memoryless_textures_ = + capabilities.SupportsDeviceTransientTextures(); is_valid_ = true; } @@ -261,6 +169,101 @@ ISize AllocatorVK::GetMaxTextureSizeSupported() const { return max_texture_size_; } +static constexpr vk::ImageUsageFlags ToVKImageUsageFlags( + PixelFormat format, + TextureUsageMask usage, + StorageMode mode, + bool supports_memoryless_textures) { + vk::ImageUsageFlags vk_usage; + + switch (mode) { + case StorageMode::kHostVisible: + case StorageMode::kDevicePrivate: + break; + case StorageMode::kDeviceTransient: + if (supports_memoryless_textures) { + vk_usage |= vk::ImageUsageFlagBits::eTransientAttachment; + } + break; + } + + if (usage & static_cast(TextureUsage::kRenderTarget)) { + if (PixelFormatIsDepthStencil(format)) { + vk_usage |= vk::ImageUsageFlagBits::eDepthStencilAttachment; + } else { + vk_usage |= vk::ImageUsageFlagBits::eColorAttachment; + } + vk_usage |= vk::ImageUsageFlagBits::eInputAttachment; + } + + if (usage & static_cast(TextureUsage::kShaderRead)) { + vk_usage |= vk::ImageUsageFlagBits::eSampled; + // Device transient images can only be used as attachments. The caller + // specified incorrect usage flags and is attempting to read a device + // transient image in a shader. Unset the transient attachment flag. See: + // https://github.com/flutter/flutter/issues/121633 + if (mode == StorageMode::kDeviceTransient) { + vk_usage &= ~vk::ImageUsageFlagBits::eTransientAttachment; + } + } + + if (usage & static_cast(TextureUsage::kShaderWrite)) { + vk_usage |= vk::ImageUsageFlagBits::eStorage; + // Device transient images can only be used as attachments. The caller + // specified incorrect usage flags and is attempting to read a device + // transient image in a shader. Unset the transient attachment flag. See: + // https://github.com/flutter/flutter/issues/121633 + if (mode == StorageMode::kDeviceTransient) { + vk_usage &= ~vk::ImageUsageFlagBits::eTransientAttachment; + } + } + + if (mode != StorageMode::kDeviceTransient) { + // Add transfer usage flags to support blit passes only if image isn't + // device transient. + vk_usage |= vk::ImageUsageFlagBits::eTransferSrc | + vk::ImageUsageFlagBits::eTransferDst; + } + + return vk_usage; +} + +static constexpr VmaMemoryUsage ToVMAMemoryUsage() { + return VMA_MEMORY_USAGE_AUTO; +} + +static constexpr vk::Flags +ToVKTextureMemoryPropertyFlags(StorageMode mode, + bool supports_memoryless_textures) { + switch (mode) { + case StorageMode::kHostVisible: + return vk::MemoryPropertyFlagBits::eHostVisible | + vk::MemoryPropertyFlagBits::eDeviceLocal; + case StorageMode::kDevicePrivate: + return vk::MemoryPropertyFlagBits::eDeviceLocal; + case StorageMode::kDeviceTransient: + if (supports_memoryless_textures) { + return vk::MemoryPropertyFlagBits::eLazilyAllocated | + vk::MemoryPropertyFlagBits::eDeviceLocal; + } + return vk::MemoryPropertyFlagBits::eDeviceLocal; + } + FML_UNREACHABLE(); +} + +static VmaAllocationCreateFlags ToVmaAllocationCreateFlags(StorageMode mode) { + VmaAllocationCreateFlags flags = 0; + switch (mode) { + case StorageMode::kHostVisible: + return flags; + case StorageMode::kDevicePrivate: + return flags; + case StorageMode::kDeviceTransient: + return flags; + } + FML_UNREACHABLE(); +} + class AllocatedTextureSourceVK final : public TextureSourceVK { public: AllocatedTextureSourceVK(std::weak_ptr resource_manager, @@ -284,17 +287,18 @@ class AllocatedTextureSourceVK final : public TextureSourceVK { image_info.arrayLayers = ToArrayLayerCount(desc.type); image_info.tiling = vk::ImageTiling::eOptimal; image_info.initialLayout = vk::ImageLayout::eUndefined; - image_info.usage = AllocatorVK::ToVKImageUsageFlags( - desc.format, desc.usage, desc.storage_mode, - supports_memoryless_textures); + image_info.usage = + ToVKImageUsageFlags(desc.format, desc.usage, desc.storage_mode, + supports_memoryless_textures); image_info.sharingMode = vk::SharingMode::eExclusive; VmaAllocationCreateInfo alloc_nfo = {}; - alloc_nfo.usage = VMA_MEMORY_USAGE_AUTO; - alloc_nfo.preferredFlags = static_cast( - AllocatorVK::ToVKTextureMemoryPropertyFlags( + alloc_nfo.usage = ToVMAMemoryUsage(); + alloc_nfo.preferredFlags = + static_cast(ToVKTextureMemoryPropertyFlags( desc.storage_mode, supports_memoryless_textures)); + alloc_nfo.flags = ToVmaAllocationCreateFlags(desc.storage_mode); auto create_info_native = static_cast(image_info); @@ -455,7 +459,7 @@ std::shared_ptr AllocatorVK::OnCreateBuffer( static_cast(buffer_info); VmaAllocationCreateInfo allocation_info = {}; - allocation_info.usage = VMA_MEMORY_USAGE_AUTO; + allocation_info.usage = ToVMAMemoryUsage(); allocation_info.preferredFlags = static_cast( ToVKBufferMemoryPropertyFlags(desc.storage_mode)); allocation_info.flags = @@ -476,11 +480,6 @@ std::shared_ptr AllocatorVK::OnCreateBuffer( &buffer_allocation_info // )}; - bool is_host_coherent_buffer = - !!(memory_properties_.memoryTypes[buffer_allocation_info.memoryType] - .propertyFlags & - vk::MemoryPropertyFlagBits::eHostCoherent); - if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Unable to allocate a device buffer: " << vk::to_string(result); @@ -493,27 +492,8 @@ std::shared_ptr AllocatorVK::OnCreateBuffer( UniqueBufferVMA{BufferVMA{allocator_.get(), // buffer_allocation, // vk::Buffer{buffer}}}, // - buffer_allocation_info, // - is_host_coherent_buffer // + buffer_allocation_info // ); } -void AllocatorVK::DebugTraceMemoryStatistics() const { -#ifdef IMPELLER_DEBUG - auto count = memory_properties_.memoryHeapCount; - std::vector budgets(count); - vmaGetHeapBudgets(allocator_.get(), budgets.data()); - size_t total_usage = 0; - for (auto i = 0u; i < count; i++) { - const VmaBudget& budget = budgets[i]; - total_usage += budget.usage; - } - // Convert bytes to MB. - total_usage *= 1e-6; - FML_TRACE_COUNTER("flutter", "AllocatorVK", - reinterpret_cast(this), // Trace Counter ID - "MemoryBudgetUsageMB", total_usage); -#endif // IMPELLER_DEBUG -} - } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h index a89cda2223a..caaedfc5209 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h @@ -5,12 +5,15 @@ #ifndef FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_ALLOCATOR_VK_H_ #define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_ALLOCATOR_VK_H_ +#include "flutter/fml/macros.h" +#include "flutter/fml/memory/ref_ptr.h" #include "impeller/core/allocator.h" #include "impeller/renderer/backend/vulkan/context_vk.h" #include "impeller/renderer/backend/vulkan/device_buffer_vk.h" #include "impeller/renderer/backend/vulkan/device_holder.h" #include "impeller/renderer/backend/vulkan/vk.h" +#include #include #include @@ -21,27 +24,6 @@ class AllocatorVK final : public Allocator { // |Allocator| ~AllocatorVK() override; - // visible for testing. - static vk::Flags ToVKBufferMemoryPropertyFlags( - StorageMode mode); - - // visible for testing. - static VmaAllocationCreateFlags ToVmaAllocationBufferCreateFlags( - StorageMode mode, - bool readback); - - // visible for testing. - static vk::ImageUsageFlags ToVKImageUsageFlags( - PixelFormat format, - TextureUsageMask usage, - StorageMode mode, - bool supports_memoryless_textures); - - // visible for testing. - static vk::Flags ToVKTextureMemoryPropertyFlags( - StorageMode mode, - bool supports_memoryless_textures); - private: friend class ContextVK; @@ -54,7 +36,6 @@ class AllocatorVK final : public Allocator { bool supports_memoryless_textures_ = false; // TODO(jonahwilliams): figure out why CI can't create these buffer pools. bool created_buffer_pool_ = true; - vk::PhysicalDeviceMemoryProperties memory_properties_; AllocatorVK(std::weak_ptr context, uint32_t vulkan_api_version, @@ -77,9 +58,6 @@ class AllocatorVK final : public Allocator { // |Allocator| ISize GetMaxTextureSizeSupported() const override; - // |Allocator| - void DebugTraceMemoryStatistics() const override; - AllocatorVK(const AllocatorVK&) = delete; AllocatorVK& operator=(const AllocatorVK&) = delete; diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc deleted file mode 100644 index 580993cec58..00000000000 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc +++ /dev/null @@ -1,100 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "flutter/testing/testing.h" // IWYU pragma: keep -#include "flutter_vma/flutter_vma.h" -#include "gtest/gtest.h" -#include "impeller/core/formats.h" -#include "impeller/renderer/backend/vulkan/allocator_vk.h" -#include "third_party/vulkan-deps/vulkan-headers/src/include/vulkan/vulkan_enums.hpp" - -namespace impeller { -namespace testing { - -TEST(AllocatorVK, BufferCreateFlags) { - EXPECT_EQ(AllocatorVK::ToVmaAllocationBufferCreateFlags( - StorageMode::kDevicePrivate, /*readback=*/false), - 0u); - EXPECT_EQ(AllocatorVK::ToVmaAllocationBufferCreateFlags( - StorageMode::kDeviceTransient, /*readback=*/false), - 0u); - uint32_t expected_flags = - VMA_ALLOCATION_CREATE_HOST_ACCESS_SEQUENTIAL_WRITE_BIT | - VMA_ALLOCATION_CREATE_MAPPED_BIT; - EXPECT_EQ(AllocatorVK::ToVmaAllocationBufferCreateFlags( - StorageMode::kHostVisible, /*readback=*/false), - expected_flags); - - expected_flags = VMA_ALLOCATION_CREATE_HOST_ACCESS_RANDOM_BIT | - VMA_ALLOCATION_CREATE_MAPPED_BIT; - EXPECT_EQ(AllocatorVK::ToVmaAllocationBufferCreateFlags( - StorageMode::kHostVisible, /*readback=*/true), - expected_flags); -} - -TEST(AllocatorVK, BufferMemoryPropertyFlags) { - EXPECT_EQ( - AllocatorVK::ToVKBufferMemoryPropertyFlags(StorageMode::kDevicePrivate), - vk::MemoryPropertyFlagBits::eDeviceLocal); - EXPECT_EQ( - AllocatorVK::ToVKBufferMemoryPropertyFlags(StorageMode::kDeviceTransient), - vk::MemoryPropertyFlagBits::eLazilyAllocated); - EXPECT_EQ( - AllocatorVK::ToVKBufferMemoryPropertyFlags(StorageMode::kHostVisible), - vk::MemoryPropertyFlagBits::eHostVisible); -} - -TEST(AllocatorVK, ImageUsageFlags) { - // Color Format - EXPECT_EQ(AllocatorVK::ToVKImageUsageFlags( - PixelFormat::kR8G8B8A8UNormInt, // - static_cast(TextureUsage::kRenderTarget), // - StorageMode::kDevicePrivate, // - /*supports_memoryless_textures=*/true // - ), - vk::ImageUsageFlagBits::eColorAttachment | - vk::ImageUsageFlagBits::eInputAttachment | - vk::ImageUsageFlagBits::eTransferSrc | - vk::ImageUsageFlagBits::eTransferDst); - - EXPECT_EQ(AllocatorVK::ToVKImageUsageFlags( - PixelFormat::kR8G8B8A8UNormInt, // - static_cast(TextureUsage::kRenderTarget), // - StorageMode::kDeviceTransient, // - /*supports_memoryless_textures=*/true // - ), - vk::ImageUsageFlagBits::eColorAttachment | - vk::ImageUsageFlagBits::eInputAttachment | - vk::ImageUsageFlagBits::eTransientAttachment); - - // Depth+Stencil Format - EXPECT_EQ(AllocatorVK::ToVKImageUsageFlags( - PixelFormat::kD24UnormS8Uint, // - static_cast(TextureUsage::kRenderTarget), // - StorageMode::kDevicePrivate, // - /*supports_memoryless_textures=*/true // - ), - vk::ImageUsageFlagBits::eDepthStencilAttachment | - vk::ImageUsageFlagBits::eTransferSrc | - vk::ImageUsageFlagBits::eTransferDst); - - EXPECT_EQ(AllocatorVK::ToVKImageUsageFlags( - PixelFormat::kD24UnormS8Uint, // - static_cast(TextureUsage::kRenderTarget), // - StorageMode::kDeviceTransient, // - /*supports_memoryless_textures=*/true // - ), - vk::ImageUsageFlagBits::eDepthStencilAttachment | - vk::ImageUsageFlagBits::eTransientAttachment); -} - -TEST(AllocatorVK, TextureMemoryPropertyFlags) { - EXPECT_EQ( - AllocatorVK::ToVKTextureMemoryPropertyFlags( - StorageMode::kDevicePrivate, /*supports_memoryless_textures=*/true), - vk::MemoryPropertyFlagBits::eDeviceLocal); -} - -} // namespace testing -} // namespace impeller \ No newline at end of file diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc index c301e70b4e6..b5ad0aba23f 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -442,6 +442,21 @@ bool CapabilitiesVK::SetPhysicalDevice(const vk::PhysicalDevice& device) { .supportedOperations & vk::SubgroupFeatureFlagBits::eArithmetic); + { + // Query texture support. + // TODO(jonahwilliams): + // https://github.com/flutter/flutter/issues/129784 + vk::PhysicalDeviceMemoryProperties memory_properties; + device.getMemoryProperties(&memory_properties); + + for (auto i = 0u; i < memory_properties.memoryTypeCount; i++) { + if (memory_properties.memoryTypes[i].propertyFlags & + vk::MemoryPropertyFlagBits::eLazilyAllocated) { + supports_device_transient_textures_ = true; + } + } + } + // Determine the optional device extensions this physical device supports. { required_common_device_extensions_.clear(); @@ -528,6 +543,11 @@ bool CapabilitiesVK::SupportsDecalSamplerAddressMode() const { return true; } +// |Capabilities| +bool CapabilitiesVK::SupportsDeviceTransientTextures() const { + return supports_device_transient_textures_; +} + // |Capabilities| PixelFormat CapabilitiesVK::GetDefaultColorFormat() const { return default_color_format_; diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h index 834d6b4f6f2..f06732b8b20 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -178,6 +178,9 @@ class CapabilitiesVK final : public Capabilities, // |Capabilities| bool SupportsDecalSamplerAddressMode() const override; + // |Capabilities| + bool SupportsDeviceTransientTextures() const override; + // |Capabilities| PixelFormat GetDefaultColorFormat() const override; diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/device_buffer_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/device_buffer_vk.cc index 7d791d2d5e3..a43a5825fd5 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/device_buffer_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/device_buffer_vk.cc @@ -5,6 +5,7 @@ #include "impeller/renderer/backend/vulkan/device_buffer_vk.h" #include "flutter/flutter_vma/flutter_vma.h" +#include "flutter/fml/trace_event.h" #include "impeller/renderer/backend/vulkan/context_vk.h" #include "vulkan/vulkan_core.h" @@ -13,16 +14,14 @@ namespace impeller { DeviceBufferVK::DeviceBufferVK(DeviceBufferDescriptor desc, std::weak_ptr context, UniqueBufferVMA buffer, - VmaAllocationInfo info, - bool is_host_coherent_buffer) + VmaAllocationInfo info) : DeviceBuffer(desc), context_(std::move(context)), resource_(ContextVK::Cast(*context_.lock().get()).GetResourceManager(), BufferResource{ std::move(buffer), // info // - }), - is_host_coherent_buffer_(is_host_coherent_buffer) {} + }) {} DeviceBufferVK::~DeviceBufferVK() = default; @@ -42,11 +41,9 @@ bool DeviceBufferVK::OnCopyHostBuffer(const uint8_t* source, if (source) { ::memmove(dest + offset, source + source_range.offset, source_range.length); } - if (!is_host_coherent_buffer_) { - ::vmaFlushAllocation(resource_->buffer.get().allocator, - resource_->buffer.get().allocation, offset, - source_range.length); - } + ::vmaFlushAllocation(resource_->buffer.get().allocator, + resource_->buffer.get().allocation, offset, + source_range.length); return true; } @@ -68,9 +65,6 @@ bool DeviceBufferVK::SetLabel(const std::string& label) { } void DeviceBufferVK::Flush(std::optional range) const { - if (is_host_coherent_buffer_) { - return; - } auto flush_range = range.value_or(Range{0, GetDeviceBufferDescriptor().size}); ::vmaFlushAllocation(resource_->buffer.get().allocator, resource_->buffer.get().allocation, flush_range.offset, @@ -78,9 +72,6 @@ void DeviceBufferVK::Flush(std::optional range) const { } void DeviceBufferVK::Invalidate(std::optional range) const { - if (is_host_coherent_buffer_) { - return; - } auto flush_range = range.value_or(Range{0, GetDeviceBufferDescriptor().size}); ::vmaInvalidateAllocation(resource_->buffer.get().allocator, resource_->buffer.get().allocation, diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/device_buffer_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/device_buffer_vk.h index b7a8c48b61d..744453dc2ac 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/device_buffer_vk.h +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/device_buffer_vk.h @@ -20,8 +20,7 @@ class DeviceBufferVK final : public DeviceBuffer, DeviceBufferVK(DeviceBufferDescriptor desc, std::weak_ptr context, UniqueBufferVMA buffer, - VmaAllocationInfo info, - bool is_host_coherent_buffer); + VmaAllocationInfo info); // |DeviceBuffer| ~DeviceBufferVK() override; @@ -52,7 +51,6 @@ class DeviceBufferVK final : public DeviceBuffer, std::weak_ptr context_; UniqueResourceVKT resource_; - const bool is_host_coherent_buffer_; // |DeviceBuffer| uint8_t* OnGetContents() const override; diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/surface_context_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/surface_context_vk.cc index f0a7912433d..d3856d615c8 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/surface_context_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/surface_context_vk.cc @@ -87,7 +87,6 @@ std::unique_ptr SurfaceContextVK::AcquireNextSurface() { impeller::PipelineLibraryVK::Cast(*pipeline_library) .DidAcquireSurfaceFrame(); } - parent_->GetResourceAllocator()->DebugTraceMemoryStatistics(); parent_->GetCommandPoolRecycler()->Dispose(); return surface; } diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index 3fc577a907b..4c590620701 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -44,8 +44,6 @@ struct MockSwapchainKHR { struct MockSemaphore {}; -struct MockBuffer {}; - static ISize currentImageSize = ISize{1, 1}; class MockDevice final { @@ -223,20 +221,14 @@ VkResult vkCreateInstance(const VkInstanceCreateInfo* pCreateInfo, void vkGetPhysicalDeviceMemoryProperties( VkPhysicalDevice physicalDevice, VkPhysicalDeviceMemoryProperties* pMemoryProperties) { - pMemoryProperties->memoryTypeCount = 2; - pMemoryProperties->memoryHeapCount = 2; + pMemoryProperties->memoryTypeCount = 1; pMemoryProperties->memoryTypes[0].heapIndex = 0; - pMemoryProperties->memoryTypes[0].propertyFlags = - VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT | - VK_MEMORY_PROPERTY_HOST_COHERENT_BIT | - VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT; - pMemoryProperties->memoryTypes[1].heapIndex = 1; - pMemoryProperties->memoryTypes[1].propertyFlags = - VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT; + // pMemoryProperties->memoryTypes[0].propertyFlags = + // VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT | + // VK_MEMORY_PROPERTY_DEVICE_COHERENT_BIT_AMD; + pMemoryProperties->memoryHeapCount = 1; pMemoryProperties->memoryHeaps[0].size = 1024 * 1024 * 1024; - pMemoryProperties->memoryHeaps[0].flags = VK_MEMORY_HEAP_DEVICE_LOCAL_BIT; - pMemoryProperties->memoryHeaps[1].size = 1024 * 1024 * 1024; - pMemoryProperties->memoryHeaps[1].flags = VK_MEMORY_HEAP_DEVICE_LOCAL_BIT; + pMemoryProperties->memoryHeaps[0].flags = 0; } VkResult vkCreatePipelineCache(VkDevice device, @@ -325,18 +317,10 @@ VkResult vkCreateBuffer(VkDevice device, const VkBufferCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator, VkBuffer* pBuffer) { - *pBuffer = reinterpret_cast(new MockBuffer()); + *pBuffer = reinterpret_cast(0xDEADDEAD); return VK_SUCCESS; } -void vkDestroyBuffer(VkDevice device, - VkBuffer buffer, - const VkAllocationCallbacks* pAllocator) { - MockDevice* mock_device = reinterpret_cast(device); - mock_device->AddCalledFunction("vkDestroyBuffer"); - delete reinterpret_cast(buffer); -} - void vkGetBufferMemoryRequirements2KHR( VkDevice device, const VkBufferMemoryRequirementsInfo2* pInfo, @@ -735,25 +719,6 @@ VkResult vkAcquireNextImageKHR(VkDevice device, return VK_SUCCESS; } -VkResult vkFlushMappedMemoryRanges(VkDevice device, - uint32_t memoryRangeCount, - const VkMappedMemoryRange* pMemoryRanges) { - MockDevice* mock_device = reinterpret_cast(device); - mock_device->AddCalledFunction("vkFlushMappedMemoryRanges"); - return VK_SUCCESS; -} - -VkResult vkMapMemory(VkDevice device, - VkDeviceMemory memory, - VkDeviceSize offset, - VkDeviceSize size, - VkMemoryMapFlags flags, - void** ppData) { - MockDevice* mock_device = reinterpret_cast(device); - mock_device->AddCalledFunction("vkMapMemory"); - return VK_SUCCESS; -} - PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance, const char* pName) { if (strcmp("vkEnumerateInstanceExtensionProperties", pName) == 0) { @@ -890,12 +855,6 @@ PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance, return (PFN_vkVoidFunction)vkDestroySurfaceKHR; } else if (strcmp("vkAcquireNextImageKHR", pName) == 0) { return (PFN_vkVoidFunction)vkAcquireNextImageKHR; - } else if (strcmp("vkFlushMappedMemoryRanges", pName) == 0) { - return (PFN_vkVoidFunction)vkFlushMappedMemoryRanges; - } else if (strcmp("vkDestroyBuffer", pName) == 0) { - return (PFN_vkVoidFunction)vkDestroyBuffer; - } else if (strcmp("vkMapMemory", pName) == 0) { - return (PFN_vkVoidFunction)vkMapMemory; } return noop; } diff --git a/engine/src/flutter/impeller/renderer/capabilities.cc b/engine/src/flutter/impeller/renderer/capabilities.cc index ca6ed479d57..1ef1fe38a80 100644 --- a/engine/src/flutter/impeller/renderer/capabilities.cc +++ b/engine/src/flutter/impeller/renderer/capabilities.cc @@ -75,6 +75,11 @@ class StandardCapabilities final : public Capabilities { return default_depth_stencil_format_; } + // |Capabilities| + bool SupportsDeviceTransientTextures() const override { + return supports_device_transient_textures_; + } + // |Capabilities| PixelFormat GetDefaultGlyphAtlasFormat() const override { return default_glyph_atlas_format_; @@ -90,6 +95,7 @@ class StandardCapabilities final : public Capabilities { bool supports_compute_subgroups, bool supports_read_from_resolve, bool supports_decal_sampler_address_mode, + bool supports_device_transient_textures, PixelFormat default_color_format, PixelFormat default_stencil_format, PixelFormat default_depth_stencil_format, @@ -104,6 +110,7 @@ class StandardCapabilities final : public Capabilities { supports_read_from_resolve_(supports_read_from_resolve), supports_decal_sampler_address_mode_( supports_decal_sampler_address_mode), + supports_device_transient_textures_(supports_device_transient_textures), default_color_format_(default_color_format), default_stencil_format_(default_stencil_format), default_depth_stencil_format_(default_depth_stencil_format), @@ -120,6 +127,7 @@ class StandardCapabilities final : public Capabilities { bool supports_compute_subgroups_ = false; bool supports_read_from_resolve_ = false; bool supports_decal_sampler_address_mode_ = false; + bool supports_device_transient_textures_ = false; PixelFormat default_color_format_ = PixelFormat::kUnknown; PixelFormat default_stencil_format_ = PixelFormat::kUnknown; PixelFormat default_depth_stencil_format_ = PixelFormat::kUnknown; @@ -203,6 +211,12 @@ CapabilitiesBuilder& CapabilitiesBuilder::SetSupportsDecalSamplerAddressMode( return *this; } +CapabilitiesBuilder& CapabilitiesBuilder::SetSupportsDeviceTransientTextures( + bool value) { + supports_device_transient_textures_ = value; + return *this; +} + CapabilitiesBuilder& CapabilitiesBuilder::SetDefaultGlyphAtlasFormat( PixelFormat value) { default_glyph_atlas_format_ = value; @@ -220,6 +234,7 @@ std::unique_ptr CapabilitiesBuilder::Build() { supports_compute_subgroups_, // supports_read_from_resolve_, // supports_decal_sampler_address_mode_, // + supports_device_transient_textures_, // default_color_format_.value_or(PixelFormat::kUnknown), // default_stencil_format_.value_or(PixelFormat::kUnknown), // default_depth_stencil_format_.value_or(PixelFormat::kUnknown), // diff --git a/engine/src/flutter/impeller/renderer/capabilities.h b/engine/src/flutter/impeller/renderer/capabilities.h index 395b2259aa7..34c5839bd59 100644 --- a/engine/src/flutter/impeller/renderer/capabilities.h +++ b/engine/src/flutter/impeller/renderer/capabilities.h @@ -81,6 +81,14 @@ class Capabilities { /// @brief Whether the context backend supports `SamplerAddressMode::Decal`. virtual bool SupportsDecalSamplerAddressMode() const = 0; + /// @brief Whether the context backend supports allocating + /// `StorageMode::kDeviceTransient` (aka "memoryless") textures, which + /// are temporary textures kept in tile memory for the duration of the + /// `RenderPass` it's attached to. + /// + /// This feature is especially useful for MSAA and stencils. + virtual bool SupportsDeviceTransientTextures() const = 0; + /// @brief Returns a supported `PixelFormat` for textures that store /// 4-channel colors (red/green/blue/alpha). virtual PixelFormat GetDefaultColorFormat() const = 0; @@ -141,6 +149,8 @@ class CapabilitiesBuilder { CapabilitiesBuilder& SetSupportsDecalSamplerAddressMode(bool value); + CapabilitiesBuilder& SetSupportsDeviceTransientTextures(bool value); + CapabilitiesBuilder& SetDefaultGlyphAtlasFormat(PixelFormat value); std::unique_ptr Build(); @@ -155,6 +165,7 @@ class CapabilitiesBuilder { bool supports_compute_subgroups_ = false; bool supports_read_from_resolve_ = false; bool supports_decal_sampler_address_mode_ = false; + bool supports_device_transient_textures_ = false; std::optional default_color_format_ = std::nullopt; std::optional default_stencil_format_ = std::nullopt; std::optional default_depth_stencil_format_ = std::nullopt; diff --git a/engine/src/flutter/impeller/renderer/capabilities_unittests.cc b/engine/src/flutter/impeller/renderer/capabilities_unittests.cc index 1de8441443f..2692d067ad0 100644 --- a/engine/src/flutter/impeller/renderer/capabilities_unittests.cc +++ b/engine/src/flutter/impeller/renderer/capabilities_unittests.cc @@ -27,6 +27,7 @@ CAPABILITY_TEST(SupportsCompute, false); CAPABILITY_TEST(SupportsComputeSubgroups, false); CAPABILITY_TEST(SupportsReadFromResolve, false); CAPABILITY_TEST(SupportsDecalSamplerAddressMode, false); +CAPABILITY_TEST(SupportsDeviceTransientTextures, false); TEST(CapabilitiesTest, DefaultColorFormat) { auto defaults = CapabilitiesBuilder().Build(); diff --git a/engine/src/flutter/impeller/renderer/testing/mocks.h b/engine/src/flutter/impeller/renderer/testing/mocks.h index a6c15852a65..45b519cc3c9 100644 --- a/engine/src/flutter/impeller/renderer/testing/mocks.h +++ b/engine/src/flutter/impeller/renderer/testing/mocks.h @@ -200,6 +200,7 @@ class MockCapabilities : public Capabilities { MOCK_METHOD(bool, SupportsComputeSubgroups, (), (const, override)); MOCK_METHOD(bool, SupportsReadFromResolve, (), (const, override)); MOCK_METHOD(bool, SupportsDecalSamplerAddressMode, (), (const, override)); + MOCK_METHOD(bool, SupportsDeviceTransientTextures, (), (const, override)); MOCK_METHOD(PixelFormat, GetDefaultColorFormat, (), (const, override)); MOCK_METHOD(PixelFormat, GetDefaultStencilFormat, (), (const, override)); MOCK_METHOD(PixelFormat, GetDefaultDepthStencilFormat, (), (const, override));