From a31ac1ee8b5122d31b609bbc750653c77d5c122d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 11 Nov 2022 11:04:23 -0800 Subject: [PATCH 1/3] [Impeller] reuse texture --- .../backends/skia/text_render_context_skia.cc | 63 +++++++++++++------ 1 file changed, 45 insertions(+), 18 deletions(-) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index c1b6405bc193b..f0bdd5d328406 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -4,6 +4,7 @@ #include "impeller/typographer/backends/skia/text_render_context_skia.h" +#include #include #include "flutter/fml/logging.h" @@ -297,9 +298,8 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, return bitmap; } -static std::shared_ptr UploadGlyphTextureAtlas( +static std::shared_ptr CreateGlyphTextureAtlas( const std::shared_ptr& allocator, - std::shared_ptr bitmap, const ISize& atlas_size, PixelFormat format) { TRACE_EVENT0("impeller", __FUNCTION__); @@ -307,24 +307,32 @@ static std::shared_ptr UploadGlyphTextureAtlas( return nullptr; } - FML_DCHECK(bitmap != nullptr); - const auto& pixmap = bitmap->pixmap(); - TextureDescriptor texture_descriptor; texture_descriptor.storage_mode = StorageMode::kHostVisible; texture_descriptor.format = format; texture_descriptor.size = atlas_size; - if (pixmap.rowBytes() * pixmap.height() != - texture_descriptor.GetByteSizeOfBaseMipLevel()) { - return nullptr; - } - auto texture = allocator->CreateTexture(texture_descriptor); if (!texture || !texture->IsValid()) { return nullptr; } texture->SetLabel("GlyphAtlas"); + return texture; +} + +bool UploadGlyphTextureAtlas(const std::shared_ptr& texture, + std::shared_ptr bitmap) { + TRACE_EVENT0("impeller", __FUNCTION__); + + FML_DCHECK(bitmap != nullptr); + const auto& pixmap = bitmap->pixmap(); + + auto texture_descriptor = texture->GetTextureDescriptor(); + if (pixmap.rowBytes() * pixmap.height() != + texture_descriptor.GetByteSizeOfBaseMipLevel()) { + std::cerr << "descriptor mismatch" << std::endl; + return false; + } auto mapping = std::make_shared( reinterpret_cast(bitmap->getAddr(0, 0)), // data @@ -333,9 +341,10 @@ static std::shared_ptr UploadGlyphTextureAtlas( ); if (!texture->SetContents(mapping)) { - return nullptr; + std::cerr << "contents cant be set" << std::endl; + return false; } - return texture; + return true; } std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( @@ -421,16 +430,34 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( format = PixelFormat::kR8G8B8A8UNormInt; break; } - auto texture = UploadGlyphTextureAtlas(GetContext()->GetResourceAllocator(), - bitmap, atlas_size, format); - if (!texture) { - return nullptr; - } // --------------------------------------------------------------------------- // Step 8: Record the texture in the glyph atlas. + // + // If the last_texture is the same size and type, reuse this instead of + // creating a new texture. // --------------------------------------------------------------------------- - glyph_atlas->SetTexture(std::move(texture)); + auto old_texture = last_atlas->GetTexture(); + if (old_texture != nullptr && + old_texture->GetTextureDescriptor().size == atlas_size) { + std::cerr << "reuse atlas" << std::endl; + if (!UploadGlyphTextureAtlas(old_texture, bitmap)) { + return nullptr; + } + glyph_atlas->SetTexture(std::move(old_texture)); + } else { + std::cerr << "can't reuse " + << (old_texture != nullptr + ? old_texture->GetTextureDescriptor().size + : ISize(0, 0)) + << " : " << atlas_size << std::endl; + auto texture = CreateGlyphTextureAtlas(GetContext()->GetResourceAllocator(), + atlas_size, format); + if (!texture || !UploadGlyphTextureAtlas(texture, bitmap)) { + return nullptr; + } + glyph_atlas->SetTexture(std::move(texture)); + } return glyph_atlas; } From 06c73cb1dbe827ebf482b630cdf8286d98feaa36 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 11 Nov 2022 11:18:02 -0800 Subject: [PATCH 2/3] ++ --- .../backends/skia/text_render_context_skia.cc | 9 ------ impeller/typographer/typographer_unittests.cc | 28 +++++++++++++++++++ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index f0bdd5d328406..c095cc624e9d7 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -4,7 +4,6 @@ #include "impeller/typographer/backends/skia/text_render_context_skia.h" -#include #include #include "flutter/fml/logging.h" @@ -330,7 +329,6 @@ bool UploadGlyphTextureAtlas(const std::shared_ptr& texture, auto texture_descriptor = texture->GetTextureDescriptor(); if (pixmap.rowBytes() * pixmap.height() != texture_descriptor.GetByteSizeOfBaseMipLevel()) { - std::cerr << "descriptor mismatch" << std::endl; return false; } @@ -341,7 +339,6 @@ bool UploadGlyphTextureAtlas(const std::shared_ptr& texture, ); if (!texture->SetContents(mapping)) { - std::cerr << "contents cant be set" << std::endl; return false; } return true; @@ -440,17 +437,11 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( auto old_texture = last_atlas->GetTexture(); if (old_texture != nullptr && old_texture->GetTextureDescriptor().size == atlas_size) { - std::cerr << "reuse atlas" << std::endl; if (!UploadGlyphTextureAtlas(old_texture, bitmap)) { return nullptr; } glyph_atlas->SetTexture(std::move(old_texture)); } else { - std::cerr << "can't reuse " - << (old_texture != nullptr - ? old_texture->GetTextureDescriptor().size - : ISize(0, 0)) - << " : " << atlas_size << std::endl; auto texture = CreateGlyphTextureAtlas(GetContext()->GetResourceAllocator(), atlas_size, format); if (!texture || !UploadGlyphTextureAtlas(texture, bitmap)) { diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 7bd81264c4df7..4e9a79f3dd134 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -137,6 +137,34 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); } +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 skellingtons", sk_font); + ASSERT_TRUE(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()); + + auto* first_texture = atlas->GetTexture().get(); + + // now create a new glyph atlas with a nearly identical blob. + + auto blob2 = SkTextBlob::MakeFromString("spooky skellington2", sk_font); + auto next_atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + TextFrameFromTextBlob(blob2)); + ASSERT_NE(atlas, next_atlas); + auto* second_texture = next_atlas->GetTexture().get(); + + ASSERT_EQ(second_texture, first_texture); +} + TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { auto context = TextRenderContext::Create(GetContext()); auto atlas_context = std::make_shared(); From 20da57db124fd0a3acacc2cd8aa946684b1fc90a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 11 Nov 2022 11:20:05 -0800 Subject: [PATCH 3/3] ++ --- impeller/typographer/backends/skia/text_render_context_skia.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index c095cc624e9d7..b710768505a9c 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -436,7 +436,8 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- auto old_texture = last_atlas->GetTexture(); if (old_texture != nullptr && - old_texture->GetTextureDescriptor().size == atlas_size) { + old_texture->GetTextureDescriptor().size == atlas_size && + old_texture->GetTextureDescriptor().format == format) { if (!UploadGlyphTextureAtlas(old_texture, bitmap)) { return nullptr; }