From 144af3487e6bcdda888cb039fd1897366f45cf7e Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 26 Apr 2023 21:33:04 -0700 Subject: [PATCH] Reland: [Impeller] Use a device buffer for SkBitmap allocation, use Linear texture on Metal backend. (flutter/engine#41538) Original PR: https://github.com/flutter/engine/pull/41374 This was reverted because it broke on simulators as they do not support linear textures. To fix this, I've ifdef'd out the DeviceBufferMTL implementation of AsTexture so that it falls back to the slow path copy. Also updated the capabilities check so that the glyph atlas updates the texture contents when it changes. --- .../renderer/backend/metal/context_mtl.mm | 9 + .../backend/metal/device_buffer_mtl.h | 2 + .../backend/metal/device_buffer_mtl.mm | 2 + .../backend/vulkan/capabilities_vk.cc | 6 + .../renderer/backend/vulkan/capabilities_vk.h | 3 + .../flutter/impeller/renderer/capabilities.cc | 16 ++ .../flutter/impeller/renderer/capabilities.h | 5 + .../renderer/capabilities_unittests.cc | 1 + .../backends/skia/text_render_context_skia.cc | 162 +++++++++++++----- .../backends/skia/text_render_context_skia.h | 29 ++++ .../impeller/typographer/glyph_atlas.cc | 10 +- .../impeller/typographer/glyph_atlas.h | 8 +- .../impeller/typographer/lazy_glyph_atlas.cc | 5 +- .../typographer/text_render_context.cc | 4 +- .../typographer/text_render_context.h | 2 + .../typographer/typographer_unittests.cc | 106 ++++++++---- 16 files changed, 286 insertions(+), 84 deletions(-) diff --git a/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm index c83f5b7075c..576753f79f8 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm @@ -45,6 +45,14 @@ static bool DeviceSupportsComputeSubgroups(id device) { return supports_subgroups; } +static constexpr bool SupportsLinearTexture() { +#ifdef FML_OS_IOS_SIMULATOR + return false; +#else + return true; +#endif // FML_OS_IOS_SIMULATOR +} + static std::unique_ptr InferMetalCapabilities( id device, PixelFormat color_format) { @@ -55,6 +63,7 @@ static std::unique_ptr InferMetalCapabilities( .SetSupportsBufferToTextureBlits(true) .SetSupportsTextureToTextureBlits(true) .SetSupportsDecalTileMode(true) + .SetSupportsSharedDeviceBufferTextureMemory(SupportsLinearTexture()) .SetSupportsFramebufferFetch(DeviceSupportsFramebufferFetch(device)) .SetDefaultColorFormat(color_format) .SetDefaultStencilFormat(PixelFormat::kS8UInt) diff --git a/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.h b/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.h index 061ad6004cc..06869eb661d 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.h +++ b/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.h @@ -35,10 +35,12 @@ class DeviceBufferMTL final : public DeviceBuffer, // |DeviceBuffer| uint8_t* OnGetContents() const override; +#ifndef FML_OS_IOS_SIMULATOR // |DeviceBuffer| std::shared_ptr AsTexture(Allocator& allocator, const TextureDescriptor& descriptor, uint16_t row_bytes) const override; +#endif // FML_OS_IOS_SIMULATOR // |DeviceBuffer| bool OnCopyHostBuffer(const uint8_t* source, diff --git a/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.mm index a8534c9a5da..3418df47f48 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.mm @@ -29,6 +29,7 @@ uint8_t* DeviceBufferMTL::OnGetContents() const { return reinterpret_cast(buffer_.contents); } +#ifndef FML_OS_IOS_SIMULATOR std::shared_ptr DeviceBufferMTL::AsTexture( Allocator& allocator, const TextureDescriptor& descriptor, @@ -52,6 +53,7 @@ std::shared_ptr DeviceBufferMTL::AsTexture( } return std::make_shared(descriptor, texture); } +#endif // FML_OS_IOS_SIMULATOR [[nodiscard]] bool DeviceBufferMTL::OnCopyHostBuffer(const uint8_t* source, Range source_range, diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc index ff4410f1593..91bb80073e6 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -348,10 +348,16 @@ bool CapabilitiesVK::SupportsReadFromOnscreenTexture() const { return false; } +// |Capabilities| bool CapabilitiesVK::SupportsDecalTileMode() const { return true; } +// |Capabilities| +bool CapabilitiesVK::SupportsSharedDeviceBufferTextureMemory() const { + return false; +} + // |Capabilities| PixelFormat CapabilitiesVK::GetDefaultColorFormat() const { return color_format_; diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h index 006ed6f0e08..0628b23d38d 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -79,6 +79,9 @@ class CapabilitiesVK final : public Capabilities, // |Capabilities| bool SupportsDecalTileMode() const override; + // |Capabilities| + bool SupportsSharedDeviceBufferTextureMemory() const override; + // |Capabilities| PixelFormat GetDefaultColorFormat() const override; diff --git a/engine/src/flutter/impeller/renderer/capabilities.cc b/engine/src/flutter/impeller/renderer/capabilities.cc index d0531f5f265..b3106abdc79 100644 --- a/engine/src/flutter/impeller/renderer/capabilities.cc +++ b/engine/src/flutter/impeller/renderer/capabilities.cc @@ -66,6 +66,11 @@ class StandardCapabilities final : public Capabilities { return supports_decal_tile_mode_; } + // |Capabilities| + bool SupportsSharedDeviceBufferTextureMemory() const override { + return supports_shared_device_buffer_texture_memory_; + } + // |Capabilities| PixelFormat GetDefaultColorFormat() const override { return default_color_format_; @@ -88,6 +93,7 @@ class StandardCapabilities final : public Capabilities { bool supports_read_from_onscreen_texture, bool supports_read_from_resolve, bool supports_decal_tile_mode, + bool supports_shared_device_buffer_texture_memory, PixelFormat default_color_format, PixelFormat default_stencil_format) : has_threading_restrictions_(has_threading_restrictions), @@ -102,6 +108,8 @@ class StandardCapabilities final : public Capabilities { supports_read_from_onscreen_texture), supports_read_from_resolve_(supports_read_from_resolve), supports_decal_tile_mode_(supports_decal_tile_mode), + supports_shared_device_buffer_texture_memory_( + supports_shared_device_buffer_texture_memory), default_color_format_(default_color_format), default_stencil_format_(default_stencil_format) {} @@ -118,6 +126,7 @@ class StandardCapabilities final : public Capabilities { bool supports_read_from_onscreen_texture_ = false; bool supports_read_from_resolve_ = false; bool supports_decal_tile_mode_ = false; + bool supports_shared_device_buffer_texture_memory_ = false; PixelFormat default_color_format_ = PixelFormat::kUnknown; PixelFormat default_stencil_format_ = PixelFormat::kUnknown; @@ -202,6 +211,12 @@ CapabilitiesBuilder& CapabilitiesBuilder::SetSupportsDecalTileMode(bool value) { return *this; } +CapabilitiesBuilder& +CapabilitiesBuilder::SetSupportsSharedDeviceBufferTextureMemory(bool value) { + supports_shared_device_buffer_texture_memory_ = value; + return *this; +} + std::unique_ptr CapabilitiesBuilder::Build() { return std::unique_ptr(new StandardCapabilities( // has_threading_restrictions_, // @@ -215,6 +230,7 @@ std::unique_ptr CapabilitiesBuilder::Build() { supports_read_from_onscreen_texture_, // supports_read_from_resolve_, // supports_decal_tile_mode_, // + supports_shared_device_buffer_texture_memory_, // *default_color_format_, // *default_stencil_format_ // )); diff --git a/engine/src/flutter/impeller/renderer/capabilities.h b/engine/src/flutter/impeller/renderer/capabilities.h index 6fb1118dd7c..2c025a329ca 100644 --- a/engine/src/flutter/impeller/renderer/capabilities.h +++ b/engine/src/flutter/impeller/renderer/capabilities.h @@ -37,6 +37,8 @@ class Capabilities { virtual bool SupportsDecalTileMode() const = 0; + virtual bool SupportsSharedDeviceBufferTextureMemory() const = 0; + virtual PixelFormat GetDefaultColorFormat() const = 0; virtual PixelFormat GetDefaultStencilFormat() const = 0; @@ -79,6 +81,8 @@ class CapabilitiesBuilder { CapabilitiesBuilder& SetSupportsDecalTileMode(bool value); + CapabilitiesBuilder& SetSupportsSharedDeviceBufferTextureMemory(bool value); + std::unique_ptr Build(); private: @@ -93,6 +97,7 @@ class CapabilitiesBuilder { bool supports_read_from_onscreen_texture_ = false; bool supports_read_from_resolve_ = false; bool supports_decal_tile_mode_ = false; + bool supports_shared_device_buffer_texture_memory_ = false; std::optional default_color_format_ = std::nullopt; std::optional default_stencil_format_ = std::nullopt; diff --git a/engine/src/flutter/impeller/renderer/capabilities_unittests.cc b/engine/src/flutter/impeller/renderer/capabilities_unittests.cc index 1dc9d17e3c6..342dcba2b8a 100644 --- a/engine/src/flutter/impeller/renderer/capabilities_unittests.cc +++ b/engine/src/flutter/impeller/renderer/capabilities_unittests.cc @@ -29,6 +29,7 @@ CAPABILITY_TEST(SupportsComputeSubgroups, false); CAPABILITY_TEST(SupportsReadFromOnscreenTexture, false); CAPABILITY_TEST(SupportsReadFromResolve, false); CAPABILITY_TEST(SupportsDecalTileMode, false); +CAPABILITY_TEST(SupportsSharedDeviceBufferTextureMemory, false); } // namespace testing } // namespace impeller diff --git a/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc b/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc index 51ec87b712d..2a53d8869bf 100644 --- a/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -10,11 +10,13 @@ #include "flutter/fml/trace_event.h" #include "impeller/base/allocation.h" #include "impeller/core/allocator.h" +#include "impeller/core/device_buffer.h" #include "impeller/typographer/backends/skia/typeface_skia.h" -#include "third_party/skia/include/core/SkBitmap.h" + #include "third_party/skia/include/core/SkCanvas.h" #include "third_party/skia/include/core/SkFont.h" #include "third_party/skia/include/core/SkFontMetrics.h" +#include "third_party/skia/include/core/SkPixelRef.h" #include "third_party/skia/include/core/SkRSXform.h" #include "third_party/skia/include/core/SkSurface.h" #include "third_party/skia/src/core/SkIPoint16.h" // nogncheck @@ -36,6 +38,16 @@ std::unique_ptr TextRenderContext::Create( // https://github.com/flutter/flutter/issues/114563 constexpr auto kPadding = 2; +std::optional ComputeMinimumAlignment( + const std::shared_ptr& allocator, + const std::shared_ptr& capabilities, + PixelFormat format) { + if (!capabilities->SupportsSharedDeviceBufferTextureMemory()) { + return std::nullopt; + } + return allocator->MinimumBytesPerRow(format); +} + TextRenderContextSkia::TextRenderContextSkia(std::shared_ptr context) : TextRenderContext(std::move(context)) {} @@ -138,13 +150,21 @@ namespace { ISize OptimumAtlasSizeForFontGlyphPairs( const FontGlyphPair::Set& pairs, std::vector& glyph_positions, - const std::shared_ptr& atlas_context) { - static constexpr auto kMinAtlasSize = 8u; + const std::shared_ptr& atlas_context, + std::optional minimum_alignment) { + // This size needs to be above the minimum required aligment for linear + // textures. This is 256 for older intel macs and decreases on iOS devices. + static constexpr auto kMinAtlasSize = 256u; static constexpr auto kMaxAtlasSize = 4096u; + // In case a device happens to have a larger minimum alignment, verify that + // 256 is sufficient here. + uint16_t minimum_size = minimum_alignment.value_or(0) > kMinAtlasSize + ? minimum_alignment.value() + : kMinAtlasSize; TRACE_EVENT0("impeller", __FUNCTION__); - ISize current_size(kMinAtlasSize, kMinAtlasSize); + ISize current_size(minimum_size, minimum_size); size_t total_pairs = pairs.size() + 1; do { auto rect_packer = std::shared_ptr( @@ -346,9 +366,12 @@ static bool UpdateAtlasBitmap(const GlyphAtlas& atlas, return true; } -static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, - const ISize& atlas_size) { +static std::pair, std::shared_ptr> +CreateAtlasBitmap(const GlyphAtlas& atlas, + std::shared_ptr allocator, + const ISize& atlas_size) { TRACE_EVENT0("impeller", __FUNCTION__); + auto font_allocator = FontImpellerAllocator(std::move(allocator)); auto bitmap = std::make_shared(); SkImageInfo image_info; @@ -363,17 +386,18 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, break; } - if (!bitmap->tryAllocPixels(image_info)) { - return nullptr; + bitmap->setInfo(image_info); + if (!bitmap->tryAllocPixels(&font_allocator)) { + return std::make_pair(nullptr, nullptr); } auto surface = SkSurface::MakeRasterDirect(bitmap->pixmap()); if (!surface) { - return nullptr; + return std::make_pair(nullptr, nullptr); } auto canvas = surface->getCanvas(); if (!canvas) { - return nullptr; + return std::make_pair(nullptr, nullptr); } bool has_color = atlas.GetType() == GlyphAtlas::Type::kColorBitmap; @@ -384,13 +408,16 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, return true; }); - return bitmap; + auto device_buffer = font_allocator.GetDeviceBuffer(); + if (!device_buffer.has_value()) { + return std::make_pair(nullptr, nullptr); + } + return std::make_pair(bitmap, device_buffer.value()); } static bool UpdateGlyphTextureAtlas(std::shared_ptr bitmap, const std::shared_ptr& texture) { TRACE_EVENT0("impeller", __FUNCTION__); - FML_DCHECK(bitmap != nullptr); auto texture_descriptor = texture->GetTextureDescriptor(); @@ -404,14 +431,12 @@ static bool UpdateGlyphTextureAtlas(std::shared_ptr bitmap, } static std::shared_ptr UploadGlyphTextureAtlas( - const std::shared_ptr& allocator, - std::shared_ptr bitmap, + Allocator& allocator, + const std::shared_ptr& device_buffer, + const std::shared_ptr& bitmap, const ISize& atlas_size, PixelFormat format) { TRACE_EVENT0("impeller", __FUNCTION__); - if (!allocator) { - return nullptr; - } FML_DCHECK(bitmap != nullptr); const auto& pixmap = bitmap->pixmap(); @@ -421,32 +446,27 @@ static std::shared_ptr UploadGlyphTextureAtlas( texture_descriptor.format = format; texture_descriptor.size = atlas_size; + // If the alignment isn't a multiple of the pixel format, we cannot use + // a linear texture and instead must blit to a new texture. if (pixmap.rowBytes() * pixmap.height() != texture_descriptor.GetByteSizeOfBaseMipLevel()) { return nullptr; } - auto texture = allocator->CreateTexture(texture_descriptor); + FML_DCHECK(allocator.MinimumBytesPerRow(format) <= pixmap.rowBytes()); + auto texture = device_buffer->AsTexture(allocator, texture_descriptor, + texture_descriptor.GetBytesPerRow()); if (!texture || !texture->IsValid()) { return nullptr; } texture->SetLabel("GlyphAtlas"); - - auto mapping = std::make_shared( - reinterpret_cast(bitmap->getAddr(0, 0)), // data - texture_descriptor.GetByteSizeOfBaseMipLevel(), // size - [bitmap](auto, auto) mutable { bitmap.reset(); } // proc - ); - - if (!texture->SetContents(mapping)) { - return nullptr; - } return texture; } std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, + const std::shared_ptr& capabilities, FrameIterator frame_iterator) const { TRACE_EVENT0("impeller", __FUNCTION__); if (!IsValid()) { @@ -502,27 +522,42 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- // Step 5: Draw new font-glyph pairs into the existing bitmap. // --------------------------------------------------------------------------- - auto bitmap = atlas_context->GetBitmap(); + auto [bitmap, device_buffer] = atlas_context->GetBitmap(); if (!UpdateAtlasBitmap(*last_atlas, bitmap, new_glyphs)) { return nullptr; } // --------------------------------------------------------------------------- // Step 6: Update the existing texture with the updated bitmap. + // This is only necessary on backends that don't support creating + // a texture that shares memory with the underlying device buffer. // --------------------------------------------------------------------------- - if (!UpdateGlyphTextureAtlas(bitmap, last_atlas->GetTexture())) { + if (!capabilities->SupportsSharedDeviceBufferTextureMemory() && + !UpdateGlyphTextureAtlas(bitmap, last_atlas->GetTexture())) { return nullptr; } return last_atlas; } // A new glyph atlas must be created. + PixelFormat format; + switch (type) { + case GlyphAtlas::Type::kSignedDistanceField: + case GlyphAtlas::Type::kAlphaBitmap: + format = PixelFormat::kA8UNormInt; + break; + case GlyphAtlas::Type::kColorBitmap: + format = PixelFormat::kR8G8B8A8UNormInt; + break; + } // --------------------------------------------------------------------------- // Step 4: Get the optimum size of the texture atlas. // --------------------------------------------------------------------------- auto glyph_atlas = std::make_shared(type); + auto min_alignment = ComputeMinimumAlignment( + GetContext()->GetResourceAllocator(), capabilities, format); auto atlas_size = OptimumAtlasSizeForFontGlyphPairs( - font_glyph_pairs, glyph_positions, atlas_context); + font_glyph_pairs, glyph_positions, atlas_context, min_alignment); atlas_context->UpdateGlyphAtlas(glyph_atlas, atlas_size); if (atlas_size.IsEmpty()) { @@ -552,30 +587,24 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- // Step 7: Draw font-glyph pairs in the correct spot in the atlas. // --------------------------------------------------------------------------- - auto bitmap = CreateAtlasBitmap(*glyph_atlas, atlas_size); + auto [bitmap, device_buffer] = CreateAtlasBitmap( + *glyph_atlas, GetContext()->GetResourceAllocator(), atlas_size); if (!bitmap) { return nullptr; } - atlas_context->UpdateBitmap(bitmap); + atlas_context->UpdateBitmap(bitmap, device_buffer); // --------------------------------------------------------------------------- // Step 8: Upload the atlas as a texture. // --------------------------------------------------------------------------- - PixelFormat format; - switch (type) { - case GlyphAtlas::Type::kSignedDistanceField: - ConvertBitmapToSignedDistanceField( - reinterpret_cast(bitmap->getPixels()), atlas_size.width, - atlas_size.height); - case GlyphAtlas::Type::kAlphaBitmap: - format = PixelFormat::kA8UNormInt; - break; - case GlyphAtlas::Type::kColorBitmap: - format = PixelFormat::kR8G8B8A8UNormInt; - break; + if (type == GlyphAtlas::Type::kSignedDistanceField) { + ConvertBitmapToSignedDistanceField( + reinterpret_cast(bitmap->getPixels()), atlas_size.width, + atlas_size.height); } - auto texture = UploadGlyphTextureAtlas(GetContext()->GetResourceAllocator(), - bitmap, atlas_size, format); + auto texture = + UploadGlyphTextureAtlas(*GetContext()->GetResourceAllocator().get(), + device_buffer, bitmap, atlas_size, format); if (!texture) { return nullptr; } @@ -588,4 +617,43 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( return glyph_atlas; } +FontImpellerAllocator::FontImpellerAllocator( + std::shared_ptr allocator) + : allocator_(std::move(allocator)) {} + +std::optional> +FontImpellerAllocator::GetDeviceBuffer() const { + return buffer_; +} + +bool FontImpellerAllocator::allocPixelRef(SkBitmap* bitmap) { + const SkImageInfo& info = bitmap->info(); + if (kUnknown_SkColorType == info.colorType() || info.width() < 0 || + info.height() < 0 || !info.validRowBytes(bitmap->rowBytes())) { + return false; + } + + DeviceBufferDescriptor descriptor; + descriptor.storage_mode = StorageMode::kHostVisible; + descriptor.size = ((bitmap->height() - 1) * bitmap->rowBytes()) + + (bitmap->width() * bitmap->bytesPerPixel()); + + auto device_buffer = allocator_->CreateBuffer(descriptor); + + struct ImpellerPixelRef final : public SkPixelRef { + ImpellerPixelRef(int w, int h, void* s, size_t r) + : SkPixelRef(w, h, s, r) {} + + ~ImpellerPixelRef() override {} + }; + + auto pixel_ref = sk_sp( + new ImpellerPixelRef(info.width(), info.height(), + device_buffer->OnGetContents(), bitmap->rowBytes())); + + bitmap->setPixelRef(std::move(pixel_ref), 0, 0); + buffer_ = std::move(device_buffer); + return true; +} + } // namespace impeller diff --git a/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.h b/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.h index 0c97a8716e7..49200c8b4c1 100644 --- a/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.h +++ b/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.h @@ -6,9 +6,37 @@ #include "flutter/fml/macros.h" #include "impeller/typographer/text_render_context.h" +#include "third_party/skia/include/core/SkBitmap.h" namespace impeller { +class DeviceBuffer; +class Allocator; + +/// @brief An implementation of an SkBitmap allocator that deferrs allocation to +/// an Impeller allocator. This allows usage of Skia software rendering +/// to write to a host buffer or linear texture without an extra copy. +/// +/// This class is an exact copy of the implementation in +/// image_decode_impeller.cc due to the lack of a reasonable library +/// that could be shared. +class FontImpellerAllocator : public SkBitmap::Allocator { + public: + explicit FontImpellerAllocator( + std::shared_ptr allocator); + + ~FontImpellerAllocator() = default; + + // |Allocator| + bool allocPixelRef(SkBitmap* bitmap) override; + + std::optional> GetDeviceBuffer() const; + + private: + std::shared_ptr allocator_; + std::optional> buffer_; +}; + class TextRenderContextSkia : public TextRenderContext { public: TextRenderContextSkia(std::shared_ptr context); @@ -19,6 +47,7 @@ class TextRenderContextSkia : public TextRenderContext { std::shared_ptr CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, + const std::shared_ptr& capabilities, FrameIterator iterator) const override; private: diff --git a/engine/src/flutter/impeller/typographer/glyph_atlas.cc b/engine/src/flutter/impeller/typographer/glyph_atlas.cc index 6908b9eaa20..c5c0be86436 100644 --- a/engine/src/flutter/impeller/typographer/glyph_atlas.cc +++ b/engine/src/flutter/impeller/typographer/glyph_atlas.cc @@ -22,8 +22,9 @@ const ISize& GlyphAtlasContext::GetAtlasSize() const { return atlas_size_; } -std::shared_ptr GlyphAtlasContext::GetBitmap() const { - return bitmap_; +std::pair, std::shared_ptr> +GlyphAtlasContext::GetBitmap() const { + return std::make_pair(bitmap_, device_buffer_); } std::shared_ptr GlyphAtlasContext::GetRectPacker() const { @@ -36,8 +37,11 @@ void GlyphAtlasContext::UpdateGlyphAtlas(std::shared_ptr atlas, atlas_size_ = size; } -void GlyphAtlasContext::UpdateBitmap(std::shared_ptr bitmap) { +void GlyphAtlasContext::UpdateBitmap( + std::shared_ptr bitmap, + std::shared_ptr device_buffer) { bitmap_ = std::move(bitmap); + device_buffer_ = std::move(device_buffer); } void GlyphAtlasContext::UpdateRectPacker( diff --git a/engine/src/flutter/impeller/typographer/glyph_atlas.h b/engine/src/flutter/impeller/typographer/glyph_atlas.h index e7282a0cdb6..0e342fab1e9 100644 --- a/engine/src/flutter/impeller/typographer/glyph_atlas.h +++ b/engine/src/flutter/impeller/typographer/glyph_atlas.h @@ -10,6 +10,7 @@ #include #include "flutter/fml/macros.h" +#include "impeller/core/device_buffer.h" #include "impeller/core/texture.h" #include "impeller/geometry/rect.h" #include "impeller/renderer/pipeline.h" @@ -153,7 +154,8 @@ class GlyphAtlasContext { //---------------------------------------------------------------------------- /// @brief Retrieve the previous (if any) SkBitmap instance. - std::shared_ptr GetBitmap() const; + std::pair, std::shared_ptr> + GetBitmap() const; //---------------------------------------------------------------------------- /// @brief Retrieve the previous (if any) rect packer. @@ -163,7 +165,8 @@ class GlyphAtlasContext { /// @brief Update the context with a newly constructed glyph atlas. void UpdateGlyphAtlas(std::shared_ptr atlas, ISize size); - void UpdateBitmap(std::shared_ptr bitmap); + void UpdateBitmap(std::shared_ptr bitmap, + std::shared_ptr device_buffer); void UpdateRectPacker(std::shared_ptr rect_packer); @@ -171,6 +174,7 @@ class GlyphAtlasContext { std::shared_ptr atlas_; ISize atlas_size_; std::shared_ptr bitmap_; + std::shared_ptr device_buffer_; std::shared_ptr rect_packer_; FML_DISALLOW_COPY_AND_ASSIGN(GlyphAtlasContext); diff --git a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc index 3e415716a52..23969d787cc 100644 --- a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc +++ b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc @@ -37,6 +37,7 @@ std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( } } + auto capabilities = context->GetCapabilities(); auto text_context = TextRenderContext::Create(std::move(context)); if (!text_context || !text_context->IsValid()) { return nullptr; @@ -50,8 +51,8 @@ std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( i++; return &result; }; - auto atlas = - text_context->CreateGlyphAtlas(type, std::move(atlas_context), iterator); + auto atlas = text_context->CreateGlyphAtlas(type, std::move(atlas_context), + capabilities, iterator); if (!atlas || !atlas->IsValid()) { VALIDATION_LOG << "Could not create valid atlas."; return nullptr; diff --git a/engine/src/flutter/impeller/typographer/text_render_context.cc b/engine/src/flutter/impeller/typographer/text_render_context.cc index 8cc8dbf00a0..1fdc17878ac 100644 --- a/engine/src/flutter/impeller/typographer/text_render_context.cc +++ b/engine/src/flutter/impeller/typographer/text_render_context.cc @@ -29,6 +29,7 @@ const std::shared_ptr& TextRenderContext::GetContext() const { std::shared_ptr TextRenderContext::CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, + const std::shared_ptr& capabilities, const TextFrame& frame) const { size_t count = 0; FrameIterator iterator = [&]() -> const TextFrame* { @@ -38,7 +39,8 @@ std::shared_ptr TextRenderContext::CreateGlyphAtlas( } return nullptr; }; - return CreateGlyphAtlas(type, std::move(atlas_context), iterator); + return CreateGlyphAtlas(type, std::move(atlas_context), capabilities, + iterator); } } // namespace impeller diff --git a/engine/src/flutter/impeller/typographer/text_render_context.h b/engine/src/flutter/impeller/typographer/text_render_context.h index c63ac912a1f..ca1dc74d129 100644 --- a/engine/src/flutter/impeller/typographer/text_render_context.h +++ b/engine/src/flutter/impeller/typographer/text_render_context.h @@ -45,11 +45,13 @@ class TextRenderContext { virtual std::shared_ptr CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, + const std::shared_ptr& capabilities, FrameIterator iterator) const = 0; std::shared_ptr CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, + const std::shared_ptr& capabilities, const TextFrame& frame) const; protected: diff --git a/engine/src/flutter/impeller/typographer/typographer_unittests.cc b/engine/src/flutter/impeller/typographer/typographer_unittests.cc index 445cc283fe6..5223553136b 100644 --- a/engine/src/flutter/impeller/typographer/typographer_unittests.cc +++ b/engine/src/flutter/impeller/typographer/typographer_unittests.cc @@ -4,6 +4,7 @@ #include "flutter/testing/testing.h" #include "impeller/playground/playground_test.h" +#include "impeller/renderer/capabilities.h" #include "impeller/typographer/backends/skia/text_frame_skia.h" #include "impeller/typographer/backends/skia/text_render_context_skia.h" #include "impeller/typographer/lazy_glyph_atlas.h" @@ -41,9 +42,9 @@ TEST_P(TypographerTest, CanCreateGlyphAtlas) { SkFont sk_font; auto blob = SkTextBlob::MakeFromString("hello", sk_font); ASSERT_TRUE(blob); - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas->GetType(), GlyphAtlas::Type::kAlphaBitmap); @@ -116,9 +117,9 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { SkFont sk_font; auto blob = SkTextBlob::MakeFromString("AGH", sk_font); ASSERT_TRUE(blob); - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -133,18 +134,18 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { SkFont sk_font; auto blob = SkTextBlob::MakeFromString("spooky skellingtons", sk_font); ASSERT_TRUE(blob); - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas, atlas_context->GetGlyphAtlas()); // now attempt to re-create an atlas with the same text blob. - auto next_atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto next_atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); ASSERT_EQ(atlas, next_atlas); ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); } @@ -173,8 +174,9 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { } return nullptr; }; - auto atlas = context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, - atlas_context, iterator); + auto atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), iterator); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -182,18 +184,16 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { atlas->GetTexture()->GetSize().height); } -// TODO(jonahwilliams): Re-enable -// https://github.com/flutter/flutter/issues/122839 -TEST_P(TypographerTest, DISABLED_GlyphAtlasTextureIsRecycledIfUnchanged) { +TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto context = TextRenderContext::Create(GetContext()); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -205,9 +205,9 @@ TEST_P(TypographerTest, DISABLED_GlyphAtlasTextureIsRecycledIfUnchanged) { // Now create a new glyph atlas with a nearly identical blob. auto blob2 = SkTextBlob::MakeFromString("spooky 2", sk_font); - auto next_atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob2)); + auto next_atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob2)); ASSERT_EQ(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -224,9 +224,9 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { SkFont sk_font; auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -239,9 +239,9 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { // but change the type. auto blob2 = SkTextBlob::MakeFromString("spooky 1", sk_font); - auto next_atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kColorBitmap, atlas_context, - TextFrameFromTextBlob(blob2)); + auto next_atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kColorBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob2)); ASSERT_NE(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -251,6 +251,54 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { ASSERT_NE(old_packer, new_packer); } +TEST_P(TypographerTest, GlyphAtlasUsesLinearTextureAlphaBitmap) { + if (!GetContext() + ->GetCapabilities() + ->SupportsSharedDeviceBufferTextureMemory()) { + GTEST_SKIP() + << "Skipping test that requires " + "SupportsSharedDeviceBufferTextureMemory on non metal platform"; + } + + auto context = TextRenderContext::Create(GetContext()); + auto atlas_context = std::make_shared(); + ASSERT_TRUE(context && context->IsValid()); + SkFont sk_font; + auto blob = SkTextBlob::MakeFromString("s", sk_font); + ASSERT_TRUE(blob); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + GetContext()->GetCapabilities(), TextFrameFromTextBlob(blob)); + + auto* first_texture = atlas->GetTexture().get(); + + ASSERT_TRUE(first_texture->IsValid()); +} + +TEST_P(TypographerTest, GlyphAtlasUsesLinearTextureColor) { + if (!GetContext() + ->GetCapabilities() + ->SupportsSharedDeviceBufferTextureMemory()) { + GTEST_SKIP() + << "Skipping test that requires " + "SupportsSharedDeviceBufferTextureMemory on non metal platform"; + } + + auto context = TextRenderContext::Create(GetContext()); + auto atlas_context = std::make_shared(); + ASSERT_TRUE(context && context->IsValid()); + SkFont sk_font; + auto blob = SkTextBlob::MakeFromString("s", sk_font); + ASSERT_TRUE(blob); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kColorBitmap, atlas_context, + GetContext()->GetCapabilities(), TextFrameFromTextBlob(blob)); + + auto* first_texture = atlas->GetTexture().get(); + + ASSERT_TRUE(first_texture->IsValid()); +} + TEST_P(TypographerTest, FontGlyphPairTypeChangesHashAndEquals) { Font font = Font(nullptr, {}); FontGlyphPair pair_1 = {