From 18a52d814676a1adeb9f1285a30891cca36a05be Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 14 Nov 2023 17:50:17 -0800 Subject: [PATCH 01/17] [Impeller] drawable backed texture. --- .../renderer/backend/metal/allocator_mtl.mm | 2 +- .../backend/metal/device_buffer_mtl.mm | 2 +- .../renderer/backend/metal/surface_mtl.mm | 2 +- impeller/renderer/backend/metal/texture_mtl.h | 54 ++-- .../renderer/backend/metal/texture_mtl.mm | 304 +++++++++++++----- 5 files changed, 245 insertions(+), 119 deletions(-) diff --git a/impeller/renderer/backend/metal/allocator_mtl.mm b/impeller/renderer/backend/metal/allocator_mtl.mm index 76e3083b7fbcd..57f6f6c64a19c 100644 --- a/impeller/renderer/backend/metal/allocator_mtl.mm +++ b/impeller/renderer/backend/metal/allocator_mtl.mm @@ -217,7 +217,7 @@ static MTLStorageMode ToMTLStorageMode(StorageMode mode, if (!texture) { return nullptr; } - return std::make_shared(desc, texture); + return TextureMTL::BetterName(desc, texture); } uint16_t AllocatorMTL::MinimumBytesPerRow(PixelFormat format) const { diff --git a/impeller/renderer/backend/metal/device_buffer_mtl.mm b/impeller/renderer/backend/metal/device_buffer_mtl.mm index a8534c9a5da2a..7e4493637c7eb 100644 --- a/impeller/renderer/backend/metal/device_buffer_mtl.mm +++ b/impeller/renderer/backend/metal/device_buffer_mtl.mm @@ -50,7 +50,7 @@ if (!texture) { return nullptr; } - return std::make_shared(descriptor, texture); + return TextureMTL::BetterName(descriptor, texture); } [[nodiscard]] bool DeviceBufferMTL::OnCopyHostBuffer(const uint8_t* source, diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm index beb900a89be13..6a74feee21d1b 100644 --- a/impeller/renderer/backend/metal/surface_mtl.mm +++ b/impeller/renderer/backend/metal/surface_mtl.mm @@ -79,7 +79,7 @@ resolve_tex_desc.compression_type = CompressionType::kLossy; resolve_tex = allocator.CreateTexture(resolve_tex_desc); } else { - resolve_tex = std::make_shared(resolve_tex_desc, texture); + resolve_tex = TextureMTL::BetterName(resolve_tex_desc, texture); } if (!resolve_tex) { diff --git a/impeller/renderer/backend/metal/texture_mtl.h b/impeller/renderer/backend/metal/texture_mtl.h index 9fa5512973c16..e34f543969a6e 100644 --- a/impeller/renderer/backend/metal/texture_mtl.h +++ b/impeller/renderer/backend/metal/texture_mtl.h @@ -10,55 +10,39 @@ #include "impeller/base/backend_cast.h" #include "impeller/core/texture.h" +@protocol CAMetalDrawable; + namespace impeller { -class TextureMTL final : public Texture, - public BackendCast { +class TextureMTL : public Texture, public BackendCast { public: - TextureMTL(TextureDescriptor desc, - id texture, - bool wrapped = false); - static std::shared_ptr Wrapper( TextureDescriptor desc, id texture, std::function deletion_proc = nullptr); - // |Texture| - ~TextureMTL() override; - - id GetMTLTexture() const; - - bool IsWrapped() const; - - bool GenerateMipmap(id encoder); - - private: - id texture_ = nullptr; - bool is_valid_ = false; - bool is_wrapped_ = false; - - // |Texture| - void SetLabel(std::string_view label) override; + static std::shared_ptr BetterName(TextureDescriptor desc, + id texture); - // |Texture| - bool OnSetContents(const uint8_t* contents, - size_t length, - size_t slice) override; + virtual id GetMTLTexture() const = 0; - // |Texture| - bool OnSetContents(std::shared_ptr mapping, - size_t slice) override; + /// @brief Return the backing Metal drawable (if it has been acquired) or + /// otherwise block on drawable aquisition. + /// + /// This is only valud for textures that return true from + /// [IsDrawableBacked]. + virtual id WaitForNextDrawable() const = 0; - // |Texture| - bool IsValid() const override; + virtual bool IsWrapped() const = 0; - // |Texture| - ISize GetSize() const override; + /// @brief Whether or not this texture is backed by a CAMetalLayer and needs + /// to acquire a drawable before a texture can be returned. + virtual bool IsDrawableBacked() const = 0; - TextureMTL(const TextureMTL&) = delete; + virtual bool GenerateMipmap(id encoder) = 0; - TextureMTL& operator=(const TextureMTL&) = delete; + protected: + explicit TextureMTL(const TextureDescriptor& desc); }; } // namespace impeller diff --git a/impeller/renderer/backend/metal/texture_mtl.mm b/impeller/renderer/backend/metal/texture_mtl.mm index a2c2d95481528..5b53eda7af527 100644 --- a/impeller/renderer/backend/metal/texture_mtl.mm +++ b/impeller/renderer/backend/metal/texture_mtl.mm @@ -4,120 +4,262 @@ #include "impeller/renderer/backend/metal/texture_mtl.h" +#include + +#include "fml/synchronization/count_down_latch.h" +#include "fml/trace_event.h" #include "impeller/base/validation.h" #include "impeller/core/texture_descriptor.h" namespace impeller { -std::shared_ptr WrapperMTL(TextureDescriptor desc, - const void* mtl_texture, - std::function deletion_proc) { - return TextureMTL::Wrapper(desc, (__bridge id)mtl_texture, - std::move(deletion_proc)); -} +TextureMTL::TextureMTL(const TextureDescriptor& desc) : Texture(desc) {} -TextureMTL::TextureMTL(TextureDescriptor p_desc, +/// @brief an implementation of TextureMTL that is backed by a real Metal +/// texture. +class BetterNameTextureMTL final : public TextureMTL { + public: + BetterNameTextureMTL(TextureDescriptor p_desc, id texture, bool wrapped) - : Texture(p_desc), texture_(texture) { - const auto& desc = GetTextureDescriptor(); + : TextureMTL(p_desc), texture_(texture) { + const auto& desc = GetTextureDescriptor(); + + if (!desc.IsValid() || !texture_) { + return; + } - if (!desc.IsValid() || !texture_) { - return; + if (desc.size != GetSize()) { + VALIDATION_LOG + << "The texture and its descriptor disagree about its size."; + return; + } + + is_wrapped_ = wrapped; + is_valid_ = true; } - if (desc.size != GetSize()) { - VALIDATION_LOG << "The texture and its descriptor disagree about its size."; - return; + // |Texture| + ~BetterNameTextureMTL() override = default; + + private: + id texture_ = nullptr; + bool is_valid_ = false; + bool is_wrapped_ = false; + + // |TextureMTL| + id GetMTLTexture() const override { return texture_; } + + // |TextureMTL| + bool IsWrapped() const override { return is_wrapped_; } + + // |TextureMTL| + bool IsDrawableBacked() const override { return false; } + + // |TextureMTL| + id WaitForNextDrawable() const override { return nil; } + + bool GenerateMipmap(id encoder) override { + if (!texture_) { + return false; + } + + [encoder generateMipmapsForTexture:texture_]; + mipmap_generated_ = true; + + return true; } - is_wrapped_ = wrapped; - is_valid_ = true; -} + // |Texture| + void SetLabel(std::string_view label) override { + [texture_ setLabel:@(label.data())]; + } + // |Texture| + bool OnSetContents(const uint8_t* contents, + size_t length, + size_t slice) override { + if (!IsValid() || !contents || is_wrapped_) { + return false; + } -std::shared_ptr TextureMTL::Wrapper( - TextureDescriptor desc, - id texture, - std::function deletion_proc) { - if (deletion_proc) { - return std::shared_ptr( - new TextureMTL(desc, texture, true), - [deletion_proc = std::move(deletion_proc)](TextureMTL* t) { - deletion_proc(); - delete t; - }); + const auto& desc = GetTextureDescriptor(); + + // Out of bounds access. + if (length != desc.GetByteSizeOfBaseMipLevel()) { + return false; + } + + const auto region = + MTLRegionMake2D(0u, 0u, desc.size.width, desc.size.height); + [texture_ replaceRegion:region // + mipmapLevel:0u // + slice:slice // + withBytes:contents // + bytesPerRow:desc.GetBytesPerRow() // + bytesPerImage:desc.GetByteSizeOfBaseMipLevel() // + ]; + + return true; } - return std::shared_ptr(new TextureMTL(desc, texture, true)); -} -TextureMTL::~TextureMTL() = default; + // |Texture| + bool OnSetContents(std::shared_ptr mapping, + size_t slice) override { + // Metal has no threading restrictions. So we can pass this data along to + // the client rendering API immediately. + return OnSetContents(mapping->GetMapping(), mapping->GetSize(), slice); + } -void TextureMTL::SetLabel(std::string_view label) { - [texture_ setLabel:@(label.data())]; -} + // |Texture| + bool IsValid() const override { return is_valid_; } -// |Texture| -bool TextureMTL::OnSetContents(std::shared_ptr mapping, - size_t slice) { - // Metal has no threading restrictions. So we can pass this data along to the - // client rendering API immediately. - return OnSetContents(mapping->GetMapping(), mapping->GetSize(), slice); -} + // |Texture| + ISize GetSize() const override { + return {static_cast(texture_.width), + static_cast(texture_.height)}; + } -// |Texture| -bool TextureMTL::OnSetContents(const uint8_t* contents, - size_t length, - size_t slice) { - if (!IsValid() || !contents || is_wrapped_) { - return false; + BetterNameTextureMTL(const BetterNameTextureMTL&) = delete; + + BetterNameTextureMTL& operator=(const BetterNameTextureMTL&) = delete; +}; + +/// @brief an implementation of TextureMTL that is backed by a CAMetalLayer. +/// +/// When asked for a Metal texture, this texture class will reference and cache +/// the next drawable of the provided metal layer. This class is intended to be +/// used to allow referencing the onscreen Metal drawable without blocking on +/// drawable acquisition. +class DrawableTextureMTL final : public TextureMTL { + public: + DrawableTextureMTL(const TextureDescriptor& p_desc, CAMetalLayer* layer) + : TextureMTL(p_desc), + layer_(layer), + drawable_latch_(std::make_shared(1u)) { + const auto& desc = GetTextureDescriptor(); + + if (!desc.IsValid() || !layer) { + return; + } + + if (ISize{static_cast(layer.drawableSize.width), + static_cast(layer.drawableSize.height)} != + GetSize()) { + VALIDATION_LOG + << "The texture and its descriptor disagree about its size."; + return; + } + + is_valid_ = true; } - const auto& desc = GetTextureDescriptor(); + // |Texture| + ~DrawableTextureMTL() override = default; - // Out of bounds access. - if (length != desc.GetByteSizeOfBaseMipLevel()) { - return false; + private: + CAMetalLayer* layer_; + bool is_valid_ = false; + + mutable bool acquired_drawable_ = false; + mutable id drawable_ = nullptr; + mutable id texture_ = nullptr; + mutable std::shared_ptr drawable_latch_; + + id WaitForNextDrawable() const override { + drawable_latch_->Wait(); + return drawable_; } - const auto region = - MTLRegionMake2D(0u, 0u, desc.size.width, desc.size.height); - [texture_ replaceRegion:region // - mipmapLevel:0u // - slice:slice // - withBytes:contents // - bytesPerRow:desc.GetBytesPerRow() // - bytesPerImage:desc.GetByteSizeOfBaseMipLevel() // - ]; - - return true; -} + // |TextureMTL| + id GetMTLTexture() const override { + // If the drawable has already been acquired, then return the cached Metal + // texture. If next drawable returned a nil texture, this value will be nil. + if (acquired_drawable_) { + return texture_; + } -ISize TextureMTL::GetSize() const { - return {static_cast(texture_.width), - static_cast(texture_.height)}; -} + id current_drawable = nil; + { + TRACE_EVENT0("impeller", "WaitForNextDrawable"); + current_drawable = [layer_ nextDrawable]; + } + acquired_drawable_ = true; + drawable_latch_->CountDown(); -id TextureMTL::GetMTLTexture() const { - return texture_; -} + if (!current_drawable) { + VALIDATION_LOG << "Could not acquire current drawable."; + return nullptr; + } + drawable_ = current_drawable; + texture_ = drawable_.texture; + return texture_; + } -bool TextureMTL::IsValid() const { - return is_valid_; -} + // |TextureMTL| + bool IsWrapped() const override { return false; } -bool TextureMTL::IsWrapped() const { - return is_wrapped_; -} + // |TextureMTL| + bool IsDrawableBacked() const override { return true; } + + bool GenerateMipmap(id encoder) override { + VALIDATION_LOG << "Cannot generate mipmaps for a drawable-backed texture."; + return false; + } + + // |Texture| + void SetLabel(std::string_view label) override { + // NOOP + } + + // |Texture| + bool OnSetContents(const uint8_t* contents, + size_t length, + size_t slice) override { + VALIDATION_LOG << "Cannot set contents for a drawable-backed texture."; + return false; + } -bool TextureMTL::GenerateMipmap(id encoder) { - if (!texture_) { + // |Texture| + bool OnSetContents(std::shared_ptr mapping, + size_t slice) override { + VALIDATION_LOG << "Cannot set contents for a drawable-backed texture."; return false; } - [encoder generateMipmapsForTexture:texture_]; - mipmap_generated_ = true; + // |Texture| + bool IsValid() const override { return is_valid_; } + + // |Texture| + ISize GetSize() const override { + return {static_cast(layer_.drawableSize.width), + static_cast(layer_.drawableSize.height)}; + } + + DrawableTextureMTL(const DrawableTextureMTL&) = delete; + + DrawableTextureMTL& operator=(const DrawableTextureMTL&) = delete; +}; + +// (__bridge id)mtl_texture, + +std::shared_ptr TextureMTL::Wrapper( + TextureDescriptor desc, + id texture, + std::function deletion_proc) { + if (deletion_proc) { + return std::shared_ptr( + new BetterNameTextureMTL(desc, texture, true), + [deletion_proc = std::move(deletion_proc)](TextureMTL* t) { + deletion_proc(); + delete t; + }); + } + return std::make_shared(desc, texture, true); +} - return true; +std::shared_ptr TextureMTL::BetterName(TextureDescriptor desc, + id texture) { + return std::make_shared(desc, texture, false); } } // namespace impeller From a70d76c2c54c9a09ebc07a9cfb67da738e26f995 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 14 Nov 2023 18:43:39 -0800 Subject: [PATCH 02/17] ++ --- .../renderer/backend/metal/allocator_mtl.mm | 2 +- .../backend/metal/device_buffer_mtl.mm | 2 +- .../renderer/backend/metal/surface_mtl.mm | 2 +- impeller/renderer/backend/metal/texture_mtl.h | 10 ++++++++-- .../renderer/backend/metal/texture_mtl.mm | 19 +++++++++++++++++-- 5 files changed, 28 insertions(+), 7 deletions(-) diff --git a/impeller/renderer/backend/metal/allocator_mtl.mm b/impeller/renderer/backend/metal/allocator_mtl.mm index 57f6f6c64a19c..150576699df62 100644 --- a/impeller/renderer/backend/metal/allocator_mtl.mm +++ b/impeller/renderer/backend/metal/allocator_mtl.mm @@ -217,7 +217,7 @@ static MTLStorageMode ToMTLStorageMode(StorageMode mode, if (!texture) { return nullptr; } - return TextureMTL::BetterName(desc, texture); + return TextureMTL::Create(desc, texture); } uint16_t AllocatorMTL::MinimumBytesPerRow(PixelFormat format) const { diff --git a/impeller/renderer/backend/metal/device_buffer_mtl.mm b/impeller/renderer/backend/metal/device_buffer_mtl.mm index 7e4493637c7eb..860e216264975 100644 --- a/impeller/renderer/backend/metal/device_buffer_mtl.mm +++ b/impeller/renderer/backend/metal/device_buffer_mtl.mm @@ -50,7 +50,7 @@ if (!texture) { return nullptr; } - return TextureMTL::BetterName(descriptor, texture); + return TextureMTL::Create(descriptor, texture); } [[nodiscard]] bool DeviceBufferMTL::OnCopyHostBuffer(const uint8_t* source, diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm index 6a74feee21d1b..1d73324e3f734 100644 --- a/impeller/renderer/backend/metal/surface_mtl.mm +++ b/impeller/renderer/backend/metal/surface_mtl.mm @@ -79,7 +79,7 @@ resolve_tex_desc.compression_type = CompressionType::kLossy; resolve_tex = allocator.CreateTexture(resolve_tex_desc); } else { - resolve_tex = TextureMTL::BetterName(resolve_tex_desc, texture); + resolve_tex = TextureMTL::Create(resolve_tex_desc, texture); } if (!resolve_tex) { diff --git a/impeller/renderer/backend/metal/texture_mtl.h b/impeller/renderer/backend/metal/texture_mtl.h index e34f543969a6e..8b2b9319723c6 100644 --- a/impeller/renderer/backend/metal/texture_mtl.h +++ b/impeller/renderer/backend/metal/texture_mtl.h @@ -11,6 +11,7 @@ #include "impeller/core/texture.h" @protocol CAMetalDrawable; +@class CAMetalLayer; namespace impeller { @@ -21,8 +22,13 @@ class TextureMTL : public Texture, public BackendCast { id texture, std::function deletion_proc = nullptr); - static std::shared_ptr BetterName(TextureDescriptor desc, - id texture); + /// @brief Create a new metal texture. + static std::shared_ptr Create(TextureDescriptor desc, + id texture); + + /// @brief Create a new texture backed by a lazily acquired drawable + static std::shared_ptr WrapDrawable(TextureDescriptor desc, + CAMetalLayer* layer); virtual id GetMTLTexture() const = 0; diff --git a/impeller/renderer/backend/metal/texture_mtl.mm b/impeller/renderer/backend/metal/texture_mtl.mm index 5b53eda7af527..1d79bdf611b36 100644 --- a/impeller/renderer/backend/metal/texture_mtl.mm +++ b/impeller/renderer/backend/metal/texture_mtl.mm @@ -5,6 +5,7 @@ #include "impeller/renderer/backend/metal/texture_mtl.h" #include +#include #include "fml/synchronization/count_down_latch.h" #include "fml/trace_event.h" @@ -13,6 +14,13 @@ namespace impeller { +#pragma GCC diagnostic push +// Disable the diagnostic for iOS Simulators. Metal without emulation isn't +// available prior to iOS 13 and that's what the simulator headers say when +// support for CAMetalLayer begins. CAMetalLayer is available on iOS 8.0 and +// above which is well below Flutters support level. +#pragma GCC diagnostic ignored "-Wunguarded-availability-new" + TextureMTL::TextureMTL(const TextureDescriptor& desc) : Texture(desc) {} /// @brief an implementation of TextureMTL that is backed by a real Metal @@ -257,9 +265,16 @@ new BetterNameTextureMTL(desc, texture, true), return std::make_shared(desc, texture, true); } -std::shared_ptr TextureMTL::BetterName(TextureDescriptor desc, - id texture) { +std::shared_ptr TextureMTL::Create(TextureDescriptor desc, + id texture) { return std::make_shared(desc, texture, false); } +std::shared_ptr TextureMTL::WrapDrawable(TextureDescriptor desc, + CAMetalLayer* layer) { + return std::make_shared(desc, layer); +} + +#pragma GCC diagnostic pop + } // namespace impeller From 90ff1c9491ff12a8ebfb41404c3025e02bd33e23 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 14 Nov 2023 18:53:56 -0800 Subject: [PATCH 03/17] ++ --- impeller/renderer/backend/metal/texture_mtl.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/impeller/renderer/backend/metal/texture_mtl.h b/impeller/renderer/backend/metal/texture_mtl.h index 8b2b9319723c6..91adb6afe9485 100644 --- a/impeller/renderer/backend/metal/texture_mtl.h +++ b/impeller/renderer/backend/metal/texture_mtl.h @@ -26,9 +26,17 @@ class TextureMTL : public Texture, public BackendCast { static std::shared_ptr Create(TextureDescriptor desc, id texture); +#pragma GCC diagnostic push +// Disable the diagnostic for iOS Simulators. Metal without emulation isn't +// available prior to iOS 13 and that's what the simulator headers say when +// support for CAMetalLayer begins. CAMetalLayer is available on iOS 8.0 and +// above which is well below Flutters support level. +#pragma GCC diagnostic ignored "-Wunguarded-availability-new" + /// @brief Create a new texture backed by a lazily acquired drawable static std::shared_ptr WrapDrawable(TextureDescriptor desc, CAMetalLayer* layer); +#pragma GCC diagnostic pop virtual id GetMTLTexture() const = 0; From 980767bc7142303055c020149a7c54a85dab6558 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 07:41:55 -0800 Subject: [PATCH 04/17] add unittests. --- impeller/BUILD.gn | 1 + impeller/renderer/backend/metal/BUILD.gn | 19 +++++ .../renderer/backend/metal/texture_mtl.mm | 14 ++-- .../backend/metal/texture_mtl_unittests.mm | 72 +++++++++++++++++++ 4 files changed, 98 insertions(+), 8 deletions(-) create mode 100644 impeller/renderer/backend/metal/texture_mtl_unittests.mm diff --git a/impeller/BUILD.gn b/impeller/BUILD.gn index 2eaab0e2b76a8..73cd6307f9b1d 100644 --- a/impeller/BUILD.gn +++ b/impeller/BUILD.gn @@ -91,6 +91,7 @@ impeller_component("impeller_unittests") { "scene/importer:importer_unittests", "shader_archive:shader_archive_unittests", "tessellator:tessellator_unittests", + "renderer/backend/metal:metal_unittests", ] if (impeller_supports_rendering) { diff --git a/impeller/renderer/backend/metal/BUILD.gn b/impeller/renderer/backend/metal/BUILD.gn index 254cae97f063a..2f9afaad7c05a 100644 --- a/impeller/renderer/backend/metal/BUILD.gn +++ b/impeller/renderer/backend/metal/BUILD.gn @@ -57,3 +57,22 @@ impeller_component("metal") { frameworks = [ "Metal.framework" ] } + +impeller_component("metal_unittests") { + testonly = true + + sources = [ + "texture_mtl_unittests.mm" + ] + + deps = [ + ":metal", + "//flutter/testing:testing_lib", + ] + + frameworks = [ + "AppKit.framework", + "QuartzCore.framework", + ] + +} diff --git a/impeller/renderer/backend/metal/texture_mtl.mm b/impeller/renderer/backend/metal/texture_mtl.mm index 1d79bdf611b36..18b98f8191586 100644 --- a/impeller/renderer/backend/metal/texture_mtl.mm +++ b/impeller/renderer/backend/metal/texture_mtl.mm @@ -11,6 +11,7 @@ #include "fml/trace_event.h" #include "impeller/base/validation.h" #include "impeller/core/texture_descriptor.h" +#include "impeller/renderer/backend/metal/formats_mtl.h" namespace impeller { @@ -150,11 +151,10 @@ ISize GetSize() const override { return; } - if (ISize{static_cast(layer.drawableSize.width), - static_cast(layer.drawableSize.height)} != - GetSize()) { - VALIDATION_LOG - << "The texture and its descriptor disagree about its size."; + if (desc.size != GetSize() || + ToMTLPixelFormat(desc.format) != layer_.pixelFormat) { + VALIDATION_LOG << "The texture and its descriptor disagree about its " + "size and/or format."; return; } @@ -215,9 +215,7 @@ bool GenerateMipmap(id encoder) override { } // |Texture| - void SetLabel(std::string_view label) override { - // NOOP - } + void SetLabel(std::string_view label) override {} // |Texture| bool OnSetContents(const uint8_t* contents, diff --git a/impeller/renderer/backend/metal/texture_mtl_unittests.mm b/impeller/renderer/backend/metal/texture_mtl_unittests.mm new file mode 100644 index 0000000000000..f1692e96db738 --- /dev/null +++ b/impeller/renderer/backend/metal/texture_mtl_unittests.mm @@ -0,0 +1,72 @@ +// 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" +#include "impeller/core/formats.h" +#include "impeller/core/texture_descriptor.h" +#include "impeller/renderer/backend/metal/formats_mtl.h" +#include "impeller/renderer/backend/metal/texture_mtl.h" +#include "impeller/renderer/capabilities.h" + +#include +#include + +#include "gtest/gtest.h" + +namespace impeller { +namespace testing { + +TEST(TextureMTL, CreateFromDrawable) { + auto device = MTLCreateSystemDefaultDevice(); + auto layer = [[CAMetalLayer alloc] init]; + layer.device = device; + layer.drawableSize = CGSize{100, 100}; + layer.pixelFormat = ToMTLPixelFormat(PixelFormat::kR8G8B8A8UNormInt); + + TextureDescriptor desc; + desc.size = {100, 100}; + desc.format = PixelFormat::kR8G8B8A8UNormInt; + + auto drawable_texture = TextureMTL::WrapDrawable(desc, layer); + + ASSERT_TRUE(drawable_texture->IsValid()); + EXPECT_TRUE(drawable_texture->IsDrawableBacked()); + + // Spawn a thread and acquire the drawable in the thread. + auto thread = std::thread([&drawable_texture]() { + // Force the drawable to be acquired. + drawable_texture->GetMTLTexture(); + }); + thread.join(); + // Block until drawable is acquired. + EXPECT_TRUE(drawable_texture->WaitForNextDrawable() != nil); + // Drawable is cached. + EXPECT_TRUE(drawable_texture->GetMTLTexture() != nil); +} + +TEST(TextureMTL, CreateFromInvalidDescriptor) { + auto device = MTLCreateSystemDefaultDevice(); + auto layer = [[CAMetalLayer alloc] init]; + layer.device = device; + layer.drawableSize = CGSize{100, 100}; + layer.pixelFormat = ToMTLPixelFormat(PixelFormat::kR8G8B8A8UNormInt); + + TextureDescriptor desc; + // Size is mismatched. + desc.size = {200, 200}; + desc.format = PixelFormat::kR8G8B8A8UNormInt; + + auto drawable_texture_a = TextureMTL::WrapDrawable(desc, layer); + EXPECT_FALSE(drawable_texture_a->IsValid()); + + desc.size = {100, 100}; + // Pixel format is mismatched. + desc.format = PixelFormat::kD24UnormS8Uint; + + auto drawable_texture_b = TextureMTL::WrapDrawable(desc, layer); + EXPECT_FALSE(drawable_texture_b->IsValid()); +} + +} // namespace testing +} // namespace impeller \ No newline at end of file From acb7c680261ffd833e0436a9250e5af173566cf0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 07:53:35 -0800 Subject: [PATCH 05/17] remove mutable. --- impeller/BUILD.gn | 2 +- impeller/renderer/backend/metal/BUILD.gn | 5 +- .../renderer/backend/metal/texture_mtl.mm | 68 ++++++++++++------- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/impeller/BUILD.gn b/impeller/BUILD.gn index 73cd6307f9b1d..3237235fe9154 100644 --- a/impeller/BUILD.gn +++ b/impeller/BUILD.gn @@ -87,11 +87,11 @@ impeller_component("impeller_unittests") { "core:allocator_unittests", "display_list:skia_conversions_unittests", "geometry:geometry_unittests", + "renderer/backend/metal:metal_unittests", "runtime_stage:runtime_stage_unittests", "scene/importer:importer_unittests", "shader_archive:shader_archive_unittests", "tessellator:tessellator_unittests", - "renderer/backend/metal:metal_unittests", ] if (impeller_supports_rendering) { diff --git a/impeller/renderer/backend/metal/BUILD.gn b/impeller/renderer/backend/metal/BUILD.gn index 2f9afaad7c05a..7a7b6b115d6d9 100644 --- a/impeller/renderer/backend/metal/BUILD.gn +++ b/impeller/renderer/backend/metal/BUILD.gn @@ -61,9 +61,7 @@ impeller_component("metal") { impeller_component("metal_unittests") { testonly = true - sources = [ - "texture_mtl_unittests.mm" - ] + sources = [ "texture_mtl_unittests.mm" ] deps = [ ":metal", @@ -74,5 +72,4 @@ impeller_component("metal_unittests") { "AppKit.framework", "QuartzCore.framework", ] - } diff --git a/impeller/renderer/backend/metal/texture_mtl.mm b/impeller/renderer/backend/metal/texture_mtl.mm index 18b98f8191586..8c7c84eb4db6b 100644 --- a/impeller/renderer/backend/metal/texture_mtl.mm +++ b/impeller/renderer/backend/metal/texture_mtl.mm @@ -133,6 +133,38 @@ ISize GetSize() const override { BetterNameTextureMTL& operator=(const BetterNameTextureMTL&) = delete; }; +/// @brief A class that holds the mutable state of the DrawableTextureMTL. +class DrawableHolder { + public: + DrawableHolder() = default; + + ~DrawableHolder() = default; + + bool AcquireNextDrawable(CAMetalLayer* layer) { + id current_drawable = nil; + { + TRACE_EVENT0("impeller", "WaitForNextDrawable"); + current_drawable = [layer nextDrawable]; + } + has_drawable_ = true; + drawable_latch_->CountDown(); + if (!current_drawable) { + VALIDATION_LOG << "Could not acquire current drawable."; + return false; + } + drawable_ = current_drawable; + texture_ = drawable_.texture; + + return true; + } + + bool has_drawable_ = false; + id drawable_ = nullptr; + id texture_ = nullptr; + std::shared_ptr drawable_latch_ = + std::make_shared(1u); +}; + /// @brief an implementation of TextureMTL that is backed by a CAMetalLayer. /// /// When asked for a Metal texture, this texture class will reference and cache @@ -144,7 +176,7 @@ ISize GetSize() const override { DrawableTextureMTL(const TextureDescriptor& p_desc, CAMetalLayer* layer) : TextureMTL(p_desc), layer_(layer), - drawable_latch_(std::make_shared(1u)) { + drawable_holder_(std::make_shared()) { const auto& desc = GetTextureDescriptor(); if (!desc.IsValid() || !layer) { @@ -167,40 +199,28 @@ ISize GetSize() const override { private: CAMetalLayer* layer_; bool is_valid_ = false; - - mutable bool acquired_drawable_ = false; - mutable id drawable_ = nullptr; - mutable id texture_ = nullptr; - mutable std::shared_ptr drawable_latch_; + std::shared_ptr drawable_holder_; id WaitForNextDrawable() const override { - drawable_latch_->Wait(); - return drawable_; + if (drawable_holder_->has_drawable_) { + return drawable_holder_->drawable_; + } + drawable_holder_->drawable_latch_->Wait(); + return drawable_holder_->drawable_; } // |TextureMTL| id GetMTLTexture() const override { // If the drawable has already been acquired, then return the cached Metal // texture. If next drawable returned a nil texture, this value will be nil. - if (acquired_drawable_) { - return texture_; - } - - id current_drawable = nil; - { - TRACE_EVENT0("impeller", "WaitForNextDrawable"); - current_drawable = [layer_ nextDrawable]; + if (drawable_holder_->has_drawable_) { + return drawable_holder_->texture_; } - acquired_drawable_ = true; - drawable_latch_->CountDown(); - if (!current_drawable) { - VALIDATION_LOG << "Could not acquire current drawable."; - return nullptr; + if (drawable_holder_->AcquireNextDrawable(layer_)) { + return drawable_holder_->texture_; } - drawable_ = current_drawable; - texture_ = drawable_.texture; - return texture_; + return nullptr; } // |TextureMTL| From 915b592dd434ba0b98bd0d4eb49fda9a2275a863 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 08:08:13 -0800 Subject: [PATCH 06/17] ++ --- ci/licenses_golden/excluded_files | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index e518702b32318..9a55770ceb142 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -162,6 +162,7 @@ ../../../flutter/impeller/image/README.md ../../../flutter/impeller/playground ../../../flutter/impeller/renderer/backend/gles/test +../../../flutter/impeller/renderer/backend/metal/texture_mtl_unittests.mm ../../../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 From 5651c46e08ddfa1069ad20b1c9d61055c7869f1f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 08:42:50 -0800 Subject: [PATCH 07/17] ++ --- impeller/renderer/backend/metal/texture_mtl_unittests.mm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/impeller/renderer/backend/metal/texture_mtl_unittests.mm b/impeller/renderer/backend/metal/texture_mtl_unittests.mm index f1692e96db738..3d2ca870de421 100644 --- a/impeller/renderer/backend/metal/texture_mtl_unittests.mm +++ b/impeller/renderer/backend/metal/texture_mtl_unittests.mm @@ -22,11 +22,11 @@ auto layer = [[CAMetalLayer alloc] init]; layer.device = device; layer.drawableSize = CGSize{100, 100}; - layer.pixelFormat = ToMTLPixelFormat(PixelFormat::kR8G8B8A8UNormInt); + layer.pixelFormat = ToMTLPixelFormat(PixelFormat::kB8G8R8A8UNormInt); TextureDescriptor desc; desc.size = {100, 100}; - desc.format = PixelFormat::kR8G8B8A8UNormInt; + desc.format = PixelFormat::kB8G8R8A8UNormInt; auto drawable_texture = TextureMTL::WrapDrawable(desc, layer); @@ -50,12 +50,12 @@ auto layer = [[CAMetalLayer alloc] init]; layer.device = device; layer.drawableSize = CGSize{100, 100}; - layer.pixelFormat = ToMTLPixelFormat(PixelFormat::kR8G8B8A8UNormInt); + layer.pixelFormat = ToMTLPixelFormat(PixelFormat::kB8G8R8A8UNormInt); TextureDescriptor desc; // Size is mismatched. desc.size = {200, 200}; - desc.format = PixelFormat::kR8G8B8A8UNormInt; + desc.format = PixelFormat::kB8G8R8A8UNormInt; auto drawable_texture_a = TextureMTL::WrapDrawable(desc, layer); EXPECT_FALSE(drawable_texture_a->IsValid()); From f3ccf9ab7bc072127ec0c6b97b1caa34cfe9d789 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 09:02:02 -0800 Subject: [PATCH 08/17] ++ --- impeller/renderer/backend/metal/texture_mtl_unittests.mm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/impeller/renderer/backend/metal/texture_mtl_unittests.mm b/impeller/renderer/backend/metal/texture_mtl_unittests.mm index 3d2ca870de421..7f7406a3c68b6 100644 --- a/impeller/renderer/backend/metal/texture_mtl_unittests.mm +++ b/impeller/renderer/backend/metal/texture_mtl_unittests.mm @@ -46,6 +46,8 @@ } TEST(TextureMTL, CreateFromInvalidDescriptor) { + ScopedValidationDisable validation; + auto device = MTLCreateSystemDefaultDevice(); auto layer = [[CAMetalLayer alloc] init]; layer.device = device; From 3063d65bcf88e34bcb4a79e3a08935b0c9ac14d5 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 11:02:53 -0800 Subject: [PATCH 09/17] revert texture changes. --- impeller/renderer/backend/metal/texture_mtl.h | 64 ++-- .../renderer/backend/metal/texture_mtl.mm | 335 +++++------------- 2 files changed, 113 insertions(+), 286 deletions(-) diff --git a/impeller/renderer/backend/metal/texture_mtl.h b/impeller/renderer/backend/metal/texture_mtl.h index 91adb6afe9485..9fa5512973c16 100644 --- a/impeller/renderer/backend/metal/texture_mtl.h +++ b/impeller/renderer/backend/metal/texture_mtl.h @@ -10,53 +10,55 @@ #include "impeller/base/backend_cast.h" #include "impeller/core/texture.h" -@protocol CAMetalDrawable; -@class CAMetalLayer; - namespace impeller { -class TextureMTL : public Texture, public BackendCast { +class TextureMTL final : public Texture, + public BackendCast { public: + TextureMTL(TextureDescriptor desc, + id texture, + bool wrapped = false); + static std::shared_ptr Wrapper( TextureDescriptor desc, id texture, std::function deletion_proc = nullptr); - /// @brief Create a new metal texture. - static std::shared_ptr Create(TextureDescriptor desc, - id texture); + // |Texture| + ~TextureMTL() override; + + id GetMTLTexture() const; + + bool IsWrapped() const; + + bool GenerateMipmap(id encoder); -#pragma GCC diagnostic push -// Disable the diagnostic for iOS Simulators. Metal without emulation isn't -// available prior to iOS 13 and that's what the simulator headers say when -// support for CAMetalLayer begins. CAMetalLayer is available on iOS 8.0 and -// above which is well below Flutters support level. -#pragma GCC diagnostic ignored "-Wunguarded-availability-new" + private: + id texture_ = nullptr; + bool is_valid_ = false; + bool is_wrapped_ = false; - /// @brief Create a new texture backed by a lazily acquired drawable - static std::shared_ptr WrapDrawable(TextureDescriptor desc, - CAMetalLayer* layer); -#pragma GCC diagnostic pop + // |Texture| + void SetLabel(std::string_view label) override; - virtual id GetMTLTexture() const = 0; + // |Texture| + bool OnSetContents(const uint8_t* contents, + size_t length, + size_t slice) override; - /// @brief Return the backing Metal drawable (if it has been acquired) or - /// otherwise block on drawable aquisition. - /// - /// This is only valud for textures that return true from - /// [IsDrawableBacked]. - virtual id WaitForNextDrawable() const = 0; + // |Texture| + bool OnSetContents(std::shared_ptr mapping, + size_t slice) override; - virtual bool IsWrapped() const = 0; + // |Texture| + bool IsValid() const override; - /// @brief Whether or not this texture is backed by a CAMetalLayer and needs - /// to acquire a drawable before a texture can be returned. - virtual bool IsDrawableBacked() const = 0; + // |Texture| + ISize GetSize() const override; - virtual bool GenerateMipmap(id encoder) = 0; + TextureMTL(const TextureMTL&) = delete; - protected: - explicit TextureMTL(const TextureDescriptor& desc); + TextureMTL& operator=(const TextureMTL&) = delete; }; } // namespace impeller diff --git a/impeller/renderer/backend/metal/texture_mtl.mm b/impeller/renderer/backend/metal/texture_mtl.mm index 8c7c84eb4db6b..a2c2d95481528 100644 --- a/impeller/renderer/backend/metal/texture_mtl.mm +++ b/impeller/renderer/backend/metal/texture_mtl.mm @@ -4,295 +4,120 @@ #include "impeller/renderer/backend/metal/texture_mtl.h" -#include -#include - -#include "fml/synchronization/count_down_latch.h" -#include "fml/trace_event.h" #include "impeller/base/validation.h" #include "impeller/core/texture_descriptor.h" -#include "impeller/renderer/backend/metal/formats_mtl.h" namespace impeller { -#pragma GCC diagnostic push -// Disable the diagnostic for iOS Simulators. Metal without emulation isn't -// available prior to iOS 13 and that's what the simulator headers say when -// support for CAMetalLayer begins. CAMetalLayer is available on iOS 8.0 and -// above which is well below Flutters support level. -#pragma GCC diagnostic ignored "-Wunguarded-availability-new" - -TextureMTL::TextureMTL(const TextureDescriptor& desc) : Texture(desc) {} +std::shared_ptr WrapperMTL(TextureDescriptor desc, + const void* mtl_texture, + std::function deletion_proc) { + return TextureMTL::Wrapper(desc, (__bridge id)mtl_texture, + std::move(deletion_proc)); +} -/// @brief an implementation of TextureMTL that is backed by a real Metal -/// texture. -class BetterNameTextureMTL final : public TextureMTL { - public: - BetterNameTextureMTL(TextureDescriptor p_desc, +TextureMTL::TextureMTL(TextureDescriptor p_desc, id texture, bool wrapped) - : TextureMTL(p_desc), texture_(texture) { - const auto& desc = GetTextureDescriptor(); - - if (!desc.IsValid() || !texture_) { - return; - } - - if (desc.size != GetSize()) { - VALIDATION_LOG - << "The texture and its descriptor disagree about its size."; - return; - } - - is_wrapped_ = wrapped; - is_valid_ = true; - } - - // |Texture| - ~BetterNameTextureMTL() override = default; - - private: - id texture_ = nullptr; - bool is_valid_ = false; - bool is_wrapped_ = false; - - // |TextureMTL| - id GetMTLTexture() const override { return texture_; } + : Texture(p_desc), texture_(texture) { + const auto& desc = GetTextureDescriptor(); - // |TextureMTL| - bool IsWrapped() const override { return is_wrapped_; } - - // |TextureMTL| - bool IsDrawableBacked() const override { return false; } - - // |TextureMTL| - id WaitForNextDrawable() const override { return nil; } - - bool GenerateMipmap(id encoder) override { - if (!texture_) { - return false; - } - - [encoder generateMipmapsForTexture:texture_]; - mipmap_generated_ = true; - - return true; + if (!desc.IsValid() || !texture_) { + return; } - // |Texture| - void SetLabel(std::string_view label) override { - [texture_ setLabel:@(label.data())]; - } - // |Texture| - bool OnSetContents(const uint8_t* contents, - size_t length, - size_t slice) override { - if (!IsValid() || !contents || is_wrapped_) { - return false; - } - - const auto& desc = GetTextureDescriptor(); - - // Out of bounds access. - if (length != desc.GetByteSizeOfBaseMipLevel()) { - return false; - } - - const auto region = - MTLRegionMake2D(0u, 0u, desc.size.width, desc.size.height); - [texture_ replaceRegion:region // - mipmapLevel:0u // - slice:slice // - withBytes:contents // - bytesPerRow:desc.GetBytesPerRow() // - bytesPerImage:desc.GetByteSizeOfBaseMipLevel() // - ]; - - return true; + if (desc.size != GetSize()) { + VALIDATION_LOG << "The texture and its descriptor disagree about its size."; + return; } - // |Texture| - bool OnSetContents(std::shared_ptr mapping, - size_t slice) override { - // Metal has no threading restrictions. So we can pass this data along to - // the client rendering API immediately. - return OnSetContents(mapping->GetMapping(), mapping->GetSize(), slice); - } - - // |Texture| - bool IsValid() const override { return is_valid_; } - - // |Texture| - ISize GetSize() const override { - return {static_cast(texture_.width), - static_cast(texture_.height)}; - } - - BetterNameTextureMTL(const BetterNameTextureMTL&) = delete; - - BetterNameTextureMTL& operator=(const BetterNameTextureMTL&) = delete; -}; - -/// @brief A class that holds the mutable state of the DrawableTextureMTL. -class DrawableHolder { - public: - DrawableHolder() = default; - - ~DrawableHolder() = default; - - bool AcquireNextDrawable(CAMetalLayer* layer) { - id current_drawable = nil; - { - TRACE_EVENT0("impeller", "WaitForNextDrawable"); - current_drawable = [layer nextDrawable]; - } - has_drawable_ = true; - drawable_latch_->CountDown(); - if (!current_drawable) { - VALIDATION_LOG << "Could not acquire current drawable."; - return false; - } - drawable_ = current_drawable; - texture_ = drawable_.texture; - - return true; - } - - bool has_drawable_ = false; - id drawable_ = nullptr; - id texture_ = nullptr; - std::shared_ptr drawable_latch_ = - std::make_shared(1u); -}; - -/// @brief an implementation of TextureMTL that is backed by a CAMetalLayer. -/// -/// When asked for a Metal texture, this texture class will reference and cache -/// the next drawable of the provided metal layer. This class is intended to be -/// used to allow referencing the onscreen Metal drawable without blocking on -/// drawable acquisition. -class DrawableTextureMTL final : public TextureMTL { - public: - DrawableTextureMTL(const TextureDescriptor& p_desc, CAMetalLayer* layer) - : TextureMTL(p_desc), - layer_(layer), - drawable_holder_(std::make_shared()) { - const auto& desc = GetTextureDescriptor(); - - if (!desc.IsValid() || !layer) { - return; - } - - if (desc.size != GetSize() || - ToMTLPixelFormat(desc.format) != layer_.pixelFormat) { - VALIDATION_LOG << "The texture and its descriptor disagree about its " - "size and/or format."; - return; - } - - is_valid_ = true; - } - - // |Texture| - ~DrawableTextureMTL() override = default; - - private: - CAMetalLayer* layer_; - bool is_valid_ = false; - std::shared_ptr drawable_holder_; + is_wrapped_ = wrapped; + is_valid_ = true; +} - id WaitForNextDrawable() const override { - if (drawable_holder_->has_drawable_) { - return drawable_holder_->drawable_; - } - drawable_holder_->drawable_latch_->Wait(); - return drawable_holder_->drawable_; +std::shared_ptr TextureMTL::Wrapper( + TextureDescriptor desc, + id texture, + std::function deletion_proc) { + if (deletion_proc) { + return std::shared_ptr( + new TextureMTL(desc, texture, true), + [deletion_proc = std::move(deletion_proc)](TextureMTL* t) { + deletion_proc(); + delete t; + }); } + return std::shared_ptr(new TextureMTL(desc, texture, true)); +} - // |TextureMTL| - id GetMTLTexture() const override { - // If the drawable has already been acquired, then return the cached Metal - // texture. If next drawable returned a nil texture, this value will be nil. - if (drawable_holder_->has_drawable_) { - return drawable_holder_->texture_; - } +TextureMTL::~TextureMTL() = default; - if (drawable_holder_->AcquireNextDrawable(layer_)) { - return drawable_holder_->texture_; - } - return nullptr; - } - - // |TextureMTL| - bool IsWrapped() const override { return false; } +void TextureMTL::SetLabel(std::string_view label) { + [texture_ setLabel:@(label.data())]; +} - // |TextureMTL| - bool IsDrawableBacked() const override { return true; } +// |Texture| +bool TextureMTL::OnSetContents(std::shared_ptr mapping, + size_t slice) { + // Metal has no threading restrictions. So we can pass this data along to the + // client rendering API immediately. + return OnSetContents(mapping->GetMapping(), mapping->GetSize(), slice); +} - bool GenerateMipmap(id encoder) override { - VALIDATION_LOG << "Cannot generate mipmaps for a drawable-backed texture."; +// |Texture| +bool TextureMTL::OnSetContents(const uint8_t* contents, + size_t length, + size_t slice) { + if (!IsValid() || !contents || is_wrapped_) { return false; } - // |Texture| - void SetLabel(std::string_view label) override {} + const auto& desc = GetTextureDescriptor(); - // |Texture| - bool OnSetContents(const uint8_t* contents, - size_t length, - size_t slice) override { - VALIDATION_LOG << "Cannot set contents for a drawable-backed texture."; + // Out of bounds access. + if (length != desc.GetByteSizeOfBaseMipLevel()) { return false; } - // |Texture| - bool OnSetContents(std::shared_ptr mapping, - size_t slice) override { - VALIDATION_LOG << "Cannot set contents for a drawable-backed texture."; - return false; - } - - // |Texture| - bool IsValid() const override { return is_valid_; } + const auto region = + MTLRegionMake2D(0u, 0u, desc.size.width, desc.size.height); + [texture_ replaceRegion:region // + mipmapLevel:0u // + slice:slice // + withBytes:contents // + bytesPerRow:desc.GetBytesPerRow() // + bytesPerImage:desc.GetByteSizeOfBaseMipLevel() // + ]; + + return true; +} - // |Texture| - ISize GetSize() const override { - return {static_cast(layer_.drawableSize.width), - static_cast(layer_.drawableSize.height)}; - } +ISize TextureMTL::GetSize() const { + return {static_cast(texture_.width), + static_cast(texture_.height)}; +} - DrawableTextureMTL(const DrawableTextureMTL&) = delete; +id TextureMTL::GetMTLTexture() const { + return texture_; +} - DrawableTextureMTL& operator=(const DrawableTextureMTL&) = delete; -}; +bool TextureMTL::IsValid() const { + return is_valid_; +} -// (__bridge id)mtl_texture, +bool TextureMTL::IsWrapped() const { + return is_wrapped_; +} -std::shared_ptr TextureMTL::Wrapper( - TextureDescriptor desc, - id texture, - std::function deletion_proc) { - if (deletion_proc) { - return std::shared_ptr( - new BetterNameTextureMTL(desc, texture, true), - [deletion_proc = std::move(deletion_proc)](TextureMTL* t) { - deletion_proc(); - delete t; - }); +bool TextureMTL::GenerateMipmap(id encoder) { + if (!texture_) { + return false; } - return std::make_shared(desc, texture, true); -} -std::shared_ptr TextureMTL::Create(TextureDescriptor desc, - id texture) { - return std::make_shared(desc, texture, false); -} + [encoder generateMipmapsForTexture:texture_]; + mipmap_generated_ = true; -std::shared_ptr TextureMTL::WrapDrawable(TextureDescriptor desc, - CAMetalLayer* layer) { - return std::make_shared(desc, layer); + return true; } -#pragma GCC diagnostic pop - } // namespace impeller From fea4f0bb8f95e53d19d425bbc9a561729bda0881 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 11:46:15 -0800 Subject: [PATCH 10/17] use single texture class. --- impeller/renderer/backend/metal/BUILD.gn | 2 + .../backend/metal/lazy_drawable_holder.h | 42 +++++++++++++ .../backend/metal/lazy_drawable_holder.mm | 45 +++++++++++++ impeller/renderer/backend/metal/texture_mtl.h | 27 +++++--- .../renderer/backend/metal/texture_mtl.mm | 63 +++++++++++++------ .../backend/metal/texture_mtl_unittests.mm | 35 +++-------- 6 files changed, 160 insertions(+), 54 deletions(-) create mode 100644 impeller/renderer/backend/metal/lazy_drawable_holder.h create mode 100644 impeller/renderer/backend/metal/lazy_drawable_holder.mm diff --git a/impeller/renderer/backend/metal/BUILD.gn b/impeller/renderer/backend/metal/BUILD.gn index 7a7b6b115d6d9..a9982ad0de678 100644 --- a/impeller/renderer/backend/metal/BUILD.gn +++ b/impeller/renderer/backend/metal/BUILD.gn @@ -48,6 +48,8 @@ impeller_component("metal") { "texture_wrapper_mtl.mm", "vertex_descriptor_mtl.h", "vertex_descriptor_mtl.mm", + "lazy_drawable_holder.h", + "lazy_drawable_holder.mm", ] public_deps = [ diff --git a/impeller/renderer/backend/metal/lazy_drawable_holder.h b/impeller/renderer/backend/metal/lazy_drawable_holder.h new file mode 100644 index 0000000000000..c79032abdb26a --- /dev/null +++ b/impeller/renderer/backend/metal/lazy_drawable_holder.h @@ -0,0 +1,42 @@ +// 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. + +#pragma once + +#include + +#include +#include +#include "fml/synchronization/count_down_latch.h" +#include "impeller/base/thread.h" +#include "impeller/base/thread_safety.h" +#include "impeller/geometry/scalar.h" + +@protocol CAMetalDrawable; +@class CAMetalLayer; + +namespace impeller { + +/// @brief The drawable holder manages requesting and caching the next drawable +/// and its texture from a CAMetalLayer. +class LazyDrawableHolder { + public: + explicit LazyDrawableHolder(CAMetalLayer* layer); + + ~LazyDrawableHolder() = default; + + id AcquireNextDrawable(); + + id GetDrawable() const; + + private: + CAMetalLayer* layer_ = nullptr; + bool acquired_ = false; + id drawable_ = nullptr; + id texture_ = nullptr; + std::shared_ptr drawable_latch_ = + std::make_shared(1u); +}; + +} // namespace impeller diff --git a/impeller/renderer/backend/metal/lazy_drawable_holder.mm b/impeller/renderer/backend/metal/lazy_drawable_holder.mm new file mode 100644 index 0000000000000..8709f6302ba42 --- /dev/null +++ b/impeller/renderer/backend/metal/lazy_drawable_holder.mm @@ -0,0 +1,45 @@ +// 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 "impeller/renderer/backend/metal/lazy_drawable_holder.h" + +#include + +#include "flutter/fml/trace_event.h" +#include "impeller/base/validation.h" + +namespace impeller { + +LazyDrawableHolder::LazyDrawableHolder(CAMetalLayer* layer) : layer_(layer) {} + +id LazyDrawableHolder::GetDrawable() const { + if (acquired_) { + return drawable_; + } + drawable_latch_->Wait(); + return drawable_; +} + +id LazyDrawableHolder::AcquireNextDrawable() { + if (acquired_) { + return texture_; + } + id current_drawable = nil; + { + TRACE_EVENT0("impeller", "WaitForNextDrawable"); + current_drawable = [layer_ nextDrawable]; + } + acquired_ = true; + drawable_latch_->CountDown(); + if (!current_drawable) { + VALIDATION_LOG << "Could not acquire current drawable."; + return nullptr; + } + drawable_ = current_drawable; + texture_ = drawable_.texture; + + return texture_; +} + +} // namespace impeller diff --git a/impeller/renderer/backend/metal/texture_mtl.h b/impeller/renderer/backend/metal/texture_mtl.h index 9fa5512973c16..400235ab5fdae 100644 --- a/impeller/renderer/backend/metal/texture_mtl.h +++ b/impeller/renderer/backend/metal/texture_mtl.h @@ -15,15 +15,25 @@ namespace impeller { class TextureMTL final : public Texture, public BackendCast { public: + /// @brief This callback needs to always return the same texture when called + /// multiple times. + using AcquireTextureProc = std::function()>; + TextureMTL(TextureDescriptor desc, - id texture, - bool wrapped = false); + const AcquireTextureProc& aquire_proc, + bool wrapped = false, + bool drawable = false); static std::shared_ptr Wrapper( TextureDescriptor desc, id texture, std::function deletion_proc = nullptr); + static std::shared_ptr Create( + TextureDescriptor desc, + id texture + ); + // |Texture| ~TextureMTL() override; @@ -31,12 +41,19 @@ class TextureMTL final : public Texture, bool IsWrapped() const; + /// @brief Whether or not this texture is wrapping a Metal drawable. + bool IsDrawable() const; + + // |Texture| + bool IsValid() const override; + bool GenerateMipmap(id encoder); private: - id texture_ = nullptr; + AcquireTextureProc aquire_proc_ = {}; bool is_valid_ = false; bool is_wrapped_ = false; + bool is_drawable_ = false; // |Texture| void SetLabel(std::string_view label) override; @@ -49,10 +66,6 @@ class TextureMTL final : public Texture, // |Texture| bool OnSetContents(std::shared_ptr mapping, size_t slice) override; - - // |Texture| - bool IsValid() const override; - // |Texture| ISize GetSize() const override; diff --git a/impeller/renderer/backend/metal/texture_mtl.mm b/impeller/renderer/backend/metal/texture_mtl.mm index a2c2d95481528..d65937e8b9f09 100644 --- a/impeller/renderer/backend/metal/texture_mtl.mm +++ b/impeller/renderer/backend/metal/texture_mtl.mm @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "impeller/renderer/backend/metal/texture_mtl.h" +#include #include "impeller/base/validation.h" #include "impeller/core/texture_descriptor.h" @@ -17,12 +18,13 @@ } TextureMTL::TextureMTL(TextureDescriptor p_desc, - id texture, - bool wrapped) - : Texture(p_desc), texture_(texture) { + const AcquireTextureProc& aquire_proc, + bool wrapped, + bool drawable) + : Texture(p_desc), aquire_proc_(aquire_proc), is_drawable_(drawable) { const auto& desc = GetTextureDescriptor(); - if (!desc.IsValid() || !texture_) { + if (!desc.IsValid() || !aquire_proc) { return; } @@ -41,19 +43,29 @@ std::function deletion_proc) { if (deletion_proc) { return std::shared_ptr( - new TextureMTL(desc, texture, true), + new TextureMTL( + desc, [texture]() { return texture; }, true), [deletion_proc = std::move(deletion_proc)](TextureMTL* t) { deletion_proc(); delete t; }); } - return std::shared_ptr(new TextureMTL(desc, texture, true)); + return std::shared_ptr( + new TextureMTL(desc, [texture]() { return texture; }, true)); +} + +std::shared_ptr TextureMTL::Create(TextureDescriptor desc, + id texture) { + return std::make_shared(desc, [texture]() { return texture; }); } TextureMTL::~TextureMTL() = default; void TextureMTL::SetLabel(std::string_view label) { - [texture_ setLabel:@(label.data())]; + if (is_drawable_) { + return; + } + [aquire_proc_() setLabel:@(label.data())]; } // |Texture| @@ -68,7 +80,7 @@ new TextureMTL(desc, texture, true), bool TextureMTL::OnSetContents(const uint8_t* contents, size_t length, size_t slice) { - if (!IsValid() || !contents || is_wrapped_) { + if (!IsValid() || !contents || is_wrapped_ || is_drawable_) { return false; } @@ -81,24 +93,28 @@ new TextureMTL(desc, texture, true), const auto region = MTLRegionMake2D(0u, 0u, desc.size.width, desc.size.height); - [texture_ replaceRegion:region // - mipmapLevel:0u // - slice:slice // - withBytes:contents // - bytesPerRow:desc.GetBytesPerRow() // - bytesPerImage:desc.GetByteSizeOfBaseMipLevel() // + [aquire_proc_() replaceRegion:region // + mipmapLevel:0u // + slice:slice // + withBytes:contents // + bytesPerRow:desc.GetBytesPerRow() // + bytesPerImage:desc.GetByteSizeOfBaseMipLevel() // ]; return true; } ISize TextureMTL::GetSize() const { - return {static_cast(texture_.width), - static_cast(texture_.height)}; + if (is_drawable_) { + return GetTextureDescriptor().size; + } + const auto& texture = aquire_proc_(); + return {static_cast(texture.width), + static_cast(texture.height)}; } id TextureMTL::GetMTLTexture() const { - return texture_; + return aquire_proc_(); } bool TextureMTL::IsValid() const { @@ -109,12 +125,21 @@ new TextureMTL(desc, texture, true), return is_wrapped_; } +bool TextureMTL::IsDrawable() const { + return is_drawable_; +} + bool TextureMTL::GenerateMipmap(id encoder) { - if (!texture_) { + if (is_drawable_) { + return false; + } + + auto texture = aquire_proc_(); + if (!texture) { return false; } - [encoder generateMipmapsForTexture:texture_]; + [encoder generateMipmapsForTexture:texture]; mipmap_generated_ = true; return true; diff --git a/impeller/renderer/backend/metal/texture_mtl_unittests.mm b/impeller/renderer/backend/metal/texture_mtl_unittests.mm index 7f7406a3c68b6..c4821ee6f8191 100644 --- a/impeller/renderer/backend/metal/texture_mtl_unittests.mm +++ b/impeller/renderer/backend/metal/texture_mtl_unittests.mm @@ -6,6 +6,7 @@ #include "impeller/core/formats.h" #include "impeller/core/texture_descriptor.h" #include "impeller/renderer/backend/metal/formats_mtl.h" +#include "impeller/renderer/backend/metal/lazy_drawable_holder.h" #include "impeller/renderer/backend/metal/texture_mtl.h" #include "impeller/renderer/capabilities.h" @@ -28,10 +29,13 @@ desc.size = {100, 100}; desc.format = PixelFormat::kB8G8R8A8UNormInt; - auto drawable_texture = TextureMTL::WrapDrawable(desc, layer); + auto drawble_holder = std::make_shared(layer); + auto drawable_texture = std::make_shared(desc, [&drawble_holder]() { + return drawble_holder->AcquireNextDrawable(); + }, /*wrapped=*/false, /*drawable=*/true); ASSERT_TRUE(drawable_texture->IsValid()); - EXPECT_TRUE(drawable_texture->IsDrawableBacked()); + EXPECT_TRUE(drawable_texture->IsDrawable()); // Spawn a thread and acquire the drawable in the thread. auto thread = std::thread([&drawable_texture]() { @@ -40,35 +44,10 @@ }); thread.join(); // Block until drawable is acquired. - EXPECT_TRUE(drawable_texture->WaitForNextDrawable() != nil); + EXPECT_TRUE(drawble_holder->GetDrawable() != nil); // Drawable is cached. EXPECT_TRUE(drawable_texture->GetMTLTexture() != nil); } -TEST(TextureMTL, CreateFromInvalidDescriptor) { - ScopedValidationDisable validation; - - auto device = MTLCreateSystemDefaultDevice(); - auto layer = [[CAMetalLayer alloc] init]; - layer.device = device; - layer.drawableSize = CGSize{100, 100}; - layer.pixelFormat = ToMTLPixelFormat(PixelFormat::kB8G8R8A8UNormInt); - - TextureDescriptor desc; - // Size is mismatched. - desc.size = {200, 200}; - desc.format = PixelFormat::kB8G8R8A8UNormInt; - - auto drawable_texture_a = TextureMTL::WrapDrawable(desc, layer); - EXPECT_FALSE(drawable_texture_a->IsValid()); - - desc.size = {100, 100}; - // Pixel format is mismatched. - desc.format = PixelFormat::kD24UnormS8Uint; - - auto drawable_texture_b = TextureMTL::WrapDrawable(desc, layer); - EXPECT_FALSE(drawable_texture_b->IsValid()); -} - } // namespace testing } // namespace impeller \ No newline at end of file From 43e4820d7439e3848f3428140e2825681a704136 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 11:46:46 -0800 Subject: [PATCH 11/17] + --- impeller/renderer/backend/metal/BUILD.gn | 4 ++-- impeller/renderer/backend/metal/texture_mtl.h | 6 ++---- impeller/renderer/backend/metal/texture_mtl_unittests.mm | 7 ++++--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/impeller/renderer/backend/metal/BUILD.gn b/impeller/renderer/backend/metal/BUILD.gn index a9982ad0de678..e038782fb4ec2 100644 --- a/impeller/renderer/backend/metal/BUILD.gn +++ b/impeller/renderer/backend/metal/BUILD.gn @@ -26,6 +26,8 @@ impeller_component("metal") { "formats_mtl.mm", "gpu_tracer_mtl.h", "gpu_tracer_mtl.mm", + "lazy_drawable_holder.h", + "lazy_drawable_holder.mm", "pipeline_library_mtl.h", "pipeline_library_mtl.mm", "pipeline_mtl.h", @@ -48,8 +50,6 @@ impeller_component("metal") { "texture_wrapper_mtl.mm", "vertex_descriptor_mtl.h", "vertex_descriptor_mtl.mm", - "lazy_drawable_holder.h", - "lazy_drawable_holder.mm", ] public_deps = [ diff --git a/impeller/renderer/backend/metal/texture_mtl.h b/impeller/renderer/backend/metal/texture_mtl.h index 400235ab5fdae..632b0491e56b3 100644 --- a/impeller/renderer/backend/metal/texture_mtl.h +++ b/impeller/renderer/backend/metal/texture_mtl.h @@ -29,10 +29,8 @@ class TextureMTL final : public Texture, id texture, std::function deletion_proc = nullptr); - static std::shared_ptr Create( - TextureDescriptor desc, - id texture - ); + static std::shared_ptr Create(TextureDescriptor desc, + id texture); // |Texture| ~TextureMTL() override; diff --git a/impeller/renderer/backend/metal/texture_mtl_unittests.mm b/impeller/renderer/backend/metal/texture_mtl_unittests.mm index c4821ee6f8191..4f18ba66affd9 100644 --- a/impeller/renderer/backend/metal/texture_mtl_unittests.mm +++ b/impeller/renderer/backend/metal/texture_mtl_unittests.mm @@ -30,9 +30,10 @@ desc.format = PixelFormat::kB8G8R8A8UNormInt; auto drawble_holder = std::make_shared(layer); - auto drawable_texture = std::make_shared(desc, [&drawble_holder]() { - return drawble_holder->AcquireNextDrawable(); - }, /*wrapped=*/false, /*drawable=*/true); + auto drawable_texture = std::make_shared( + desc, + [&drawble_holder]() { return drawble_holder->AcquireNextDrawable(); }, + /*wrapped=*/false, /*drawable=*/true); ASSERT_TRUE(drawable_texture->IsValid()); EXPECT_TRUE(drawable_texture->IsDrawable()); From 19ef071d8171af31aebbc63617803a277a8ea530 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 11:50:15 -0800 Subject: [PATCH 12/17] ++ --- impeller/renderer/backend/metal/texture_mtl_unittests.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/backend/metal/texture_mtl_unittests.mm b/impeller/renderer/backend/metal/texture_mtl_unittests.mm index 4f18ba66affd9..8bacbe11394ae 100644 --- a/impeller/renderer/backend/metal/texture_mtl_unittests.mm +++ b/impeller/renderer/backend/metal/texture_mtl_unittests.mm @@ -51,4 +51,4 @@ } } // namespace testing -} // namespace impeller \ No newline at end of file +} // namespace impeller From 4264cdfdc0f892f526587cecfd4e7ddf3bf4b1f9 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 12:27:00 -0800 Subject: [PATCH 13/17] license and ignore --- ci/licenses_golden/licenses_flutter | 4 ++++ impeller/renderer/backend/metal/lazy_drawable_holder.mm | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 78f1bb857c5b8..4457c6414f467 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -3317,6 +3317,8 @@ ORIGIN: ../../../flutter/impeller/renderer/backend/metal/formats_mtl.h + ../../. ORIGIN: ../../../flutter/impeller/renderer/backend/metal/formats_mtl.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/metal/gpu_tracer_mtl.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/metal/gpu_tracer_mtl.mm + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/metal/lazy_drawable_holder.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/metal/lazy_drawable_holder.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/metal/pipeline_library_mtl.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/metal/pipeline_library_mtl.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/metal/pipeline_mtl.h + ../../../flutter/LICENSE @@ -6079,6 +6081,8 @@ FILE: ../../../flutter/impeller/renderer/backend/metal/formats_mtl.h FILE: ../../../flutter/impeller/renderer/backend/metal/formats_mtl.mm FILE: ../../../flutter/impeller/renderer/backend/metal/gpu_tracer_mtl.h FILE: ../../../flutter/impeller/renderer/backend/metal/gpu_tracer_mtl.mm +FILE: ../../../flutter/impeller/renderer/backend/metal/lazy_drawable_holder.h +FILE: ../../../flutter/impeller/renderer/backend/metal/lazy_drawable_holder.mm FILE: ../../../flutter/impeller/renderer/backend/metal/pipeline_library_mtl.h FILE: ../../../flutter/impeller/renderer/backend/metal/pipeline_library_mtl.mm FILE: ../../../flutter/impeller/renderer/backend/metal/pipeline_mtl.h diff --git a/impeller/renderer/backend/metal/lazy_drawable_holder.mm b/impeller/renderer/backend/metal/lazy_drawable_holder.mm index 8709f6302ba42..140a1d4d30c4c 100644 --- a/impeller/renderer/backend/metal/lazy_drawable_holder.mm +++ b/impeller/renderer/backend/metal/lazy_drawable_holder.mm @@ -11,6 +11,13 @@ namespace impeller { +#pragma GCC diagnostic push + // Disable the diagnostic for iOS Simulators. Metal without emulation isn't + // available prior to iOS 13 and that's what the simulator headers say when + // support for CAMetalLayer begins. CAMetalLayer is available on iOS 8.0 and + // above which is well below Flutters support level. +#pragma GCC diagnostic ignored "-Wunguarded-availability-new" + LazyDrawableHolder::LazyDrawableHolder(CAMetalLayer* layer) : layer_(layer) {} id LazyDrawableHolder::GetDrawable() const { @@ -42,4 +49,6 @@ return texture_; } +#pragma GCC diagnostic pop + } // namespace impeller From d00491e7f40dc7eb6a1c6603d888baaf05b2cb40 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 12:28:17 -0800 Subject: [PATCH 14/17] ++ --- impeller/renderer/backend/metal/lazy_drawable_holder.mm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/impeller/renderer/backend/metal/lazy_drawable_holder.mm b/impeller/renderer/backend/metal/lazy_drawable_holder.mm index 140a1d4d30c4c..78c25ce79c794 100644 --- a/impeller/renderer/backend/metal/lazy_drawable_holder.mm +++ b/impeller/renderer/backend/metal/lazy_drawable_holder.mm @@ -12,10 +12,10 @@ namespace impeller { #pragma GCC diagnostic push - // Disable the diagnostic for iOS Simulators. Metal without emulation isn't - // available prior to iOS 13 and that's what the simulator headers say when - // support for CAMetalLayer begins. CAMetalLayer is available on iOS 8.0 and - // above which is well below Flutters support level. +// Disable the diagnostic for iOS Simulators. Metal without emulation isn't +// available prior to iOS 13 and that's what the simulator headers say when +// support for CAMetalLayer begins. CAMetalLayer is available on iOS 8.0 and +// above which is well below Flutters support level. #pragma GCC diagnostic ignored "-Wunguarded-availability-new" LazyDrawableHolder::LazyDrawableHolder(CAMetalLayer* layer) : layer_(layer) {} From 22f5730f1d58d7b61b1c39436aa00708355d7aba Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 15:49:06 -0800 Subject: [PATCH 15/17] ++ --- impeller/aiks/canvas.cc | 2 +- .../backend/metal/lazy_drawable_holder.h | 40 ++++++--------- .../backend/metal/lazy_drawable_holder.mm | 49 +++++++++---------- .../backend/metal/texture_mtl_unittests.mm | 13 +++-- 4 files changed, 43 insertions(+), 61 deletions(-) diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index 0bcb16580d2f2..d07fdd319dfa3 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -691,7 +691,7 @@ void Canvas::DrawAtlas(const std::shared_ptr& atlas, } std::shared_ptr contents = std::make_shared(); - contents->SetColors(std::move(colors)); + contents->SetColors({}); contents->SetTransforms(std::move(transforms)); contents->SetTextureCoordinates(std::move(texture_coordinates)); contents->SetTexture(atlas->GetTexture()); diff --git a/impeller/renderer/backend/metal/lazy_drawable_holder.h b/impeller/renderer/backend/metal/lazy_drawable_holder.h index c79032abdb26a..5062e453e5d00 100644 --- a/impeller/renderer/backend/metal/lazy_drawable_holder.h +++ b/impeller/renderer/backend/metal/lazy_drawable_holder.h @@ -6,37 +6,25 @@ #include -#include -#include -#include "fml/synchronization/count_down_latch.h" -#include "impeller/base/thread.h" -#include "impeller/base/thread_safety.h" -#include "impeller/geometry/scalar.h" +#include +#include "impeller/core/texture_descriptor.h" +#include "impeller/renderer/backend/metal/texture_mtl.h" @protocol CAMetalDrawable; @class CAMetalLayer; namespace impeller { -/// @brief The drawable holder manages requesting and caching the next drawable -/// and its texture from a CAMetalLayer. -class LazyDrawableHolder { - public: - explicit LazyDrawableHolder(CAMetalLayer* layer); - - ~LazyDrawableHolder() = default; - - id AcquireNextDrawable(); - - id GetDrawable() const; - - private: - CAMetalLayer* layer_ = nullptr; - bool acquired_ = false; - id drawable_ = nullptr; - id texture_ = nullptr; - std::shared_ptr drawable_latch_ = - std::make_shared(1u); -}; +/// @brief Create a deferred drawable from a CAMetalLayer. +std::shared_future> GetDrawableDeferred( + CAMetalLayer* layer); + +/// @brief Create a TextureMTL from a deferred drawable. +/// +/// This function is safe to call multiple times and will only call +/// nextDrawable once. +std::shared_ptr CreateTextureFromDrawableFuture( + TextureDescriptor desc, + const std::shared_future>& drawble_future); } // namespace impeller diff --git a/impeller/renderer/backend/metal/lazy_drawable_holder.mm b/impeller/renderer/backend/metal/lazy_drawable_holder.mm index 78c25ce79c794..73886bcf4e16a 100644 --- a/impeller/renderer/backend/metal/lazy_drawable_holder.mm +++ b/impeller/renderer/backend/metal/lazy_drawable_holder.mm @@ -5,6 +5,8 @@ #include "impeller/renderer/backend/metal/lazy_drawable_holder.h" #include +#include +#include #include "flutter/fml/trace_event.h" #include "impeller/base/validation.h" @@ -18,35 +20,28 @@ // above which is well below Flutters support level. #pragma GCC diagnostic ignored "-Wunguarded-availability-new" -LazyDrawableHolder::LazyDrawableHolder(CAMetalLayer* layer) : layer_(layer) {} - -id LazyDrawableHolder::GetDrawable() const { - if (acquired_) { - return drawable_; - } - drawable_latch_->Wait(); - return drawable_; +std::shared_future> GetDrawableDeferred(CAMetalLayer* layer) { + auto future = std::async(std::launch::deferred, [layer]() -> id { + id current_drawable = nil; + { + TRACE_EVENT0("impeller", "WaitForNextDrawable"); + current_drawable = [layer nextDrawable]; + } + if (!current_drawable) { + VALIDATION_LOG << "Could not acquire current drawable."; + return nullptr; + } + return current_drawable; + }); + return std::shared_future>(std::move(future)); } -id LazyDrawableHolder::AcquireNextDrawable() { - if (acquired_) { - return texture_; - } - id current_drawable = nil; - { - TRACE_EVENT0("impeller", "WaitForNextDrawable"); - current_drawable = [layer_ nextDrawable]; - } - acquired_ = true; - drawable_latch_->CountDown(); - if (!current_drawable) { - VALIDATION_LOG << "Could not acquire current drawable."; - return nullptr; - } - drawable_ = current_drawable; - texture_ = drawable_.texture; - - return texture_; +std::shared_ptr CreateTextureFromDrawableFuture( + TextureDescriptor desc, + const std::shared_future>& drawble_future) { + return std::make_shared( + desc, [drawble_future]() { return drawble_future.get().texture; }, + /*wrapped=*/false, /*drawable=*/true); } #pragma GCC diagnostic pop diff --git a/impeller/renderer/backend/metal/texture_mtl_unittests.mm b/impeller/renderer/backend/metal/texture_mtl_unittests.mm index 8bacbe11394ae..2e263eb0e7db4 100644 --- a/impeller/renderer/backend/metal/texture_mtl_unittests.mm +++ b/impeller/renderer/backend/metal/texture_mtl_unittests.mm @@ -28,12 +28,9 @@ TextureDescriptor desc; desc.size = {100, 100}; desc.format = PixelFormat::kB8G8R8A8UNormInt; - - auto drawble_holder = std::make_shared(layer); - auto drawable_texture = std::make_shared( - desc, - [&drawble_holder]() { return drawble_holder->AcquireNextDrawable(); }, - /*wrapped=*/false, /*drawable=*/true); + auto drawable_future = GetDrawableDeferred(layer); + auto drawable_texture = + CreateTextureFromDrawableFuture(desc, drawable_future); ASSERT_TRUE(drawable_texture->IsValid()); EXPECT_TRUE(drawable_texture->IsDrawable()); @@ -45,9 +42,11 @@ }); thread.join(); // Block until drawable is acquired. - EXPECT_TRUE(drawble_holder->GetDrawable() != nil); + EXPECT_TRUE(drawable_future.get() != nil); // Drawable is cached. EXPECT_TRUE(drawable_texture->GetMTLTexture() != nil); + // Once more for good measure. + EXPECT_TRUE(drawable_texture->GetMTLTexture() != nil); } } // namespace testing From bdef2bc4ba5f14ca3e6c9a1794886b1feb5546dc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 15:50:29 -0800 Subject: [PATCH 16/17] ++ --- .../backend/metal/lazy_drawable_holder.mm | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/impeller/renderer/backend/metal/lazy_drawable_holder.mm b/impeller/renderer/backend/metal/lazy_drawable_holder.mm index 73886bcf4e16a..b847791a288b1 100644 --- a/impeller/renderer/backend/metal/lazy_drawable_holder.mm +++ b/impeller/renderer/backend/metal/lazy_drawable_holder.mm @@ -20,19 +20,21 @@ // above which is well below Flutters support level. #pragma GCC diagnostic ignored "-Wunguarded-availability-new" -std::shared_future> GetDrawableDeferred(CAMetalLayer* layer) { - auto future = std::async(std::launch::deferred, [layer]() -> id { - id current_drawable = nil; - { - TRACE_EVENT0("impeller", "WaitForNextDrawable"); - current_drawable = [layer nextDrawable]; - } - if (!current_drawable) { - VALIDATION_LOG << "Could not acquire current drawable."; - return nullptr; - } - return current_drawable; - }); +std::shared_future> GetDrawableDeferred( + CAMetalLayer* layer) { + auto future = + std::async(std::launch::deferred, [layer]() -> id { + id current_drawable = nil; + { + TRACE_EVENT0("impeller", "WaitForNextDrawable"); + current_drawable = [layer nextDrawable]; + } + if (!current_drawable) { + VALIDATION_LOG << "Could not acquire current drawable."; + return nullptr; + } + return current_drawable; + }); return std::shared_future>(std::move(future)); } From f2282438d973d0059d734c94da866e63fbe5c4b0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 Nov 2023 16:27:28 -0800 Subject: [PATCH 17/17] revert unrelated change --- impeller/aiks/canvas.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index d07fdd319dfa3..0bcb16580d2f2 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -691,7 +691,7 @@ void Canvas::DrawAtlas(const std::shared_ptr& atlas, } std::shared_ptr contents = std::make_shared(); - contents->SetColors({}); + contents->SetColors(std::move(colors)); contents->SetTransforms(std::move(transforms)); contents->SetTextureCoordinates(std::move(texture_coordinates)); contents->SetTexture(atlas->GetTexture());