From 6bb444f4dae30b90308c4df47336c814235c4492 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 6 Dec 2024 17:01:39 -0800 Subject: [PATCH 1/7] [Impeller] exploit perfect hash for SamplerDescriptor. --- impeller/core/formats.h | 6 ++-- impeller/core/sampler.cc | 2 +- impeller/core/sampler.h | 8 ------ impeller/core/sampler_descriptor.h | 28 ++++++++----------- .../backend/gles/buffer_bindings_gles.h | 1 - .../backend/gles/sampler_library_gles.cc | 19 ++++++++----- .../backend/gles/sampler_library_gles.h | 5 ++-- .../backend/metal/sampler_library_mtl.h | 4 +-- .../backend/metal/sampler_library_mtl.mm | 14 ++++++---- .../backend/vulkan/sampler_library_vk.cc | 15 ++++++---- .../backend/vulkan/sampler_library_vk.h | 5 ++-- .../renderer/backend/vulkan/texture_vk.cc | 2 +- .../backend/vulkan/yuv_conversion_vk.cc | 6 ++-- impeller/renderer/sampler_library.h | 2 +- impeller/renderer/testing/mocks.h | 2 +- 15 files changed, 60 insertions(+), 59 deletions(-) diff --git a/impeller/core/formats.h b/impeller/core/formats.h index 575c5431f0430..76afebefac761 100644 --- a/impeller/core/formats.h +++ b/impeller/core/formats.h @@ -412,7 +412,7 @@ struct Viewport { /// @brief Describes how the texture should be sampled when the texture /// is being shrunk (minified) or expanded (magnified) to fit to /// the sample point. -enum class MinMagFilter { +enum class MinMagFilter : uint8_t { /// Select nearest to the sample point. Most widely supported. kNearest, @@ -422,7 +422,7 @@ enum class MinMagFilter { }; /// @brief Options for selecting and filtering between mipmap levels. -enum class MipFilter { +enum class MipFilter : uint8_t { /// @brief The texture is sampled as if it only had a single mipmap level. /// /// All samples are read from level 0. @@ -438,7 +438,7 @@ enum class MipFilter { kLinear, }; -enum class SamplerAddressMode { +enum class SamplerAddressMode : uint8_t { kClampToEdge, kRepeat, kMirror, diff --git a/impeller/core/sampler.cc b/impeller/core/sampler.cc index 12b87d3e91b1a..28ee07c95c489 100644 --- a/impeller/core/sampler.cc +++ b/impeller/core/sampler.cc @@ -6,7 +6,7 @@ namespace impeller { -Sampler::Sampler(SamplerDescriptor desc) : desc_(std::move(desc)) {} +Sampler::Sampler(SamplerDescriptor desc) : desc_(desc) {} Sampler::~Sampler() = default; diff --git a/impeller/core/sampler.h b/impeller/core/sampler.h index d5fb783816fc0..db93ed0c07395 100644 --- a/impeller/core/sampler.h +++ b/impeller/core/sampler.h @@ -5,9 +5,6 @@ #ifndef FLUTTER_IMPELLER_CORE_SAMPLER_H_ #define FLUTTER_IMPELLER_CORE_SAMPLER_H_ -#include - -#include "impeller/base/comparable.h" #include "impeller/core/sampler_descriptor.h" namespace impeller { @@ -29,11 +26,6 @@ class Sampler { Sampler& operator=(const Sampler&) = delete; }; -using SamplerMap = std::unordered_map, - ComparableHash, - ComparableEqual>; - } // namespace impeller #endif // FLUTTER_IMPELLER_CORE_SAMPLER_H_ diff --git a/impeller/core/sampler_descriptor.h b/impeller/core/sampler_descriptor.h index 31e91c129a9db..08287788ce857 100644 --- a/impeller/core/sampler_descriptor.h +++ b/impeller/core/sampler_descriptor.h @@ -5,14 +5,13 @@ #ifndef FLUTTER_IMPELLER_CORE_SAMPLER_DESCRIPTOR_H_ #define FLUTTER_IMPELLER_CORE_SAMPLER_DESCRIPTOR_H_ -#include "impeller/base/comparable.h" #include "impeller/core/formats.h" namespace impeller { class Context; -struct SamplerDescriptor final : public Comparable { +struct SamplerDescriptor final { MinMagFilter min_filter = MinMagFilter::kNearest; MinMagFilter mag_filter = MinMagFilter::kNearest; MipFilter mip_filter = MipFilter::kNearest; @@ -30,20 +29,17 @@ struct SamplerDescriptor final : public Comparable { MinMagFilter mag_filter, MipFilter mip_filter); - // Comparable - std::size_t GetHash() const override { - return fml::HashCombine(min_filter, mag_filter, mip_filter, - width_address_mode, height_address_mode, - depth_address_mode); - } - - // Comparable - bool IsEqual(const SamplerDescriptor& o) const override { - return min_filter == o.min_filter && mag_filter == o.mag_filter && - mip_filter == o.mip_filter && - width_address_mode == o.width_address_mode && - height_address_mode == o.height_address_mode && - depth_address_mode == o.depth_address_mode; + static uint32_t ToKey(const SamplerDescriptor& d) { + static_assert(sizeof(MinMagFilter) == 1); + static_assert(sizeof(MipFilter) == 1); + static_assert(sizeof(SamplerAddressMode) == 1); + + return static_cast(d.min_filter) << 0 | + static_cast(d.mag_filter) << 8 | + static_cast(d.mip_filter) << 16 | + static_cast(d.width_address_mode) << 24 | + static_cast(d.height_address_mode) << 32 | + static_cast(d.depth_address_mode) << 40; } }; diff --git a/impeller/renderer/backend/gles/buffer_bindings_gles.h b/impeller/renderer/backend/gles/buffer_bindings_gles.h index 29f032da76cf9..d5e5aad531d16 100644 --- a/impeller/renderer/backend/gles/buffer_bindings_gles.h +++ b/impeller/renderer/backend/gles/buffer_bindings_gles.h @@ -5,7 +5,6 @@ #ifndef FLUTTER_IMPELLER_RENDERER_BACKEND_GLES_BUFFER_BINDINGS_GLES_H_ #define FLUTTER_IMPELLER_RENDERER_BACKEND_GLES_BUFFER_BINDINGS_GLES_H_ -#include #include #include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h" diff --git a/impeller/renderer/backend/gles/sampler_library_gles.cc b/impeller/renderer/backend/gles/sampler_library_gles.cc index 5536d6ca70929..9f6def2a82655 100644 --- a/impeller/renderer/backend/gles/sampler_library_gles.cc +++ b/impeller/renderer/backend/gles/sampler_library_gles.cc @@ -7,6 +7,7 @@ #include "impeller/base/config.h" #include "impeller/base/validation.h" #include "impeller/core/formats.h" +#include "impeller/core/sampler_descriptor.h" #include "impeller/renderer/backend/gles/sampler_gles.h" namespace impeller { @@ -22,7 +23,7 @@ SamplerLibraryGLES::~SamplerLibraryGLES() = default; // |SamplerLibrary| const std::unique_ptr& SamplerLibraryGLES::GetSampler( - SamplerDescriptor descriptor) { + const SamplerDescriptor& descriptor) { if (!supports_decal_sampler_address_mode_ && (descriptor.width_address_mode == SamplerAddressMode::kDecal || descriptor.height_address_mode == SamplerAddressMode::kDecal || @@ -31,13 +32,17 @@ const std::unique_ptr& SamplerLibraryGLES::GetSampler( "current OpenGLES backend."; return kNullSampler; } - - auto found = samplers_.find(descriptor); - if (found != samplers_.end()) { - return found->second; + uint64_t p_key = SamplerDescriptor::ToKey(descriptor); + for (const auto& [key, value] : samplers_) { + if (key == p_key) { + return value; + } } - return (samplers_[descriptor] = - std::unique_ptr(new SamplerGLES(descriptor))); + + auto sampler = std::unique_ptr(new SamplerGLES(descriptor)); + samplers_.push_back(std::make_pair(p_key, std::move(sampler))); + + return samplers_.back().second; } } // namespace impeller diff --git a/impeller/renderer/backend/gles/sampler_library_gles.h b/impeller/renderer/backend/gles/sampler_library_gles.h index 502a908c234e2..db87332e93c66 100644 --- a/impeller/renderer/backend/gles/sampler_library_gles.h +++ b/impeller/renderer/backend/gles/sampler_library_gles.h @@ -7,6 +7,7 @@ #include "impeller/core/sampler.h" #include "impeller/core/sampler_descriptor.h" +#include "impeller/renderer/backend/gles/sampler_gles.h" #include "impeller/renderer/sampler_library.h" namespace impeller { @@ -20,13 +21,13 @@ class SamplerLibraryGLES final : public SamplerLibrary { private: friend class ContextGLES; - SamplerMap samplers_; + std::vector>> samplers_; SamplerLibraryGLES(); // |SamplerLibrary| const std::unique_ptr& GetSampler( - SamplerDescriptor descriptor) override; + const SamplerDescriptor& descriptor) override; bool supports_decal_sampler_address_mode_ = false; diff --git a/impeller/renderer/backend/metal/sampler_library_mtl.h b/impeller/renderer/backend/metal/sampler_library_mtl.h index ca3be59ce999e..9b33ac03beb08 100644 --- a/impeller/renderer/backend/metal/sampler_library_mtl.h +++ b/impeller/renderer/backend/metal/sampler_library_mtl.h @@ -27,13 +27,13 @@ class SamplerLibraryMTL final friend class ContextMTL; id device_ = nullptr; - SamplerMap samplers_; + std::vector>> samplers_; explicit SamplerLibraryMTL(id device); // |SamplerLibrary| const std::unique_ptr& GetSampler( - SamplerDescriptor descriptor) override; + const SamplerDescriptor& descriptor) override; SamplerLibraryMTL(const SamplerLibraryMTL&) = delete; diff --git a/impeller/renderer/backend/metal/sampler_library_mtl.mm b/impeller/renderer/backend/metal/sampler_library_mtl.mm index 72538338075c3..bcec0d53da4c4 100644 --- a/impeller/renderer/backend/metal/sampler_library_mtl.mm +++ b/impeller/renderer/backend/metal/sampler_library_mtl.mm @@ -16,10 +16,12 @@ static const std::unique_ptr kNullSampler = nullptr; const std::unique_ptr& SamplerLibraryMTL::GetSampler( - SamplerDescriptor descriptor) { - auto found = samplers_.find(descriptor); - if (found != samplers_.end()) { - return found->second; + const SamplerDescriptor& descriptor) { + uint64_t p_key = SamplerDescriptor::ToKey(descriptor); + for (const auto& [key, value] : samplers_) { + if (key == p_key) { + return value; + } } if (!device_) { return kNullSampler; @@ -46,8 +48,8 @@ } auto sampler = std::unique_ptr(new SamplerMTL(descriptor, mtl_sampler)); - - return (samplers_[descriptor] = std::move(sampler)); + samplers_.push_back(std::make_pair(p_key, std::move(sampler))); + return samplers_.back().second; } } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/sampler_library_vk.cc b/impeller/renderer/backend/vulkan/sampler_library_vk.cc index 821cc6fffdb19..a9b7452405b39 100644 --- a/impeller/renderer/backend/vulkan/sampler_library_vk.cc +++ b/impeller/renderer/backend/vulkan/sampler_library_vk.cc @@ -19,17 +19,20 @@ SamplerLibraryVK::~SamplerLibraryVK() = default; static const std::unique_ptr kNullSampler = nullptr; const std::unique_ptr& SamplerLibraryVK::GetSampler( - SamplerDescriptor desc) { - auto found = samplers_.find(desc); - if (found != samplers_.end()) { - return found->second; + const SamplerDescriptor& desc) { + uint64_t p_key = SamplerDescriptor::ToKey(desc); + for (const auto& [key, value] : samplers_) { + if (key == p_key) { + return value; + } } auto device_holder = device_holder_.lock(); if (!device_holder || !device_holder->GetDevice()) { return kNullSampler; } - return (samplers_[desc] = - std::make_unique(device_holder->GetDevice(), desc)); + samplers_.push_back(std::make_pair( + p_key, std::make_unique(device_holder->GetDevice(), desc))); + return samplers_.back().second; } } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/sampler_library_vk.h b/impeller/renderer/backend/vulkan/sampler_library_vk.h index 4251534031612..31dbe1f8cc94c 100644 --- a/impeller/renderer/backend/vulkan/sampler_library_vk.h +++ b/impeller/renderer/backend/vulkan/sampler_library_vk.h @@ -6,6 +6,7 @@ #define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_SAMPLER_LIBRARY_VK_H_ #include "impeller/base/backend_cast.h" +#include "impeller/core/sampler.h" #include "impeller/core/sampler_descriptor.h" #include "impeller/renderer/backend/vulkan/device_holder_vk.h" #include "impeller/renderer/sampler_library.h" @@ -23,13 +24,13 @@ class SamplerLibraryVK final friend class ContextVK; std::weak_ptr device_holder_; - SamplerMap samplers_; + std::vector>> samplers_; explicit SamplerLibraryVK(const std::weak_ptr& device_holder); // |SamplerLibrary| const std::unique_ptr& GetSampler( - SamplerDescriptor descriptor) override; + const SamplerDescriptor& descriptor) override; SamplerLibraryVK(const SamplerLibraryVK&) = delete; diff --git a/impeller/renderer/backend/vulkan/texture_vk.cc b/impeller/renderer/backend/vulkan/texture_vk.cc index 7d432ef32de25..d946c8df2fb78 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_vk.cc @@ -220,7 +220,7 @@ std::shared_ptr TextureVK::GetImmutableSamplerVariant( if (!source_) { return nullptr; } - auto conversion = source_->GetYUVConversion(); + std::shared_ptr conversion = source_->GetYUVConversion(); if (!conversion) { // Most textures don't need a sampler conversion and will go down this path. // Only needed for YUV sampling from external textures. diff --git a/impeller/renderer/backend/vulkan/yuv_conversion_vk.cc b/impeller/renderer/backend/vulkan/yuv_conversion_vk.cc index 9c3aa6ed72c3f..7a7e0210172f0 100644 --- a/impeller/renderer/backend/vulkan/yuv_conversion_vk.cc +++ b/impeller/renderer/backend/vulkan/yuv_conversion_vk.cc @@ -6,6 +6,7 @@ #include "flutter/fml/hash_combine.h" #include "impeller/base/validation.h" +#include "impeller/core/sampler_descriptor.h" #include "impeller/renderer/backend/vulkan/device_holder_vk.h" #include "impeller/renderer/backend/vulkan/sampler_vk.h" @@ -107,12 +108,13 @@ ImmutableSamplerKeyVK::ImmutableSamplerKeyVK(const SamplerVK& sampler) } bool ImmutableSamplerKeyVK::IsEqual(const ImmutableSamplerKeyVK& other) const { - return sampler.IsEqual(other.sampler) && + return SamplerDescriptor::ToKey(sampler) == + SamplerDescriptor::ToKey(other.sampler) && YUVConversionDescriptorVKEqual{}(yuv_conversion, other.yuv_conversion); } std::size_t ImmutableSamplerKeyVK::GetHash() const { - return fml::HashCombine(sampler.GetHash(), + return fml::HashCombine(SamplerDescriptor::ToKey(sampler), YUVConversionDescriptorVKHash{}(yuv_conversion)); } diff --git a/impeller/renderer/sampler_library.h b/impeller/renderer/sampler_library.h index 0b05545b49a8f..a15a5ccd2cecf 100644 --- a/impeller/renderer/sampler_library.h +++ b/impeller/renderer/sampler_library.h @@ -24,7 +24,7 @@ class SamplerLibrary { /// and guarantee that the reference will continue to be valid /// throughout the lifetime of the Impeller context. virtual const std::unique_ptr& GetSampler( - SamplerDescriptor descriptor) = 0; + const SamplerDescriptor& descriptor) = 0; protected: SamplerLibrary(); diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h index d8eca488be5ea..f8ae5f06b88f0 100644 --- a/impeller/renderer/testing/mocks.h +++ b/impeller/renderer/testing/mocks.h @@ -246,7 +246,7 @@ class MockSamplerLibrary : public SamplerLibrary { public: MOCK_METHOD(const std::unique_ptr&, GetSampler, - (SamplerDescriptor descriptor), + (const SamplerDescriptor& descriptor), (override)); }; From 54959706a200a456ed92c99f4c959f6a7d08f704 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 7 Dec 2024 12:54:54 -0800 Subject: [PATCH 2/7] remove std::move of trivially copyable type. --- impeller/entity/contents/texture_contents.cc | 4 ++-- impeller/entity/contents/texture_contents.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc index f443558dd1c69..b2c6c22a8c50c 100644 --- a/impeller/entity/contents/texture_contents.cc +++ b/impeller/entity/contents/texture_contents.cc @@ -240,8 +240,8 @@ bool TextureContents::GetStrictSourceRect() const { return strict_source_rect_enabled_; } -void TextureContents::SetSamplerDescriptor(SamplerDescriptor desc) { - sampler_descriptor_ = std::move(desc); +void TextureContents::SetSamplerDescriptor(const SamplerDescriptor& desc) { + sampler_descriptor_ = desc; } const SamplerDescriptor& TextureContents::GetSamplerDescriptor() const { diff --git a/impeller/entity/contents/texture_contents.h b/impeller/entity/contents/texture_contents.h index 34657ff54dfa5..6e7ed82f6c253 100644 --- a/impeller/entity/contents/texture_contents.h +++ b/impeller/entity/contents/texture_contents.h @@ -33,7 +33,7 @@ class TextureContents final : public Contents { std::shared_ptr GetTexture() const; - void SetSamplerDescriptor(SamplerDescriptor desc); + void SetSamplerDescriptor(const SamplerDescriptor& desc); const SamplerDescriptor& GetSamplerDescriptor() const; From b2a06a47aac53d10306b44ef074d3792c597c719 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 7 Dec 2024 13:24:27 -0800 Subject: [PATCH 3/7] more removal of std::move of trivially copyable type. --- impeller/entity/contents/tiled_texture_contents.cc | 2 +- impeller/entity/contents/tiled_texture_contents.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/entity/contents/tiled_texture_contents.cc b/impeller/entity/contents/tiled_texture_contents.cc index 549abf32ff99d..4680b953e3f32 100644 --- a/impeller/entity/contents/tiled_texture_contents.cc +++ b/impeller/entity/contents/tiled_texture_contents.cc @@ -48,7 +48,7 @@ void TiledTextureContents::SetTileModes(Entity::TileMode x_tile_mode, } void TiledTextureContents::SetSamplerDescriptor(SamplerDescriptor desc) { - sampler_descriptor_ = std::move(desc); + sampler_descriptor_ = desc; } void TiledTextureContents::SetColorFilter(ColorFilterProc color_filter) { diff --git a/impeller/entity/contents/tiled_texture_contents.h b/impeller/entity/contents/tiled_texture_contents.h index 3c883aabab8ac..37519e20f9d00 100644 --- a/impeller/entity/contents/tiled_texture_contents.h +++ b/impeller/entity/contents/tiled_texture_contents.h @@ -37,7 +37,7 @@ class TiledTextureContents final : public ColorSourceContents { void SetTileModes(Entity::TileMode x_tile_mode, Entity::TileMode y_tile_mode); - void SetSamplerDescriptor(SamplerDescriptor desc); + void SetSamplerDescriptor(const SamplerDescriptor& desc); /// @brief Set a color filter to apply directly to this tiled texture /// @param color_filter From 9c8b58f17d305689ede86e63c66b9c7c08241c70 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 7 Dec 2024 13:44:31 -0800 Subject: [PATCH 4/7] ++ --- impeller/entity/contents/tiled_texture_contents.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/entity/contents/tiled_texture_contents.cc b/impeller/entity/contents/tiled_texture_contents.cc index 4680b953e3f32..51dcd830841d6 100644 --- a/impeller/entity/contents/tiled_texture_contents.cc +++ b/impeller/entity/contents/tiled_texture_contents.cc @@ -47,7 +47,7 @@ void TiledTextureContents::SetTileModes(Entity::TileMode x_tile_mode, y_tile_mode_ = y_tile_mode; } -void TiledTextureContents::SetSamplerDescriptor(SamplerDescriptor desc) { +void TiledTextureContents::SetSamplerDescriptor(const SamplerDescriptor& desc) { sampler_descriptor_ = desc; } From cb2dc99e00f0756e7406dfd313cdfac09fa16b28 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 7 Dec 2024 15:44:46 -0800 Subject: [PATCH 5/7] more clang tidy fixes. --- impeller/core/sampler.cc | 2 +- impeller/core/sampler.h | 2 +- impeller/renderer/backend/gles/sampler_gles.cc | 5 +++-- impeller/renderer/backend/gles/sampler_gles.h | 2 +- impeller/renderer/backend/metal/sampler_mtl.h | 2 +- impeller/renderer/backend/metal/sampler_mtl.mm | 4 ++-- impeller/renderer/backend/vulkan/sampler_vk.cc | 4 ++-- impeller/renderer/backend/vulkan/sampler_vk.h | 2 +- 8 files changed, 12 insertions(+), 11 deletions(-) diff --git a/impeller/core/sampler.cc b/impeller/core/sampler.cc index 28ee07c95c489..f404c1e33d767 100644 --- a/impeller/core/sampler.cc +++ b/impeller/core/sampler.cc @@ -6,7 +6,7 @@ namespace impeller { -Sampler::Sampler(SamplerDescriptor desc) : desc_(desc) {} +Sampler::Sampler(const SamplerDescriptor& desc) : desc_(desc) {} Sampler::~Sampler() = default; diff --git a/impeller/core/sampler.h b/impeller/core/sampler.h index db93ed0c07395..3dcd2f6abb551 100644 --- a/impeller/core/sampler.h +++ b/impeller/core/sampler.h @@ -18,7 +18,7 @@ class Sampler { protected: SamplerDescriptor desc_; - explicit Sampler(SamplerDescriptor desc); + explicit Sampler(const SamplerDescriptor& desc); private: Sampler(const Sampler&) = delete; diff --git a/impeller/renderer/backend/gles/sampler_gles.cc b/impeller/renderer/backend/gles/sampler_gles.cc index 52d5e912420d3..574c4e82d3d80 100644 --- a/impeller/renderer/backend/gles/sampler_gles.cc +++ b/impeller/renderer/backend/gles/sampler_gles.cc @@ -6,13 +6,14 @@ #include "impeller/base/validation.h" #include "impeller/core/formats.h" +#include "impeller/core/sampler_descriptor.h" #include "impeller/renderer/backend/gles/formats_gles.h" #include "impeller/renderer/backend/gles/proc_table_gles.h" #include "impeller/renderer/backend/gles/texture_gles.h" namespace impeller { -SamplerGLES::SamplerGLES(SamplerDescriptor desc) : Sampler(std::move(desc)) {} +SamplerGLES::SamplerGLES(const SamplerDescriptor& desc) : Sampler(desc) {} SamplerGLES::~SamplerGLES() = default; @@ -81,7 +82,7 @@ bool SamplerGLES::ConfigureBoundTexture(const TextureGLES& texture, if (!target.has_value()) { return false; } - const auto& desc = GetDescriptor(); + const SamplerDescriptor& desc = GetDescriptor(); GLint mag_filter = ToParam(desc.mag_filter); diff --git a/impeller/renderer/backend/gles/sampler_gles.h b/impeller/renderer/backend/gles/sampler_gles.h index 0145d106ab532..837b50dabaa82 100644 --- a/impeller/renderer/backend/gles/sampler_gles.h +++ b/impeller/renderer/backend/gles/sampler_gles.h @@ -25,7 +25,7 @@ class SamplerGLES final : public Sampler, private: friend class SamplerLibraryGLES; - explicit SamplerGLES(SamplerDescriptor desc); + explicit SamplerGLES(const SamplerDescriptor&); SamplerGLES(const SamplerGLES&) = delete; diff --git a/impeller/renderer/backend/metal/sampler_mtl.h b/impeller/renderer/backend/metal/sampler_mtl.h index a295a24eb59f6..ab95f04b309e7 100644 --- a/impeller/renderer/backend/metal/sampler_mtl.h +++ b/impeller/renderer/backend/metal/sampler_mtl.h @@ -29,7 +29,7 @@ class SamplerMTL final : public Sampler, id state_ = nullptr; - SamplerMTL(SamplerDescriptor desc, id state); + SamplerMTL(const SamplerDescriptor& desc, id state); SamplerMTL(const SamplerMTL&) = delete; diff --git a/impeller/renderer/backend/metal/sampler_mtl.mm b/impeller/renderer/backend/metal/sampler_mtl.mm index 71bcd08e3138f..2d2d40c9b18b9 100644 --- a/impeller/renderer/backend/metal/sampler_mtl.mm +++ b/impeller/renderer/backend/metal/sampler_mtl.mm @@ -6,8 +6,8 @@ namespace impeller { -SamplerMTL::SamplerMTL(SamplerDescriptor desc, id state) - : Sampler(std::move(desc)), state_(state) { +SamplerMTL::SamplerMTL(const SamplerDescriptor& desc, id state) + : Sampler(desc), state_(state) { FML_DCHECK(state_); } diff --git a/impeller/renderer/backend/vulkan/sampler_vk.cc b/impeller/renderer/backend/vulkan/sampler_vk.cc index 8a1eaccf72ae3..5231d3054e56e 100644 --- a/impeller/renderer/backend/vulkan/sampler_vk.cc +++ b/impeller/renderer/backend/vulkan/sampler_vk.cc @@ -99,9 +99,9 @@ static vk::UniqueSampler CreateSampler( } SamplerVK::SamplerVK(const vk::Device& device, - SamplerDescriptor desc, + const SamplerDescriptor& desc, std::shared_ptr yuv_conversion) - : Sampler(std::move(desc)), + : Sampler(desc), device_(device), sampler_(MakeSharedVK( CreateSampler(device, desc_, yuv_conversion))), diff --git a/impeller/renderer/backend/vulkan/sampler_vk.h b/impeller/renderer/backend/vulkan/sampler_vk.h index 5868085ca517e..3b5190d4a8837 100644 --- a/impeller/renderer/backend/vulkan/sampler_vk.h +++ b/impeller/renderer/backend/vulkan/sampler_vk.h @@ -18,7 +18,7 @@ class YUVConversionVK; class SamplerVK final : public Sampler, public BackendCast { public: SamplerVK(const vk::Device& device, - SamplerDescriptor desc, + const SamplerDescriptor&, std::shared_ptr yuv_conversion = {}); // |Sampler| From 7dcbff4040d5083f9f877845f837395bd2fd14bc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 8 Dec 2024 12:44:50 -0800 Subject: [PATCH 6/7] ++ --- impeller/display_list/canvas.cc | 8 ++++---- impeller/display_list/canvas.h | 4 ++-- .../entity/contents/filters/matrix_filter_contents.cc | 4 ++-- impeller/entity/contents/filters/matrix_filter_contents.h | 2 +- impeller/entity/contents/vertices_contents.cc | 4 ++-- impeller/entity/contents/vertices_contents.h | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/impeller/display_list/canvas.cc b/impeller/display_list/canvas.cc index 11cc4d7598a9f..cf9a0570cd50c 100644 --- a/impeller/display_list/canvas.cc +++ b/impeller/display_list/canvas.cc @@ -658,7 +658,7 @@ void Canvas::DrawPoints(const Point points[], void Canvas::DrawImage(const std::shared_ptr& image, Point offset, const Paint& paint, - SamplerDescriptor sampler) { + const SamplerDescriptor& sampler) { if (!image) { return; } @@ -666,14 +666,14 @@ void Canvas::DrawImage(const std::shared_ptr& image, const auto source = Rect::MakeSize(image->GetSize()); const auto dest = source.Shift(offset); - DrawImageRect(image, source, dest, paint, std::move(sampler)); + DrawImageRect(image, source, dest, paint, sampler); } void Canvas::DrawImageRect(const std::shared_ptr& image, Rect source, Rect dest, const Paint& paint, - SamplerDescriptor sampler, + const SamplerDescriptor& sampler, SourceRectConstraint src_rect_constraint) { if (!image || source.IsEmpty() || dest.IsEmpty()) { return; @@ -704,7 +704,7 @@ void Canvas::DrawImageRect(const std::shared_ptr& image, texture_contents->SetSourceRect(*clipped_source); texture_contents->SetStrictSourceRect(src_rect_constraint == SourceRectConstraint::kStrict); - texture_contents->SetSamplerDescriptor(std::move(sampler)); + texture_contents->SetSamplerDescriptor(sampler); texture_contents->SetOpacity(paint.color.alpha); texture_contents->SetDeferApplyingOpacity(paint.HasColorFilter()); diff --git a/impeller/display_list/canvas.h b/impeller/display_list/canvas.h index fbb4a8bb0abde..d92c6d79a86e3 100644 --- a/impeller/display_list/canvas.h +++ b/impeller/display_list/canvas.h @@ -209,14 +209,14 @@ class Canvas { void DrawImage(const std::shared_ptr& image, Point offset, const Paint& paint, - SamplerDescriptor sampler = {}); + const SamplerDescriptor& sampler = {}); void DrawImageRect( const std::shared_ptr& image, Rect source, Rect dest, const Paint& paint, - SamplerDescriptor sampler = {}, + const SamplerDescriptor& sampler = {}, SourceRectConstraint src_rect_constraint = SourceRectConstraint::kFast); void DrawTextFrame(const std::shared_ptr& text_frame, diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index 444a62ee298aa..743716d4f4cb8 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -20,8 +20,8 @@ void MatrixFilterContents::SetRenderingMode( FilterContents::SetRenderingMode(rendering_mode); } -void MatrixFilterContents::SetSamplerDescriptor(SamplerDescriptor desc) { - sampler_descriptor_ = std::move(desc); +void MatrixFilterContents::SetSamplerDescriptor(const SamplerDescriptor& desc) { + sampler_descriptor_ = desc; } namespace { diff --git a/impeller/entity/contents/filters/matrix_filter_contents.h b/impeller/entity/contents/filters/matrix_filter_contents.h index 9083433a6e207..f853243d84cd0 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.h +++ b/impeller/entity/contents/filters/matrix_filter_contents.h @@ -21,7 +21,7 @@ class MatrixFilterContents final : public FilterContents { // |FilterContents| void SetRenderingMode(Entity::RenderingMode rendering_mode) override; - void SetSamplerDescriptor(SamplerDescriptor desc); + void SetSamplerDescriptor(const SamplerDescriptor& desc); // |FilterContents| std::optional GetFilterCoverage( diff --git a/impeller/entity/contents/vertices_contents.cc b/impeller/entity/contents/vertices_contents.cc index 3669dd09a47e8..a2968d0e310e5 100644 --- a/impeller/entity/contents/vertices_contents.cc +++ b/impeller/entity/contents/vertices_contents.cc @@ -70,8 +70,8 @@ std::optional VerticesSimpleBlendContents::GetCoverage( } void VerticesSimpleBlendContents::SetSamplerDescriptor( - SamplerDescriptor descriptor) { - descriptor_ = std::move(descriptor); + const SamplerDescriptor& descriptor) { + descriptor_ = descriptor; } void VerticesSimpleBlendContents::SetTileMode(Entity::TileMode tile_mode_x, diff --git a/impeller/entity/contents/vertices_contents.h b/impeller/entity/contents/vertices_contents.h index 601ff47f6a251..d1a30ec8aa835 100644 --- a/impeller/entity/contents/vertices_contents.h +++ b/impeller/entity/contents/vertices_contents.h @@ -36,7 +36,7 @@ class VerticesSimpleBlendContents final : public Contents { void SetLazyTexture(const LazyTexture& lazy_texture); - void SetSamplerDescriptor(SamplerDescriptor descriptor); + void SetSamplerDescriptor(const SamplerDescriptor& descriptor); void SetTileMode(Entity::TileMode tile_mode_x, Entity::TileMode tile_mode_y); From 1f56035262795923a3f12b576d59cc7525db9874 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 11 Dec 2024 16:53:17 -0800 Subject: [PATCH 7/7] switch to raw_ptr. --- impeller/compiler/reflector.cc | 2 +- impeller/entity/contents/atlas_contents.cc | 4 +-- .../contents/filters/blend_filter_contents.cc | 10 +++---- .../contents/framebuffer_blend_contents.cc | 2 +- .../contents/test/recording_render_pass.cc | 13 ++++---- .../contents/test/recording_render_pass.h | 13 ++++---- impeller/entity/contents/vertices_contents.cc | 2 +- .../playground/imgui/imgui_impl_impeller.cc | 4 +-- .../backend/gles/buffer_bindings_gles.cc | 2 +- .../backend/gles/sampler_library_gles.h | 2 +- .../backend/metal/compute_pass_mtl.mm | 13 ++++---- .../renderer/backend/metal/render_pass_mtl.h | 13 ++++---- .../renderer/backend/metal/render_pass_mtl.mm | 13 ++++---- .../backend/vulkan/compute_pass_vk.cc | 13 ++++---- .../renderer/backend/vulkan/render_pass_vk.cc | 13 ++++---- .../renderer/backend/vulkan/render_pass_vk.h | 13 ++++---- impeller/renderer/command.h | 2 +- impeller/renderer/render_pass.cc | 15 +++++----- impeller/renderer/render_pass.h | 26 ++++++++-------- impeller/renderer/renderer_unittests.cc | 30 +++++++------------ impeller/renderer/testing/mocks.h | 2 +- lib/gpu/render_pass.cc | 8 ++--- 22 files changed, 98 insertions(+), 117 deletions(-) diff --git a/impeller/compiler/reflector.cc b/impeller/compiler/reflector.cc index 5f9ba45eb1f48..c71619a66089e 100644 --- a/impeller/compiler/reflector.cc +++ b/impeller/compiler/reflector.cc @@ -1390,7 +1390,7 @@ std::vector Reflector::ReflectBindPrototypes( .argument_name = "texture", }); proto.args.push_back(BindPrototypeArgument{ - .type_name = "const std::unique_ptr&", + .type_name = "raw_ptr", .argument_name = "sampler", }); } diff --git a/impeller/entity/contents/atlas_contents.cc b/impeller/entity/contents/atlas_contents.cc index 3addc0bac0375..d23c63d933cf2 100644 --- a/impeller/entity/contents/atlas_contents.cc +++ b/impeller/entity/contents/atlas_contents.cc @@ -47,7 +47,7 @@ bool AtlasContents::Render(const ContentContext& renderer, dst_sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal; dst_sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal; } - const std::unique_ptr& dst_sampler = + raw_ptr dst_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( dst_sampler_descriptor); @@ -58,7 +58,7 @@ bool AtlasContents::Render(const ContentContext& renderer, auto dst_sampler_descriptor = geometry_->GetSamplerDescriptor(); - const std::unique_ptr& dst_sampler = + raw_ptr dst_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( dst_sampler_descriptor); diff --git a/impeller/entity/contents/filters/blend_filter_contents.cc b/impeller/entity/contents/filters/blend_filter_contents.cc index fd617161d1a05..99b47dd26df3f 100644 --- a/impeller/entity/contents/filters/blend_filter_contents.cc +++ b/impeller/entity/contents/filters/blend_filter_contents.cc @@ -187,7 +187,7 @@ static std::optional AdvancedBlend( dst_sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal; dst_sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal; } - const std::unique_ptr& dst_sampler = + raw_ptr dst_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( dst_sampler_descriptor); FS::BindTextureSamplerDst(pass, dst_snapshot->texture, dst_sampler); @@ -210,7 +210,7 @@ static std::optional AdvancedBlend( src_sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal; src_sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal; } - const std::unique_ptr& src_sampler = + raw_ptr src_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( src_sampler_descriptor); blend_info.color_factor = 0; @@ -361,7 +361,7 @@ std::optional BlendFilterContents::CreateForegroundAdvancedBlend( dst_sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal; dst_sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal; } - const std::unique_ptr& dst_sampler = + raw_ptr dst_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( dst_sampler_descriptor); FS::BindTextureSamplerDst(pass, dst_snapshot->texture, dst_sampler); @@ -469,7 +469,7 @@ std::optional BlendFilterContents::CreateForegroundPorterDuffBlend( dst_sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal; dst_sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal; } - const std::unique_ptr& dst_sampler = + raw_ptr dst_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( dst_sampler_descriptor); FS::BindTextureSamplerDst(pass, dst_snapshot->texture, dst_sampler); @@ -839,7 +839,7 @@ std::optional BlendFilterContents::CreateFramebufferAdvancedBlend( src_sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal; src_sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal; } - const std::unique_ptr& src_sampler = + raw_ptr src_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( src_sampler_descriptor); FS::BindTextureSamplerSrc(pass, src_texture, src_sampler); diff --git a/impeller/entity/contents/framebuffer_blend_contents.cc b/impeller/entity/contents/framebuffer_blend_contents.cc index 6597b3ff9ca44..74a5d7cedb236 100644 --- a/impeller/entity/contents/framebuffer_blend_contents.cc +++ b/impeller/entity/contents/framebuffer_blend_contents.cc @@ -128,7 +128,7 @@ bool FramebufferBlendContents::Render(const ContentContext& renderer, src_sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal; src_sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal; } - const std::unique_ptr& src_sampler = + raw_ptr src_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( src_sampler_descriptor); FS::BindTextureSamplerSrc(pass, src_snapshot->texture, src_sampler); diff --git a/impeller/entity/contents/test/recording_render_pass.cc b/impeller/entity/contents/test/recording_render_pass.cc index af2ae70f8c446..ddd2bc73f0737 100644 --- a/impeller/entity/contents/test/recording_render_pass.cc +++ b/impeller/entity/contents/test/recording_render_pass.cc @@ -143,13 +143,12 @@ bool RecordingRenderPass::BindDynamicResource( return true; } -bool RecordingRenderPass::BindResource( - ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - const ShaderMetadata* metadata, - std::shared_ptr texture, - raw_ptr sampler) { +bool RecordingRenderPass::BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata* metadata, + std::shared_ptr texture, + raw_ptr sampler) { if (delegate_) { return delegate_->BindResource(stage, type, slot, metadata, texture, sampler); diff --git a/impeller/entity/contents/test/recording_render_pass.h b/impeller/entity/contents/test/recording_render_pass.h index b544e606b3f27..e84bec1edce59 100644 --- a/impeller/entity/contents/test/recording_render_pass.h +++ b/impeller/entity/contents/test/recording_render_pass.h @@ -66,13 +66,12 @@ class RecordingRenderPass : public RenderPass { BufferView view) override; // |RenderPass| - bool BindDynamicResource( - ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - std::unique_ptr metadata, - std::shared_ptr texture, - raw_ptr sampler) override; + bool BindDynamicResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + std::unique_ptr metadata, + std::shared_ptr texture, + raw_ptr sampler) override; // |RenderPass| void OnSetLabel(std::string_view label) override; diff --git a/impeller/entity/contents/vertices_contents.cc b/impeller/entity/contents/vertices_contents.cc index a2968d0e310e5..49ced2d8b739a 100644 --- a/impeller/entity/contents/vertices_contents.cc +++ b/impeller/entity/contents/vertices_contents.cc @@ -126,7 +126,7 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, TileModeToAddressMode(tile_mode_y_, renderer.GetDeviceCapabilities()) .value_or(SamplerAddressMode::kClampToEdge); - const std::unique_ptr& dst_sampler = + raw_ptr dst_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( dst_sampler_descriptor); diff --git a/impeller/playground/imgui/imgui_impl_impeller.cc b/impeller/playground/imgui/imgui_impl_impeller.cc index 1eb7657f93751..6f9cdbd93cdc9 100644 --- a/impeller/playground/imgui/imgui_impl_impeller.cc +++ b/impeller/playground/imgui/imgui_impl_impeller.cc @@ -38,13 +38,13 @@ struct ImGui_ImplImpeller_Data { explicit ImGui_ImplImpeller_Data( - const std::unique_ptr& p_sampler) + impeller::raw_ptr p_sampler) : sampler(p_sampler) {} std::shared_ptr context; std::shared_ptr font_texture; std::shared_ptr> pipeline; - const std::unique_ptr& sampler; + impeller::raw_ptr sampler; }; static ImGui_ImplImpeller_Data* ImGui_ImplImpeller_GetBackendData() { diff --git a/impeller/renderer/backend/gles/buffer_bindings_gles.cc b/impeller/renderer/backend/gles/buffer_bindings_gles.cc index 4191a939ae025..570615c224586 100644 --- a/impeller/renderer/backend/gles/buffer_bindings_gles.cc +++ b/impeller/renderer/backend/gles/buffer_bindings_gles.cc @@ -486,7 +486,7 @@ std::optional BufferBindingsGLES::BindTextures( /// If there is a sampler for the texture at the same index, configure the /// bound texture using that sampler. /// - const auto& sampler_gles = SamplerGLES::Cast(**data.sampler); + const auto& sampler_gles = SamplerGLES::Cast(*data.sampler); if (!sampler_gles.ConfigureBoundTexture(texture_gles, gl)) { return std::nullopt; } diff --git a/impeller/renderer/backend/gles/sampler_library_gles.h b/impeller/renderer/backend/gles/sampler_library_gles.h index 6c6289ba84a2c..1cfc9a78443db 100644 --- a/impeller/renderer/backend/gles/sampler_library_gles.h +++ b/impeller/renderer/backend/gles/sampler_library_gles.h @@ -26,7 +26,7 @@ class SamplerLibraryGLES final : public SamplerLibrary { SamplerLibraryGLES(); // |SamplerLibrary| -raw_ptr GetSampler( + raw_ptr GetSampler( const SamplerDescriptor& descriptor) override; bool supports_decal_sampler_address_mode_ = false; diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.mm b/impeller/renderer/backend/metal/compute_pass_mtl.mm index cd1cdb62ca65a..c5bc526e88195 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.mm +++ b/impeller/renderer/backend/metal/compute_pass_mtl.mm @@ -105,13 +105,12 @@ } // |ComputePass| -bool ComputePassMTL::BindResource( - ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - const ShaderMetadata* metadata, - std::shared_ptr texture, - raw_ptr sampler) { +bool ComputePassMTL::BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata* metadata, + std::shared_ptr texture, + raw_ptr sampler) { if (!sampler || !texture->IsValid()) { return false; } diff --git a/impeller/renderer/backend/metal/render_pass_mtl.h b/impeller/renderer/backend/metal/render_pass_mtl.h index 92087967576e1..9442eb193092d 100644 --- a/impeller/renderer/backend/metal/render_pass_mtl.h +++ b/impeller/renderer/backend/metal/render_pass_mtl.h @@ -114,13 +114,12 @@ class RenderPassMTL final : public RenderPass { BufferView view) override; // |RenderPass| - bool BindDynamicResource( - ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - std::unique_ptr metadata, - std::shared_ptr texture, - raw_ptr sampler) override; + bool BindDynamicResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + std::unique_ptr metadata, + std::shared_ptr texture, + raw_ptr sampler) override; RenderPassMTL(const RenderPassMTL&) = delete; diff --git a/impeller/renderer/backend/metal/render_pass_mtl.mm b/impeller/renderer/backend/metal/render_pass_mtl.mm index 929cdc6f88626..fb26a8d02a765 100644 --- a/impeller/renderer/backend/metal/render_pass_mtl.mm +++ b/impeller/renderer/backend/metal/render_pass_mtl.mm @@ -400,13 +400,12 @@ static bool Bind(PassBindingsCacheMTL& pass, } // |RenderPass| -bool RenderPassMTL::BindResource( - ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - const ShaderMetadata* metadata, - std::shared_ptr texture, - raw_ptr sampler) { +bool RenderPassMTL::BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata* metadata, + std::shared_ptr texture, + raw_ptr sampler) { if (!texture) { return false; } diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 88b0dc6e8a69b..46ee5fd09aff6 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -140,13 +140,12 @@ bool ComputePassVK::BindResource(ShaderStage stage, } // |ResourceBinder| -bool ComputePassVK::BindResource( - ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - const ShaderMetadata* metadata, - std::shared_ptr texture, - raw_ptr sampler) { +bool ComputePassVK::BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata* metadata, + std::shared_ptr texture, + raw_ptr sampler) { if (bound_image_offset_ >= kMaxBindings) { return false; } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index b4c517e9b0058..b4a939d909fd2 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -601,13 +601,12 @@ bool RenderPassVK::BindResource(size_t binding, return true; } -bool RenderPassVK::BindDynamicResource( - ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - std::unique_ptr metadata, - std::shared_ptr texture, - raw_ptr sampler) { +bool RenderPassVK::BindDynamicResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + std::unique_ptr metadata, + std::shared_ptr texture, + raw_ptr sampler) { return BindResource(stage, type, slot, nullptr, texture, sampler); } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index 68c5e7fc15179..a467652c62485 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -114,13 +114,12 @@ class RenderPassVK final : public RenderPass { BufferView view) override; // |RenderPass| - bool BindDynamicResource( - ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - std::unique_ptr metadata, - std::shared_ptr texture, - raw_ptr sampler) override; + bool BindDynamicResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + std::unique_ptr metadata, + std::shared_ptr texture, + raw_ptr sampler) override; bool BindResource(size_t binding, DescriptorType type, BufferView view); diff --git a/impeller/renderer/command.h b/impeller/renderer/command.h index f23d6769a06d8..c3c0fa2114e93 100644 --- a/impeller/renderer/command.h +++ b/impeller/renderer/command.h @@ -59,7 +59,7 @@ struct TextureAndSampler { SampledImageSlot slot; ShaderStage stage; TextureResource texture; - const std::unique_ptr* sampler; + raw_ptr sampler; }; //------------------------------------------------------------------------------ diff --git a/impeller/renderer/render_pass.cc b/impeller/renderer/render_pass.cc index f387ee1aade4e..4086182f5fb07 100644 --- a/impeller/renderer/render_pass.cc +++ b/impeller/renderer/render_pass.cc @@ -263,13 +263,12 @@ bool RenderPass::BindDynamicResource(ShaderStage stage, return BindBuffer(stage, slot, std::move(resouce)); } -bool RenderPass::BindDynamicResource( - ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - std::unique_ptr metadata, - std::shared_ptr texture, - raw_ptr sampler) { +bool RenderPass::BindDynamicResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + std::unique_ptr metadata, + std::shared_ptr texture, + raw_ptr sampler) { if (!sampler) { return false; } @@ -300,7 +299,7 @@ bool RenderPass::BindTexture(ShaderStage stage, TextureAndSampler data = TextureAndSampler{ .stage = stage, .texture = std::move(resource), - .sampler = &sampler, + .sampler = sampler, }; if (!bound_textures_start_.has_value()) { diff --git a/impeller/renderer/render_pass.h b/impeller/renderer/render_pass.h index 473668f95450d..dcc6493dedc51 100644 --- a/impeller/renderer/render_pass.h +++ b/impeller/renderer/render_pass.h @@ -179,22 +179,20 @@ class RenderPass : public ResourceBinder { BufferView view) override; // |ResourceBinder| - virtual bool BindResource( - ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - const ShaderMetadata* metadata, - std::shared_ptr texture, - raw_ptr) override; + virtual bool BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata* metadata, + std::shared_ptr texture, + raw_ptr) override; /// @brief Bind with dynamically generated shader metadata. - virtual bool BindDynamicResource( - ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - std::unique_ptr metadata, - std::shared_ptr texture, - raw_ptr); + virtual bool BindDynamicResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + std::unique_ptr metadata, + std::shared_ptr texture, + raw_ptr); /// @brief Bind with dynamically generated shader metadata. virtual bool BindDynamicResource(ShaderStage stage, diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index db0d8b495bfc2..bea83ca5edc60 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -75,8 +75,7 @@ TEST_P(RendererTest, CanCreateBoxPrimitive) { auto bridge = CreateTextureForFixture("bay_bridge.jpg"); auto boston = CreateTextureForFixture("boston.jpg"); ASSERT_TRUE(bridge && boston); - raw_ptr sampler = - context->GetSamplerLibrary()->GetSampler({}); + raw_ptr sampler = context->GetSamplerLibrary()->GetSampler({}); ASSERT_TRUE(sampler); auto host_buffer = HostBuffer::Create(context->GetResourceAllocator(), @@ -238,8 +237,7 @@ TEST_P(RendererTest, CanRenderPerspectiveCube) { auto device_buffer = context->GetResourceAllocator()->CreateBufferWithCopy( reinterpret_cast(&cube), sizeof(cube)); - raw_ptr sampler = - context->GetSamplerLibrary()->GetSampler({}); + raw_ptr sampler = context->GetSamplerLibrary()->GetSampler({}); ASSERT_TRUE(sampler); Vector3 euler_angles; @@ -320,8 +318,7 @@ TEST_P(RendererTest, CanRenderMultiplePrimitives) { auto bridge = CreateTextureForFixture("bay_bridge.jpg"); auto boston = CreateTextureForFixture("boston.jpg"); ASSERT_TRUE(bridge && boston); - raw_ptr sampler = - context->GetSamplerLibrary()->GetSampler({}); + raw_ptr sampler = context->GetSamplerLibrary()->GetSampler({}); ASSERT_TRUE(sampler); auto host_buffer = HostBuffer::Create(context->GetResourceAllocator(), @@ -398,8 +395,7 @@ TEST_P(RendererTest, CanRenderToTexture) { auto bridge = CreateTextureForFixture("bay_bridge.jpg"); auto boston = CreateTextureForFixture("boston.jpg"); ASSERT_TRUE(bridge && boston); - raw_ptr sampler = - context->GetSamplerLibrary()->GetSampler({}); + raw_ptr sampler = context->GetSamplerLibrary()->GetSampler({}); ASSERT_TRUE(sampler); std::shared_ptr r2t_pass; @@ -553,8 +549,7 @@ TEST_P(RendererTest, CanBlitTextureToTexture) { auto bridge = CreateTextureForFixture("bay_bridge.jpg"); auto boston = CreateTextureForFixture("boston.jpg"); ASSERT_TRUE(bridge && boston); - raw_ptr sampler = - context->GetSamplerLibrary()->GetSampler({}); + raw_ptr sampler = context->GetSamplerLibrary()->GetSampler({}); ASSERT_TRUE(sampler); // Vertex buffer. @@ -619,7 +614,7 @@ TEST_P(RendererTest, CanBlitTextureToTexture) { frag_info.lod = 0; FS::BindFragInfo(*pass, host_buffer->EmplaceUniform(frag_info)); - auto& sampler = context->GetSamplerLibrary()->GetSampler({}); + auto sampler = context->GetSamplerLibrary()->GetSampler({}); FS::BindTex(*pass, texture, sampler); pass->Draw(); @@ -656,8 +651,7 @@ TEST_P(RendererTest, CanBlitTextureToBuffer) { auto bridge = CreateTextureForFixture("bay_bridge.jpg"); auto boston = CreateTextureForFixture("boston.jpg"); ASSERT_TRUE(bridge && boston); - raw_ptr sampler = - context->GetSamplerLibrary()->GetSampler({}); + raw_ptr sampler = context->GetSamplerLibrary()->GetSampler({}); ASSERT_TRUE(sampler); TextureDescriptor texture_desc; @@ -908,14 +902,14 @@ TEST_P(RendererTest, TheImpeller) { SamplerDescriptor noise_sampler_desc; noise_sampler_desc.width_address_mode = SamplerAddressMode::kRepeat; noise_sampler_desc.height_address_mode = SamplerAddressMode::kRepeat; - const std::unique_ptr& noise_sampler = + raw_ptr noise_sampler = context->GetSamplerLibrary()->GetSampler(noise_sampler_desc); auto cube_map = CreateTextureCubeForFixture( {"table_mountain_px.png", "table_mountain_nx.png", "table_mountain_py.png", "table_mountain_ny.png", "table_mountain_pz.png", "table_mountain_nz.png"}); - const std::unique_ptr& cube_map_sampler = + raw_ptr cube_map_sampler = context->GetSamplerLibrary()->GetSampler({}); auto host_buffer = HostBuffer::Create(context->GetResourceAllocator(), context->GetIdleWaiter()); @@ -1244,8 +1238,7 @@ TEST_P(RendererTest, StencilMask) { auto bridge = CreateTextureForFixture("bay_bridge.jpg"); auto boston = CreateTextureForFixture("boston.jpg"); ASSERT_TRUE(bridge && boston); - raw_ptr sampler = - context->GetSamplerLibrary()->GetSampler({}); + raw_ptr sampler = context->GetSamplerLibrary()->GetSampler({}); ASSERT_TRUE(sampler); static bool mirror = false; @@ -1621,8 +1614,7 @@ TEST_P(RendererTest, BindingNullTexturesDoesNotCrash) { using FS = BoxFadeFragmentShader; auto context = GetContext(); - raw_ptr sampler = - context->GetSamplerLibrary()->GetSampler({}); + raw_ptr sampler = context->GetSamplerLibrary()->GetSampler({}); auto command_buffer = context->CreateCommandBuffer(); RenderTargetAllocator allocator(context->GetResourceAllocator()); diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h index f8ae5f06b88f0..d4759b80c9c7d 100644 --- a/impeller/renderer/testing/mocks.h +++ b/impeller/renderer/testing/mocks.h @@ -244,7 +244,7 @@ class MockCommandQueue : public CommandQueue { class MockSamplerLibrary : public SamplerLibrary { public: - MOCK_METHOD(const std::unique_ptr&, + MOCK_METHOD(raw_ptr, GetSampler, (const SamplerDescriptor& descriptor), (override)); diff --git a/lib/gpu/render_pass.cc b/lib/gpu/render_pass.cc index 3416ef1d0b8a1..de1b577947edf 100644 --- a/lib/gpu/render_pass.cc +++ b/lib/gpu/render_pass.cc @@ -192,7 +192,7 @@ bool RenderPass::Draw() { texture.slot, std::make_unique( *texture.texture.GetMetadata()), - texture.texture.resource, *texture.sampler); + texture.texture.resource, texture.sampler); } for (const auto& [_, buffer] : fragment_uniform_bindings) { render_pass_->BindDynamicResource( @@ -207,7 +207,7 @@ bool RenderPass::Draw() { impeller::DescriptorType::kSampledImage, texture.slot, std::make_unique( *texture.texture.GetMetadata()), - texture.texture.resource, *texture.sampler); + texture.texture.resource, texture.sampler); } render_pass_->SetVertexBuffer(vertex_buffer); @@ -466,7 +466,7 @@ bool InternalFlutterGpu_RenderPass_BindTexture( flutter::gpu::ToImpellerSamplerAddressMode(width_address_mode); sampler_desc.height_address_mode = flutter::gpu::ToImpellerSamplerAddressMode(height_address_mode); - const std::unique_ptr& sampler = + auto sampler = wrapper->GetContext()->GetSamplerLibrary()->GetSampler(sampler_desc); flutter::gpu::RenderPass::TextureUniformMap* uniform_map = nullptr; @@ -486,7 +486,7 @@ bool InternalFlutterGpu_RenderPass_BindTexture( impeller::TextureAndSampler{ .slot = texture_binding->slot, .texture = {&texture_binding->metadata, texture->GetTexture()}, - .sampler = &sampler, + .sampler = sampler, }); return true; }