From 56f1452b5a04dcd7790f9fda49592509a23c5075 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 26 Apr 2023 16:00:16 -0700 Subject: [PATCH] Revert "[Impeller] Use a device buffer for SkBitmap allocation, use Linear texture on Metal backend. (#41374)" This reverts commit fddd5add8b628f94e00a77ba16ffe63487a7b305. --- .../renderer/backend/metal/context_mtl.mm | 1 - .../backend/vulkan/capabilities_vk.cc | 6 - .../renderer/backend/vulkan/capabilities_vk.h | 3 - impeller/renderer/capabilities.cc | 16 -- impeller/renderer/capabilities.h | 5 - impeller/renderer/capabilities_unittests.cc | 1 - .../backends/skia/text_render_context_skia.cc | 164 +++++------------- .../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 +- impeller/typographer/text_render_context.cc | 4 +- impeller/typographer/text_render_context.h | 2 - impeller/typographer/typographer_unittests.cc | 106 ++++------- 14 files changed, 85 insertions(+), 275 deletions(-) diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm index 9609dc4bd3692..c83f5b7075c47 100644 --- a/impeller/renderer/backend/metal/context_mtl.mm +++ b/impeller/renderer/backend/metal/context_mtl.mm @@ -55,7 +55,6 @@ static bool DeviceSupportsComputeSubgroups(id device) { .SetSupportsBufferToTextureBlits(true) .SetSupportsTextureToTextureBlits(true) .SetSupportsDecalTileMode(true) - .SetSupportsSharedDeviceBufferTextureMemory(true) .SetSupportsFramebufferFetch(DeviceSupportsFramebufferFetch(device)) .SetDefaultColorFormat(color_format) .SetDefaultStencilFormat(PixelFormat::kS8UInt) diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index 91bb80073e6e9..ff4410f15935a 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -348,16 +348,10 @@ 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/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index 0628b23d38d58..006ed6f0e0889 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -79,9 +79,6 @@ class CapabilitiesVK final : public Capabilities, // |Capabilities| bool SupportsDecalTileMode() const override; - // |Capabilities| - bool SupportsSharedDeviceBufferTextureMemory() const override; - // |Capabilities| PixelFormat GetDefaultColorFormat() const override; diff --git a/impeller/renderer/capabilities.cc b/impeller/renderer/capabilities.cc index b3106abdc79c5..d0531f5f265b2 100644 --- a/impeller/renderer/capabilities.cc +++ b/impeller/renderer/capabilities.cc @@ -66,11 +66,6 @@ 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_; @@ -93,7 +88,6 @@ 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), @@ -108,8 +102,6 @@ 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) {} @@ -126,7 +118,6 @@ 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; @@ -211,12 +202,6 @@ 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_, // @@ -230,7 +215,6 @@ 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/impeller/renderer/capabilities.h b/impeller/renderer/capabilities.h index 2c025a329ca6c..6fb1118dd7c95 100644 --- a/impeller/renderer/capabilities.h +++ b/impeller/renderer/capabilities.h @@ -37,8 +37,6 @@ class Capabilities { virtual bool SupportsDecalTileMode() const = 0; - virtual bool SupportsSharedDeviceBufferTextureMemory() const = 0; - virtual PixelFormat GetDefaultColorFormat() const = 0; virtual PixelFormat GetDefaultStencilFormat() const = 0; @@ -81,8 +79,6 @@ class CapabilitiesBuilder { CapabilitiesBuilder& SetSupportsDecalTileMode(bool value); - CapabilitiesBuilder& SetSupportsSharedDeviceBufferTextureMemory(bool value); - std::unique_ptr Build(); private: @@ -97,7 +93,6 @@ 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/impeller/renderer/capabilities_unittests.cc b/impeller/renderer/capabilities_unittests.cc index 342dcba2b8a99..1dc9d17e3c678 100644 --- a/impeller/renderer/capabilities_unittests.cc +++ b/impeller/renderer/capabilities_unittests.cc @@ -29,7 +29,6 @@ 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/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index 2a53d8869bfd3..51ec87b712d30 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -10,13 +10,11 @@ #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 @@ -38,16 +36,6 @@ 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)) {} @@ -150,21 +138,13 @@ namespace { ISize OptimumAtlasSizeForFontGlyphPairs( const FontGlyphPair::Set& pairs, std::vector& glyph_positions, - 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; + const std::shared_ptr& atlas_context) { + static constexpr auto kMinAtlasSize = 8u; 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(minimum_size, minimum_size); + ISize current_size(kMinAtlasSize, kMinAtlasSize); size_t total_pairs = pairs.size() + 1; do { auto rect_packer = std::shared_ptr( @@ -366,12 +346,9 @@ static bool UpdateAtlasBitmap(const GlyphAtlas& atlas, return true; } -static std::pair, std::shared_ptr> -CreateAtlasBitmap(const GlyphAtlas& atlas, - std::shared_ptr allocator, - const ISize& atlas_size) { +static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, + const ISize& atlas_size) { TRACE_EVENT0("impeller", __FUNCTION__); - auto font_allocator = FontImpellerAllocator(std::move(allocator)); auto bitmap = std::make_shared(); SkImageInfo image_info; @@ -386,18 +363,17 @@ CreateAtlasBitmap(const GlyphAtlas& atlas, break; } - bitmap->setInfo(image_info); - if (!bitmap->tryAllocPixels(&font_allocator)) { - return std::make_pair(nullptr, nullptr); + if (!bitmap->tryAllocPixels(image_info)) { + return nullptr; } auto surface = SkSurface::MakeRasterDirect(bitmap->pixmap()); if (!surface) { - return std::make_pair(nullptr, nullptr); + return nullptr; } auto canvas = surface->getCanvas(); if (!canvas) { - return std::make_pair(nullptr, nullptr); + return nullptr; } bool has_color = atlas.GetType() == GlyphAtlas::Type::kColorBitmap; @@ -408,16 +384,13 @@ CreateAtlasBitmap(const GlyphAtlas& atlas, return true; }); - 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()); + return bitmap; } 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(); @@ -431,12 +404,14 @@ static bool UpdateGlyphTextureAtlas(std::shared_ptr bitmap, } static std::shared_ptr UploadGlyphTextureAtlas( - Allocator& allocator, - const std::shared_ptr& device_buffer, - const std::shared_ptr& bitmap, + const std::shared_ptr& allocator, + 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(); @@ -446,27 +421,32 @@ 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; } - FML_DCHECK(allocator.MinimumBytesPerRow(format) <= pixmap.rowBytes()); - auto texture = device_buffer->AsTexture(allocator, texture_descriptor, - texture_descriptor.GetBytesPerRow()); + auto texture = allocator->CreateTexture(texture_descriptor); 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()) { @@ -522,42 +502,27 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- // Step 5: Draw new font-glyph pairs into the existing bitmap. // --------------------------------------------------------------------------- - auto [bitmap, device_buffer] = atlas_context->GetBitmap(); + auto bitmap = 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 (!capabilities->SupportsSharedDeviceBufferTextureMemory() && - !UpdateGlyphTextureAtlas(bitmap, last_atlas->GetTexture())) { + if (!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, min_alignment); + font_glyph_pairs, glyph_positions, atlas_context); atlas_context->UpdateGlyphAtlas(glyph_atlas, atlas_size); if (atlas_size.IsEmpty()) { @@ -587,24 +552,30 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- // Step 7: Draw font-glyph pairs in the correct spot in the atlas. // --------------------------------------------------------------------------- - auto [bitmap, device_buffer] = CreateAtlasBitmap( - *glyph_atlas, GetContext()->GetResourceAllocator(), atlas_size); + auto bitmap = CreateAtlasBitmap(*glyph_atlas, atlas_size); if (!bitmap) { return nullptr; } - atlas_context->UpdateBitmap(bitmap, device_buffer); + atlas_context->UpdateBitmap(bitmap); // --------------------------------------------------------------------------- // Step 8: Upload the atlas as a texture. // --------------------------------------------------------------------------- - if (type == GlyphAtlas::Type::kSignedDistanceField) { - ConvertBitmapToSignedDistanceField( - reinterpret_cast(bitmap->getPixels()), atlas_size.width, - atlas_size.height); - } - auto texture = - UploadGlyphTextureAtlas(*GetContext()->GetResourceAllocator().get(), - device_buffer, bitmap, atlas_size, format); + 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; + } + auto texture = UploadGlyphTextureAtlas(GetContext()->GetResourceAllocator(), + bitmap, atlas_size, format); if (!texture) { return nullptr; } @@ -617,43 +588,4 @@ 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/impeller/typographer/backends/skia/text_render_context_skia.h b/impeller/typographer/backends/skia/text_render_context_skia.h index 49200c8b4c12c..0c97a8716e7ad 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.h +++ b/impeller/typographer/backends/skia/text_render_context_skia.h @@ -6,37 +6,9 @@ #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); @@ -47,7 +19,6 @@ 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/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index c5c0be86436d5..6908b9eaa2029 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -22,9 +22,8 @@ const ISize& GlyphAtlasContext::GetAtlasSize() const { return atlas_size_; } -std::pair, std::shared_ptr> -GlyphAtlasContext::GetBitmap() const { - return std::make_pair(bitmap_, device_buffer_); +std::shared_ptr GlyphAtlasContext::GetBitmap() const { + return bitmap_; } std::shared_ptr GlyphAtlasContext::GetRectPacker() const { @@ -37,11 +36,8 @@ void GlyphAtlasContext::UpdateGlyphAtlas(std::shared_ptr atlas, atlas_size_ = size; } -void GlyphAtlasContext::UpdateBitmap( - std::shared_ptr bitmap, - std::shared_ptr device_buffer) { +void GlyphAtlasContext::UpdateBitmap(std::shared_ptr bitmap) { bitmap_ = std::move(bitmap); - device_buffer_ = std::move(device_buffer); } void GlyphAtlasContext::UpdateRectPacker( diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index 0e342fab1e970..e7282a0cdb6a0 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -10,7 +10,6 @@ #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" @@ -154,8 +153,7 @@ class GlyphAtlasContext { //---------------------------------------------------------------------------- /// @brief Retrieve the previous (if any) SkBitmap instance. - std::pair, std::shared_ptr> - GetBitmap() const; + std::shared_ptr GetBitmap() const; //---------------------------------------------------------------------------- /// @brief Retrieve the previous (if any) rect packer. @@ -165,8 +163,7 @@ 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, - std::shared_ptr device_buffer); + void UpdateBitmap(std::shared_ptr bitmap); void UpdateRectPacker(std::shared_ptr rect_packer); @@ -174,7 +171,6 @@ 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/impeller/typographer/lazy_glyph_atlas.cc b/impeller/typographer/lazy_glyph_atlas.cc index 23969d787ccf3..3e415716a528c 100644 --- a/impeller/typographer/lazy_glyph_atlas.cc +++ b/impeller/typographer/lazy_glyph_atlas.cc @@ -37,7 +37,6 @@ 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; @@ -51,8 +50,8 @@ std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( i++; return &result; }; - auto atlas = text_context->CreateGlyphAtlas(type, std::move(atlas_context), - capabilities, iterator); + auto atlas = + text_context->CreateGlyphAtlas(type, std::move(atlas_context), iterator); if (!atlas || !atlas->IsValid()) { VALIDATION_LOG << "Could not create valid atlas."; return nullptr; diff --git a/impeller/typographer/text_render_context.cc b/impeller/typographer/text_render_context.cc index 1fdc17878ac0c..8cc8dbf00a02e 100644 --- a/impeller/typographer/text_render_context.cc +++ b/impeller/typographer/text_render_context.cc @@ -29,7 +29,6 @@ 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* { @@ -39,8 +38,7 @@ std::shared_ptr TextRenderContext::CreateGlyphAtlas( } return nullptr; }; - return CreateGlyphAtlas(type, std::move(atlas_context), capabilities, - iterator); + return CreateGlyphAtlas(type, std::move(atlas_context), iterator); } } // namespace impeller diff --git a/impeller/typographer/text_render_context.h b/impeller/typographer/text_render_context.h index ca1dc74d129e8..c63ac912a1fa8 100644 --- a/impeller/typographer/text_render_context.h +++ b/impeller/typographer/text_render_context.h @@ -45,13 +45,11 @@ 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/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 5223553136b0d..445cc283fe6c7 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -4,7 +4,6 @@ #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" @@ -42,9 +41,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, - CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); + auto atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas->GetType(), GlyphAtlas::Type::kAlphaBitmap); @@ -117,9 +116,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, - CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); + auto atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -134,18 +133,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, - CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); + auto atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + 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, - CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); + auto next_atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + TextFrameFromTextBlob(blob)); ASSERT_EQ(atlas, next_atlas); ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); } @@ -174,9 +173,8 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { } return nullptr; }; - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - CapabilitiesBuilder().Build(), iterator); + auto atlas = context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, + atlas_context, iterator); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -184,16 +182,18 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { atlas->GetTexture()->GetSize().height); } -TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { +// TODO(jonahwilliams): Re-enable +// https://github.com/flutter/flutter/issues/122839 +TEST_P(TypographerTest, DISABLED_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, - CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); + auto atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + TextFrameFromTextBlob(blob)); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -205,9 +205,9 @@ TEST_P(TypographerTest, 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, - CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob2)); + auto next_atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + 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, - CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); + auto atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + 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, - CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob2)); + auto next_atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kColorBitmap, atlas_context, + TextFrameFromTextBlob(blob2)); ASSERT_NE(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -251,54 +251,6 @@ 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 = {