From 829e8af261547d5351c10cd129a8bc49271e3e0c Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 12 May 2024 10:52:17 -0700 Subject: [PATCH 01/10] [Impeller] ever growing atlas. --- .../backends/skia/typographer_context_skia.cc | 213 ++++++++++-------- impeller/typographer/rectangle_packer.cc | 10 +- impeller/typographer/rectangle_packer.h | 4 +- 3 files changed, 121 insertions(+), 106 deletions(-) diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 25f36c73bc05c..3164aa28a1426 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include "flutter/fml/logging.h" #include "flutter/fml/trace_event.h" @@ -19,10 +20,12 @@ #include "impeller/core/host_buffer.h" #include "impeller/core/platform.h" #include "impeller/core/texture_descriptor.h" +#include "impeller/geometry/size.h" #include "impeller/renderer/command_buffer.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/render_target.h" #include "impeller/typographer/backends/skia/typeface_skia.h" +#include "impeller/typographer/font_glyph_pair.h" #include "impeller/typographer/glyph_atlas.h" #include "impeller/typographer/rectangle_packer.h" #include "impeller/typographer/typographer_context.h" @@ -152,7 +155,9 @@ static size_t PairsFitInAtlasOfSize( return 0; } -static bool CanAppendToExistingAtlas( +/// Append as many glyphs to the texture as will fit, and return any +/// glyphs that did not. +static std::vector AppendToExistingAtlas( const std::shared_ptr& atlas, const std::vector& extra_pairs, std::vector& glyph_positions, @@ -160,17 +165,25 @@ static bool CanAppendToExistingAtlas( const std::shared_ptr& rect_packer) { TRACE_EVENT0("impeller", __FUNCTION__); if (!rect_packer || atlas_size.IsEmpty()) { - return false; + return {}; } // We assume that all existing glyphs will fit. After all, they fit before. // The glyph_positions only contains the values for the additional glyphs // from extra_pairs. FML_DCHECK(glyph_positions.size() == 0); - glyph_positions.reserve(extra_pairs.size()); + // glyph_positions.reserve(extra_pairs.size()); + std::vector missing; + bool stop_adding = false; + for (size_t i = 0; i < extra_pairs.size(); i++) { const FontGlyphPair& pair = extra_pairs[i]; + if (stop_adding) { + missing.push_back(pair); + continue; + } + const auto glyph_size = ISize::Ceil(pair.glyph.bounds.GetSize() * pair.scaled_font.scale); IPoint16 location_in_atlas; @@ -178,33 +191,33 @@ static bool CanAppendToExistingAtlas( glyph_size.height + kPadding, // &location_in_atlas // )) { - return false; + missing.push_back(pair); + stop_adding = true; + continue; } - glyph_positions.emplace_back(Rect::MakeXYWH(location_in_atlas.x(), // - location_in_atlas.y(), // - glyph_size.width, // - glyph_size.height // - )); + glyph_positions.push_back(Rect::MakeXYWH(location_in_atlas.x(), // + location_in_atlas.y(), // + glyph_size.width, // + glyph_size.height // + )); } - return true; + return missing; } static ISize OptimumAtlasSizeForFontGlyphPairs( const std::vector& pairs, - std::vector& glyph_positions, const std::shared_ptr& atlas_context, - GlyphAtlas::Type type, const ISize& max_texture_size) { - static constexpr auto kMinAtlasSize = 8u; - static constexpr auto kMinAlphaBitmapSize = 1024u; + // Because we can't grow the skyline packer horizontally, pick a reasonable + // large width for all atlases. + static constexpr auto kAtlasWidth = 4096u; + static constexpr auto kMinAtlasHeight = 1024u; TRACE_EVENT0("impeller", __FUNCTION__); - ISize current_size = type == GlyphAtlas::Type::kAlphaBitmap - ? ISize(kMinAlphaBitmapSize, kMinAlphaBitmapSize) - : ISize(kMinAtlasSize, kMinAtlasSize); - size_t total_pairs = pairs.size() + 1; + std::vector glyph_positions; + ISize current_size = ISize(kAtlasWidth, kMinAtlasHeight); do { auto rect_packer = std::shared_ptr( RectanglePacker::Factory(current_size.width, current_size.height)); @@ -214,15 +227,9 @@ static ISize OptimumAtlasSizeForFontGlyphPairs( if (remaining_pairs == 0) { atlas_context->UpdateRectPacker(rect_packer); return current_size; - } else if (remaining_pairs < std::ceil(total_pairs / 2)) { - current_size = ISize::MakeWH( - std::max(current_size.width, current_size.height), - Allocation::NextPowerOfTwoSize( - std::min(current_size.width, current_size.height) + 1)); } else { current_size = ISize::MakeWH( - Allocation::NextPowerOfTwoSize(current_size.width + 1), - Allocation::NextPowerOfTwoSize(current_size.height + 1)); + kAtlasWidth, Allocation::NextPowerOfTwoSize(current_size.height + 1)); } } while (current_size.width <= max_texture_size.width && current_size.height <= max_texture_size.height); @@ -322,6 +329,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( if (font_glyph_map.empty()) { return last_atlas; } + std::shared_ptr cmd_buffer = context.CreateCommandBuffer(); std::shared_ptr blit_pass = cmd_buffer->CreateBlitPass(); @@ -357,21 +365,22 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- // Step 2: Determine if the additional missing glyphs can be appended to the - // existing bitmap without recreating the atlas. This requires that - // the type is identical. + // existing bitmap without recreating the atlas. // --------------------------------------------------------------------------- std::vector glyph_positions; - if (CanAppendToExistingAtlas(last_atlas, new_glyphs, glyph_positions, - atlas_context->GetAtlasSize(), - atlas_context->GetRectPacker())) { - // The old bitmap will be reused and only the additional glyphs will be - // added. + std::vector missing_pairs; + size_t i = 0; + if (last_atlas->GetTexture()) { + // Append all glyphs that fit into the current atlas. + missing_pairs = AppendToExistingAtlas( + last_atlas, new_glyphs, glyph_positions, atlas_context->GetAtlasSize(), + atlas_context->GetRectPacker()); // --------------------------------------------------------------------------- // Step 3a: Record the positions in the glyph atlas of the newly added // glyphs. // --------------------------------------------------------------------------- - for (size_t i = 0, count = glyph_positions.size(); i < count; i++) { + for (size_t count = glyph_positions.size(); i < count; i++) { last_atlas->AddTypefaceGlyphPosition(new_glyphs[i], glyph_positions[i]); } @@ -384,83 +393,62 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( return nullptr; } - return last_atlas; - } - // A new glyph atlas must be created. - - // --------------------------------------------------------------------------- - // Step 3b: Get the optimum size of the texture atlas. - // --------------------------------------------------------------------------- - std::vector font_glyph_pairs; - font_glyph_pairs.reserve(std::accumulate( - font_glyph_map.begin(), font_glyph_map.end(), 0, - [](const int a, const auto& b) { return a + b.second.size(); })); - for (const auto& font_value : font_glyph_map) { - const ScaledFont& scaled_font = font_value.first; - for (const Glyph& glyph : font_value.second) { - font_glyph_pairs.push_back({scaled_font, glyph}); + // If all glyphs fit, just return the old atlas. + if (missing_pairs.empty()) { + return last_atlas; } } - std::shared_ptr glyph_atlas = std::make_shared(type); + + // A new glyph atlas must be created. Compute the size needed by all of the + // glyphs that couldn't be added to the existing atlas. ISize atlas_size = OptimumAtlasSizeForFontGlyphPairs( - font_glyph_pairs, // - glyph_positions, // - atlas_context, // - type, // + last_atlas->GetTexture() + ? missing_pairs + : new_glyphs, // // + atlas_context, // context.GetResourceAllocator()->GetMaxTextureSizeSupported() // ); - atlas_context->UpdateGlyphAtlas(glyph_atlas, atlas_size); + atlas_context->UpdateGlyphAtlas(last_atlas, atlas_size); if (atlas_size.IsEmpty()) { return nullptr; } - // --------------------------------------------------------------------------- - // Step 4b: Find location of font-glyph pairs in the atlas. We have this from - // the last step. So no need to do create another rect packer. But just do a - // sanity check of counts. This could also be just an assertion as only a - // construction issue would cause such a failure. - // --------------------------------------------------------------------------- - if (glyph_positions.size() != font_glyph_pairs.size()) { - return nullptr; - } - // --------------------------------------------------------------------------- - // Step 5b: Record the positions in the glyph atlas. - // --------------------------------------------------------------------------- - { - size_t i = 0; - for (auto it = font_glyph_pairs.begin(); it != font_glyph_pairs.end(); - ++i, ++it) { - glyph_atlas->AddTypefaceGlyphPosition(*it, glyph_positions[i]); - } + // Compute the new atlas size. + ISize new_atlas_size; + if (last_atlas->GetTexture()) { + new_atlas_size = + ISize{atlas_size.width, + atlas_size.height + last_atlas->GetTexture()->GetSize().height}; + } else { + new_atlas_size = atlas_size; } - // If the new atlas size is the same size as the previous texture, reuse the - // texture and treat this as an updated that replaces all glyphs. - std::shared_ptr new_texture; - if (last_atlas && last_atlas->GetTexture() && - last_atlas->GetTexture()->GetSize() == atlas_size) { - new_texture = last_atlas->GetTexture(); + if (atlas_context->GetRectPacker()) { + auto new_rect_packer = + atlas_context->GetRectPacker()->Clone(new_atlas_size.height); + atlas_context->UpdateRectPacker(std::move(new_rect_packer)); } else { - // Otherwise, create a new texture. - TextureDescriptor descriptor; - switch (type) { - case GlyphAtlas::Type::kAlphaBitmap: - descriptor.format = - context.GetCapabilities()->GetDefaultGlyphAtlasFormat(); - break; - case GlyphAtlas::Type::kColorBitmap: - descriptor.format = PixelFormat::kR8G8B8A8UNormInt; - break; - } - descriptor.size = atlas_size; - descriptor.storage_mode = StorageMode::kDevicePrivate; - new_texture = context.GetResourceAllocator()->CreateTexture(descriptor); + atlas_context->UpdateRectPacker( + RectanglePacker::Factory(new_atlas_size.width, new_atlas_size.height)); } - if (!new_texture) { - return nullptr; + TextureDescriptor descriptor; + switch (type) { + case GlyphAtlas::Type::kAlphaBitmap: + descriptor.format = + context.GetCapabilities()->GetDefaultGlyphAtlasFormat(); + break; + case GlyphAtlas::Type::kColorBitmap: + descriptor.format = PixelFormat::kR8G8B8A8UNormInt; + break; } + descriptor.size = new_atlas_size; + descriptor.storage_mode = StorageMode::kDevicePrivate; + std::shared_ptr new_texture = + context.GetResourceAllocator()->CreateTexture(descriptor); + new_texture->SetLabel("GlyphAtlas"); + // The texture needs to be cleared to transparent black so that linearly // samplex rotated/skewed glyphs do not grab uninitialized data. We could // instead use a render pass to clear to transparent black, but there are @@ -476,18 +464,45 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( blit_pass->AddCopy(buffer_view, new_texture); } - new_texture->SetLabel("GlyphAtlas"); - if (!UpdateAtlasBitmap(*glyph_atlas, blit_pass, host_buffer, new_texture, - font_glyph_pairs)) { + // Blit the old texture to the top left of the new atlas. + if (last_atlas->GetTexture()) { + blit_pass->AddCopy(last_atlas->GetTexture(), new_texture, + IRect::MakeSize(last_atlas->GetTexture()->GetSize()), + {0, 0}); + } + + // Now append all remaining glyphs. This should never have any missing data... + last_atlas->SetTexture(std::move(new_texture)); + auto expect_empty = AppendToExistingAtlas( + last_atlas, last_atlas->GetTexture() ? missing_pairs : new_glyphs, + glyph_positions, atlas_context->GetAtlasSize(), + atlas_context->GetRectPacker()); + if (!expect_empty.empty()) { + return nullptr; + } + + // --------------------------------------------------------------------------- + // Step 3a: Record the positions in the glyph atlas of the newly added + // glyphs. + // --------------------------------------------------------------------------- + for (size_t count = glyph_positions.size(); i < count; i++) { + last_atlas->AddTypefaceGlyphPosition(new_glyphs[i], glyph_positions[i]); + } + + // --------------------------------------------------------------------------- + // Step 4a: Draw new font-glyph pairs into the a host buffer and encode + // the uploads into the blit pass. + // --------------------------------------------------------------------------- + if (!UpdateAtlasBitmap(*last_atlas, blit_pass, host_buffer, + last_atlas->GetTexture(), new_glyphs)) { return nullptr; } // --------------------------------------------------------------------------- // Step 8b: Record the texture in the glyph atlas. // --------------------------------------------------------------------------- - glyph_atlas->SetTexture(std::move(new_texture)); - return glyph_atlas; + return last_atlas; } } // namespace impeller diff --git a/impeller/typographer/rectangle_packer.cc b/impeller/typographer/rectangle_packer.cc index 209a0d69c8042..ffb2a8a08582f 100644 --- a/impeller/typographer/rectangle_packer.cc +++ b/impeller/typographer/rectangle_packer.cc @@ -33,7 +33,7 @@ class SkylineRectanglePacker final : public RectanglePacker { return area_so_far_ / ((float)this->width() * this->height()); } - std::unique_ptr Clone(uint32_t scale) final; + std::unique_ptr Clone(uint32_t height) final; private: struct SkylineSegment { @@ -168,10 +168,10 @@ void SkylineRectanglePacker::addSkylineLevel(int skylineIndex, } } -std::unique_ptr SkylineRectanglePacker::Clone(uint32_t scale) { - FML_DCHECK(scale != 0); - auto packer = - std::make_unique(width(), height() * scale); +std::unique_ptr SkylineRectanglePacker::Clone( + uint32_t height) { + FML_DCHECK(height >= this->height()); + auto packer = std::make_unique(width(), height); for (SkylineSegment segment : skyline_) { packer->skyline_.push_back(segment); } diff --git a/impeller/typographer/rectangle_packer.h b/impeller/typographer/rectangle_packer.h index 5d8ea0b519a49..d7ea2150d5aa0 100644 --- a/impeller/typographer/rectangle_packer.h +++ b/impeller/typographer/rectangle_packer.h @@ -55,14 +55,14 @@ class RectanglePacker { /// @brief Create a new rectangle packer with a larger scaled height /// scaled and initialize its contents to the current packer. /// - /// @param[in] scale The scaling factor to be applied to the new height. + /// @param[in] scale The new hight for the rect packer. /// /// @return A new rectangle packer. /// /// This method is used for growing the glyph atlas while keeping /// existing rects in place. The width of the rectangle packer /// cannot be increased. - virtual std::unique_ptr Clone(uint32_t scale) = 0; + virtual std::unique_ptr Clone(uint32_t height) = 0; //---------------------------------------------------------------------------- /// @brief Empty out all previously added rectangles. From 08fb21c742b76ef472a775c740b7d8b7cf2a195d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 12 May 2024 22:15:23 -0700 Subject: [PATCH 02/10] ++ --- .../backends/skia/typographer_context_skia.cc | 145 +++++------------- 1 file changed, 35 insertions(+), 110 deletions(-) diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 3164aa28a1426..50a5f03cb0cf3 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -13,7 +13,6 @@ #include "flutter/fml/trace_event.h" #include "fml/closure.h" -#include "impeller/base/allocation.h" #include "impeller/core/allocator.h" #include "impeller/core/buffer_view.h" #include "impeller/core/formats.h" @@ -120,70 +119,28 @@ static SkImageInfo GetImageInfo(const GlyphAtlas& atlas, Size size) { FML_UNREACHABLE(); } -static size_t PairsFitInAtlasOfSize( - const std::vector& pairs, - const ISize& atlas_size, - std::vector& glyph_positions, - const std::shared_ptr& rect_packer) { - if (atlas_size.IsEmpty()) { - return false; - } - - glyph_positions.clear(); - glyph_positions.reserve(pairs.size()); - - size_t i = 0; - for (auto it = pairs.begin(); it != pairs.end(); ++i, ++it) { - const auto& pair = *it; - - const auto glyph_size = - ISize::Ceil(pair.glyph.bounds.GetSize() * pair.scaled_font.scale); - IPoint16 location_in_atlas; - if (!rect_packer->AddRect(glyph_size.width + kPadding, // - glyph_size.height + kPadding, // - &location_in_atlas // - )) { - return pairs.size() - i; - } - glyph_positions.emplace_back(Rect::MakeXYWH(location_in_atlas.x(), // - location_in_atlas.y(), // - glyph_size.width, // - glyph_size.height // - )); - } - - return 0; -} - -/// Append as many glyphs to the texture as will fit, and return any -/// glyphs that did not. -static std::vector AppendToExistingAtlas( +/// Append as many glyphs to the texture as will fit, and return the first index +/// of [extra_pairs] that did not fit. +static size_t AppendToExistingAtlas( const std::shared_ptr& atlas, const std::vector& extra_pairs, std::vector& glyph_positions, ISize atlas_size, - const std::shared_ptr& rect_packer) { + const std::shared_ptr& rect_packer, + size_t glyph_index_start = 0) { TRACE_EVENT0("impeller", __FUNCTION__); if (!rect_packer || atlas_size.IsEmpty()) { - return {}; + return 0; } // We assume that all existing glyphs will fit. After all, they fit before. // The glyph_positions only contains the values for the additional glyphs // from extra_pairs. FML_DCHECK(glyph_positions.size() == 0); - // glyph_positions.reserve(extra_pairs.size()); - std::vector missing; - bool stop_adding = false; + glyph_positions.reserve(extra_pairs.size()); - for (size_t i = 0; i < extra_pairs.size(); i++) { + for (size_t i = glyph_index_start; i < extra_pairs.size(); i++) { const FontGlyphPair& pair = extra_pairs[i]; - - if (stop_adding) { - missing.push_back(pair); - continue; - } - const auto glyph_size = ISize::Ceil(pair.glyph.bounds.GetSize() * pair.scaled_font.scale); IPoint16 location_in_atlas; @@ -191,9 +148,7 @@ static std::vector AppendToExistingAtlas( glyph_size.height + kPadding, // &location_in_atlas // )) { - missing.push_back(pair); - stop_adding = true; - continue; + return i; } glyph_positions.push_back(Rect::MakeXYWH(location_in_atlas.x(), // location_in_atlas.y(), // @@ -202,38 +157,20 @@ static std::vector AppendToExistingAtlas( )); } - return missing; + return extra_pairs.size(); } -static ISize OptimumAtlasSizeForFontGlyphPairs( - const std::vector& pairs, - const std::shared_ptr& atlas_context, - const ISize& max_texture_size) { +static ISize ComputeNextAtlasSize(ISize old_size, int64_t max_height) { // Because we can't grow the skyline packer horizontally, pick a reasonable // large width for all atlases. static constexpr auto kAtlasWidth = 4096u; static constexpr auto kMinAtlasHeight = 1024u; - TRACE_EVENT0("impeller", __FUNCTION__); - - std::vector glyph_positions; - ISize current_size = ISize(kAtlasWidth, kMinAtlasHeight); - do { - auto rect_packer = std::shared_ptr( - RectanglePacker::Factory(current_size.width, current_size.height)); - - auto remaining_pairs = PairsFitInAtlasOfSize(pairs, current_size, - glyph_positions, rect_packer); - if (remaining_pairs == 0) { - atlas_context->UpdateRectPacker(rect_packer); - return current_size; - } else { - current_size = ISize::MakeWH( - kAtlasWidth, Allocation::NextPowerOfTwoSize(current_size.height + 1)); - } - } while (current_size.width <= max_texture_size.width && - current_size.height <= max_texture_size.height); - return ISize{0, 0}; + if (old_size.IsEmpty()) { + return ISize{kAtlasWidth, kMinAtlasHeight}; + } + return ISize{kAtlasWidth, + std::clamp(old_size.height * 2, old_size.height, max_height)}; } static void DrawGlyph(SkCanvas* canvas, @@ -368,11 +305,10 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // existing bitmap without recreating the atlas. // --------------------------------------------------------------------------- std::vector glyph_positions; - std::vector missing_pairs; - size_t i = 0; + size_t first_missing_index = 0; if (last_atlas->GetTexture()) { // Append all glyphs that fit into the current atlas. - missing_pairs = AppendToExistingAtlas( + first_missing_index = AppendToExistingAtlas( last_atlas, new_glyphs, glyph_positions, atlas_context->GetAtlasSize(), atlas_context->GetRectPacker()); @@ -380,7 +316,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // Step 3a: Record the positions in the glyph atlas of the newly added // glyphs. // --------------------------------------------------------------------------- - for (size_t count = glyph_positions.size(); i < count; i++) { + for (size_t i = 0, count = glyph_positions.size(); i < count; i++) { last_atlas->AddTypefaceGlyphPosition(new_glyphs[i], glyph_positions[i]); } @@ -394,19 +330,16 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( } // If all glyphs fit, just return the old atlas. - if (missing_pairs.empty()) { + if (first_missing_index == new_glyphs.size()) { return last_atlas; } } - // A new glyph atlas must be created. Compute the size needed by all of the - // glyphs that couldn't be added to the existing atlas. - ISize atlas_size = OptimumAtlasSizeForFontGlyphPairs( - last_atlas->GetTexture() - ? missing_pairs - : new_glyphs, // // - atlas_context, // - context.GetResourceAllocator()->GetMaxTextureSizeSupported() // + // A new glyph atlas must be created. Keep the existing width, and double the + // new height. + ISize atlas_size = ComputeNextAtlasSize( + atlas_context->GetAtlasSize(), + context.GetResourceAllocator()->GetMaxTextureSizeSupported().height // ); atlas_context->UpdateGlyphAtlas(last_atlas, atlas_size); @@ -414,23 +347,14 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( return nullptr; } - // Compute the new atlas size. - ISize new_atlas_size; - if (last_atlas->GetTexture()) { - new_atlas_size = - ISize{atlas_size.width, - atlas_size.height + last_atlas->GetTexture()->GetSize().height}; - } else { - new_atlas_size = atlas_size; - } - + // Update the rect packer. if (atlas_context->GetRectPacker()) { auto new_rect_packer = - atlas_context->GetRectPacker()->Clone(new_atlas_size.height); + atlas_context->GetRectPacker()->Clone(atlas_size.height); atlas_context->UpdateRectPacker(std::move(new_rect_packer)); } else { atlas_context->UpdateRectPacker( - RectanglePacker::Factory(new_atlas_size.width, new_atlas_size.height)); + RectanglePacker::Factory(atlas_size.width, atlas_size.height)); } TextureDescriptor descriptor; @@ -443,7 +367,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( descriptor.format = PixelFormat::kR8G8B8A8UNormInt; break; } - descriptor.size = new_atlas_size; + descriptor.size = atlas_size; descriptor.storage_mode = StorageMode::kDevicePrivate; std::shared_ptr new_texture = context.GetResourceAllocator()->CreateTexture(descriptor); @@ -473,11 +397,12 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // Now append all remaining glyphs. This should never have any missing data... last_atlas->SetTexture(std::move(new_texture)); - auto expect_empty = AppendToExistingAtlas( - last_atlas, last_atlas->GetTexture() ? missing_pairs : new_glyphs, + + size_t more_missing = AppendToExistingAtlas( + last_atlas, new_glyphs, glyph_positions, atlas_context->GetAtlasSize(), - atlas_context->GetRectPacker()); - if (!expect_empty.empty()) { + atlas_context->GetRectPacker(), first_missing_index); + if (more_missing != new_glyphs.size()) { return nullptr; } @@ -485,7 +410,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // Step 3a: Record the positions in the glyph atlas of the newly added // glyphs. // --------------------------------------------------------------------------- - for (size_t count = glyph_positions.size(); i < count; i++) { + for (size_t i = first_missing_index, count = glyph_positions.size(); i < count; i++) { last_atlas->AddTypefaceGlyphPosition(new_glyphs[i], glyph_positions[i]); } From d86a60c03977e813af669c6d7902f5774644285a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 13 May 2024 14:18:19 -0700 Subject: [PATCH 03/10] more wip --- .../backends/skia/typographer_context_skia.cc | 146 ++++++++++++------ impeller/typographer/glyph_atlas.cc | 13 +- impeller/typographer/glyph_atlas.h | 5 +- 3 files changed, 113 insertions(+), 51 deletions(-) diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 50a5f03cb0cf3..7218a74c7e49a 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -126,51 +126,104 @@ static size_t AppendToExistingAtlas( const std::vector& extra_pairs, std::vector& glyph_positions, ISize atlas_size, - const std::shared_ptr& rect_packer, - size_t glyph_index_start = 0) { + int64_t height_adjustment, + const std::shared_ptr& rect_packer) { TRACE_EVENT0("impeller", __FUNCTION__); if (!rect_packer || atlas_size.IsEmpty()) { return 0; } - // We assume that all existing glyphs will fit. After all, they fit before. - // The glyph_positions only contains the values for the additional glyphs - // from extra_pairs. - FML_DCHECK(glyph_positions.size() == 0); - glyph_positions.reserve(extra_pairs.size()); - - for (size_t i = glyph_index_start; i < extra_pairs.size(); i++) { + for (size_t i = 0; i < extra_pairs.size(); i++) { const FontGlyphPair& pair = extra_pairs[i]; const auto glyph_size = ISize::Ceil(pair.glyph.bounds.GetSize() * pair.scaled_font.scale); IPoint16 location_in_atlas; - if (!rect_packer->AddRect(glyph_size.width + kPadding, // + if (!rect_packer->addRect(glyph_size.width + kPadding, // glyph_size.height + kPadding, // &location_in_atlas // )) { return i; } - glyph_positions.push_back(Rect::MakeXYWH(location_in_atlas.x(), // - location_in_atlas.y(), // - glyph_size.width, // - glyph_size.height // - )); + glyph_positions.push_back(Rect::MakeXYWH( + location_in_atlas.x(), // + location_in_atlas.y() + height_adjustment, // + glyph_size.width, // + glyph_size.height // + )); } return extra_pairs.size(); } -static ISize ComputeNextAtlasSize(ISize old_size, int64_t max_height) { +static size_t PairsFitInAtlasOfSize( + const std::vector& pairs, + const ISize& atlas_size, + std::vector& glyph_positions, + int64_t height_adjustment, + const std::shared_ptr& rect_packer, + size_t start_index) { + FML_DCHECK(!atlas_size.IsEmpty()); + + for (size_t i = start_index; i < pairs.size(); i++) { + const auto& pair = pairs[i]; + const auto glyph_size = + ISize::Ceil(pair.glyph.bounds.GetSize() * pair.scaled_font.scale); + IPoint16 location_in_atlas; + if (!rect_packer->addRect(glyph_size.width + kPadding, // + glyph_size.height + kPadding, // + &location_in_atlas // + )) { + return i; + } + glyph_positions.push_back(Rect::MakeXYWH( + location_in_atlas.x(), // + location_in_atlas.y() + height_adjustment, // + glyph_size.width, // + glyph_size.height // + )); + } + + return pairs.size(); +} + +static ISize ComputeNextAtlasSize( + const std::shared_ptr& atlas_context, + const std::vector& extra_pairs, + std::vector& glyph_positions, + size_t glyph_index_start, + int64_t max_texture_height) { // Because we can't grow the skyline packer horizontally, pick a reasonable // large width for all atlases. static constexpr auto kAtlasWidth = 4096u; - static constexpr auto kMinAtlasHeight = 1024u; + static constexpr auto kMinAtlasHeight = 4096u; + + ISize current_size = ISize(kAtlasWidth, kMinAtlasHeight); + if (atlas_context->GetAtlasSize().height > current_size.height) { + current_size.height = atlas_context->GetAtlasSize().height * 2; + } + + auto height_adjustment = atlas_context->GetAtlasSize().height; + while (current_size.height <= max_texture_height) { + std::shared_ptr rect_packer; + if (atlas_context->GetRectPacker()) { + rect_packer = RectanglePacker::Factory( + kAtlasWidth, + current_size.height - atlas_context->GetAtlasSize().height); + } else { + FML_DCHECK(glyph_index_start == 0); + rect_packer = RectanglePacker::Factory(kAtlasWidth, current_size.height); + } - if (old_size.IsEmpty()) { - return ISize{kAtlasWidth, kMinAtlasHeight}; + atlas_context->UpdateRectPacker(rect_packer); + glyph_index_start = PairsFitInAtlasOfSize( + extra_pairs, current_size, glyph_positions, height_adjustment, + rect_packer, glyph_index_start); + if (glyph_index_start == extra_pairs.size()) { + return current_size; + } + current_size = ISize(current_size.width, current_size.height * 2); } - return ISize{kAtlasWidth, - std::clamp(old_size.height * 2, old_size.height, max_height)}; + return {}; } static void DrawGlyph(SkCanvas* canvas, @@ -209,12 +262,15 @@ static bool UpdateAtlasBitmap(const GlyphAtlas& atlas, std::shared_ptr& blit_pass, HostBuffer& host_buffer, const std::shared_ptr& texture, - const std::vector& new_pairs) { + const std::vector& new_pairs, + size_t start_index, + size_t end_index) { TRACE_EVENT0("impeller", __FUNCTION__); bool has_color = atlas.GetType() == GlyphAtlas::Type::kColorBitmap; - for (const FontGlyphPair& pair : new_pairs) { + for (size_t i = start_index; i < end_index; i++) { + const FontGlyphPair& pair = new_pairs[i]; auto pos = atlas.FindFontGlyphBounds(pair); if (!pos.has_value()) { continue; @@ -305,18 +361,20 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // existing bitmap without recreating the atlas. // --------------------------------------------------------------------------- std::vector glyph_positions; + glyph_positions.reserve(new_glyphs.size()); size_t first_missing_index = 0; + if (last_atlas->GetTexture()) { // Append all glyphs that fit into the current atlas. first_missing_index = AppendToExistingAtlas( last_atlas, new_glyphs, glyph_positions, atlas_context->GetAtlasSize(), - atlas_context->GetRectPacker()); + atlas_context->GetHeightAdjustment(), atlas_context->GetRectPacker()); // --------------------------------------------------------------------------- // Step 3a: Record the positions in the glyph atlas of the newly added // glyphs. // --------------------------------------------------------------------------- - for (size_t i = 0, count = glyph_positions.size(); i < count; i++) { + for (size_t i = 0; i < first_missing_index; i++) { last_atlas->AddTypefaceGlyphPosition(new_glyphs[i], glyph_positions[i]); } @@ -325,7 +383,8 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // the uploads into the blit pass. // --------------------------------------------------------------------------- if (!UpdateAtlasBitmap(*last_atlas, blit_pass, host_buffer, - last_atlas->GetTexture(), new_glyphs)) { + last_atlas->GetTexture(), new_glyphs, 0, + first_missing_index)) { return nullptr; } @@ -334,28 +393,25 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( return last_atlas; } } + FML_LOG(ERROR) << "about to make new atlas: " + << (new_glyphs.size() - first_missing_index); // A new glyph atlas must be created. Keep the existing width, and double the // new height. + auto height_adjustment = atlas_context->GetAtlasSize().height; ISize atlas_size = ComputeNextAtlasSize( - atlas_context->GetAtlasSize(), + atlas_context, // + new_glyphs, // + glyph_positions, // + first_missing_index, // context.GetResourceAllocator()->GetMaxTextureSizeSupported().height // ); - atlas_context->UpdateGlyphAtlas(last_atlas, atlas_size); + atlas_context->UpdateGlyphAtlas(last_atlas, atlas_size, height_adjustment); if (atlas_size.IsEmpty()) { return nullptr; } - - // Update the rect packer. - if (atlas_context->GetRectPacker()) { - auto new_rect_packer = - atlas_context->GetRectPacker()->Clone(atlas_size.height); - atlas_context->UpdateRectPacker(std::move(new_rect_packer)); - } else { - atlas_context->UpdateRectPacker( - RectanglePacker::Factory(atlas_size.width, atlas_size.height)); - } + FML_CHECK(new_glyphs.size() == glyph_positions.size()); TextureDescriptor descriptor; switch (type) { @@ -398,19 +454,12 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // Now append all remaining glyphs. This should never have any missing data... last_atlas->SetTexture(std::move(new_texture)); - size_t more_missing = AppendToExistingAtlas( - last_atlas, new_glyphs, - glyph_positions, atlas_context->GetAtlasSize(), - atlas_context->GetRectPacker(), first_missing_index); - if (more_missing != new_glyphs.size()) { - return nullptr; - } - // --------------------------------------------------------------------------- // Step 3a: Record the positions in the glyph atlas of the newly added // glyphs. // --------------------------------------------------------------------------- - for (size_t i = first_missing_index, count = glyph_positions.size(); i < count; i++) { + for (size_t i = first_missing_index; i < glyph_positions.size(); i++) { + FML_LOG(ERROR) << glyph_positions[i]; last_atlas->AddTypefaceGlyphPosition(new_glyphs[i], glyph_positions[i]); } @@ -419,7 +468,8 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // the uploads into the blit pass. // --------------------------------------------------------------------------- if (!UpdateAtlasBitmap(*last_atlas, blit_pass, host_buffer, - last_atlas->GetTexture(), new_glyphs)) { + last_atlas->GetTexture(), new_glyphs, + first_missing_index, new_glyphs.size())) { return nullptr; } diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index f14944b2ae254..713435b8428ee 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -6,11 +6,14 @@ #include #include +#include "impeller/typographer/rectangle_packer.h" namespace impeller { GlyphAtlasContext::GlyphAtlasContext(GlyphAtlas::Type type) - : atlas_(std::make_shared(type)), atlas_size_(ISize(0, 0)) {} + : atlas_(std::make_shared(type)), + atlas_size_(ISize(0, 0)), + rect_packer_(RectanglePacker::Factory(4096, 512)) {} GlyphAtlasContext::~GlyphAtlasContext() {} @@ -22,14 +25,20 @@ const ISize& GlyphAtlasContext::GetAtlasSize() const { return atlas_size_; } +int64_t GlyphAtlasContext::GetHeightAdjustment() const { + return height_adjustment_; +} + std::shared_ptr GlyphAtlasContext::GetRectPacker() const { return rect_packer_; } void GlyphAtlasContext::UpdateGlyphAtlas(std::shared_ptr atlas, - ISize size) { + ISize size, + int64_t height_adjustment) { atlas_ = std::move(atlas); atlas_size_ = size; + height_adjustment_ = height_adjustment; } void GlyphAtlasContext::UpdateRectPacker( diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index e758a8f45460a..cf39a9b48738e 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -160,9 +160,11 @@ class GlyphAtlasContext { /// @brief Retrieve the previous (if any) rect packer. std::shared_ptr GetRectPacker() const; + int64_t GetHeightAdjustment() const; + //---------------------------------------------------------------------------- /// @brief Update the context with a newly constructed glyph atlas. - void UpdateGlyphAtlas(std::shared_ptr atlas, ISize size); + void UpdateGlyphAtlas(std::shared_ptr atlas, ISize size, int64_t height_adjustment_); void UpdateRectPacker(std::shared_ptr rect_packer); @@ -170,6 +172,7 @@ class GlyphAtlasContext { std::shared_ptr atlas_; ISize atlas_size_; std::shared_ptr rect_packer_; + int64_t height_adjustment_; GlyphAtlasContext(const GlyphAtlasContext&) = delete; From 40c1457fe46dbf70a4676550d13f3cf6834c9d31 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 May 2024 08:27:30 -0700 Subject: [PATCH 04/10] remove logs. --- .../typographer/backends/skia/typographer_context_skia.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 3f6bbac8911a7..56d0a3dae3a8d 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -392,11 +392,8 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( return last_atlas; } } - FML_LOG(ERROR) << "about to make new atlas: " - << (new_glyphs.size() - first_missing_index); int64_t height_adjustment = atlas_context->GetAtlasSize().height; - const int64_t max_texture_height = context.GetResourceAllocator()->GetMaxTextureSizeSupported().height; @@ -487,7 +484,6 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( context.GetCommandQueue()->Submit({std::move(cmd_buffer)}); }); - FML_LOG(ERROR) << "new atlas: " << new_texture->GetSize(); // Blit the old texture to the top left of the new atlas. if (last_atlas->GetTexture() && blit_old_atlas) { blit_pass->AddCopy(last_atlas->GetTexture(), new_texture, From 81af5a86ef8953f783f5fe4d06c9b3bd43235665 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 May 2024 08:44:46 -0700 Subject: [PATCH 05/10] delete rect packer clone. --- impeller/typographer/rectangle_packer.cc | 13 ----- impeller/typographer/rectangle_packer.h | 13 ----- impeller/typographer/typographer_unittests.cc | 52 ------------------- 3 files changed, 78 deletions(-) diff --git a/impeller/typographer/rectangle_packer.cc b/impeller/typographer/rectangle_packer.cc index 929b57290b48d..3ec7464ff7995 100644 --- a/impeller/typographer/rectangle_packer.cc +++ b/impeller/typographer/rectangle_packer.cc @@ -33,8 +33,6 @@ class SkylineRectanglePacker final : public RectanglePacker { return area_so_far_ / ((float)width() * height()); } - std::unique_ptr Clone(uint32_t height) final; - private: struct SkylineSegment { int x_; @@ -173,17 +171,6 @@ void SkylineRectanglePacker::AddSkylineLevel(size_t skyline_index, } } -std::unique_ptr SkylineRectanglePacker::Clone( - uint32_t height) { - FML_DCHECK(height >= this->height()); - auto packer = std::make_unique(width(), height); - for (SkylineSegment segment : skyline_) { - packer->skyline_.push_back(segment); - } - packer->area_so_far_ = area_so_far_; - return packer; -} - std::shared_ptr RectanglePacker::Factory(int width, int height) { return std::make_shared(width, height); diff --git a/impeller/typographer/rectangle_packer.h b/impeller/typographer/rectangle_packer.h index ba7ff6369f9b3..817bc6605e070 100644 --- a/impeller/typographer/rectangle_packer.h +++ b/impeller/typographer/rectangle_packer.h @@ -51,19 +51,6 @@ class RectanglePacker { /// virtual Scalar PercentFull() const = 0; - //---------------------------------------------------------------------------- - /// @brief Create a new rectangle packer with a larger scaled height - /// scaled and initialize its contents to the current packer. - /// - /// @param[in] scale The new hight for the rect packer. - /// - /// @return A new rectangle packer. - /// - /// This method is used for growing the glyph atlas while keeping - /// existing rects in place. The width of the rectangle packer - /// cannot be increased. - virtual std::unique_ptr Clone(uint32_t height) = 0; - //---------------------------------------------------------------------------- /// @brief Empty out all previously added rectangles. /// diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 1b5003982da00..b8252bc9546c9 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -325,58 +325,6 @@ TEST_P(TypographerTest, RectanglePackerAddsNonoverlapingRectangles) { ASSERT_EQ(packer->PercentFull(), 0); } -TEST(TypographerTest, CanCloneRectanglePackerEmpty) { - auto skyline = RectanglePacker::Factory(256, 256); - - EXPECT_EQ(skyline->PercentFull(), 0); - - auto skyline_2 = skyline->Clone(/*scale=*/2); - - EXPECT_EQ(skyline->PercentFull(), 0); -} - -TEST(TypographerTest, CanCloneRectanglePackerAndPreservePositions) { - auto skyline = RectanglePacker::Factory(256, 256); - IPoint16 loc; - EXPECT_TRUE(skyline->AddRect(100, 100, &loc)); - - EXPECT_EQ(loc.x(), 0); - EXPECT_EQ(loc.y(), 0); - auto percent = skyline->PercentFull(); - - auto skyline_2 = skyline->Clone(/*scale=*/2); - - EXPECT_LT(skyline_2->PercentFull(), percent); -} - -TEST(TypographerTest, CanCloneRectanglePackerWhileFull) { - auto skyline = RectanglePacker::Factory(256, 256); - IPoint16 loc; - // Add a rectangle the size of the entire area. - EXPECT_TRUE(skyline->AddRect(256, 256, &loc)); - // Packer is now full. - EXPECT_FALSE(skyline->AddRect(256, 256, &loc)); - - auto skyline_2 = skyline->Clone(/*scale=*/2); - - // Can now fit one more - EXPECT_TRUE(skyline_2->AddRect(256, 256, &loc)); -} - -TEST(TypographerTest, CloneToSameSizePreservesContents) { - auto skyline = RectanglePacker::Factory(256, 256); - IPoint16 loc; - // Add a rectangle the size of the entire area. - EXPECT_TRUE(skyline->AddRect(256, 256, &loc)); - // Packer is now full. - EXPECT_FALSE(skyline->AddRect(256, 256, &loc)); - - auto skyline_2 = skyline->Clone(/*scale=*/1); - - // Packer is still full. - EXPECT_FALSE(skyline->AddRect(256, 256, &loc)); -} - TEST(TypographerTest, RectanglePackerFillsRows) { auto skyline = RectanglePacker::Factory(257, 256); From fbbe668f18beffd6d178057724941113cc17ef34 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 May 2024 09:54:51 -0700 Subject: [PATCH 06/10] ++ --- impeller/typographer/backends/skia/typographer_context_skia.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 56d0a3dae3a8d..95edcb38a0d01 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -195,7 +195,7 @@ static ISize ComputeNextAtlasSize( // Because we can't grow the skyline packer horizontally, pick a reasonable // large width for all atlases. static constexpr auto kAtlasWidth = 4096u; - static constexpr auto kMinAtlasHeight = 4096u; + static constexpr auto kMinAtlasHeight = 1024; ISize current_size = ISize(kAtlasWidth, kMinAtlasHeight); if (atlas_context->GetAtlasSize().height > current_size.height) { From a569767eb408fc65738630f846d66f795aa8d3c7 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 May 2024 10:34:35 -0700 Subject: [PATCH 07/10] ++ --- .../backends/skia/typographer_context_skia.cc | 71 ++++++++----------- impeller/typographer/glyph_atlas.cc | 5 +- impeller/typographer/glyph_atlas.h | 8 +++ impeller/typographer/typographer_unittests.cc | 47 ++++++++++++ 4 files changed, 84 insertions(+), 47 deletions(-) diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 95edcb38a0d01..427394ae9b985 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -5,6 +5,7 @@ #include "impeller/typographer/backends/skia/typographer_context_skia.h" #include +#include #include #include #include @@ -13,6 +14,7 @@ #include "flutter/fml/trace_event.h" #include "fml/closure.h" +#include "impeller/base/validation.h" #include "impeller/core/allocator.h" #include "impeller/core/buffer_view.h" #include "impeller/core/formats.h" @@ -400,11 +402,15 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // IF the current atlas size is as big as it can get, then "GC" and create an // atlas with only the required glyphs. bool blit_old_atlas = true; + std::shared_ptr new_atlas = last_atlas; if (atlas_context->GetAtlasSize().height >= max_texture_height) { blit_old_atlas = false; first_missing_index = 0; glyph_positions.clear(); height_adjustment = 0; + new_atlas = std::make_shared(type); + atlas_context->UpdateRectPacker(nullptr); + atlas_context->UpdateGlyphAtlas(new_atlas, {0, 0}, 0); } // A new glyph atlas must be created. @@ -415,7 +421,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( max_texture_height // ); - atlas_context->UpdateGlyphAtlas(last_atlas, atlas_size, height_adjustment); + atlas_context->UpdateGlyphAtlas(new_atlas, atlas_size, height_adjustment); if (atlas_size.IsEmpty()) { return nullptr; } @@ -433,11 +439,9 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( } descriptor.size = atlas_size; descriptor.storage_mode = StorageMode::kDevicePrivate; - descriptor.usage = TextureUsage::kShaderRead | TextureUsage::kRenderTarget; + descriptor.usage = TextureUsage::kShaderRead; std::shared_ptr new_texture = context.GetResourceAllocator()->CreateTexture(descriptor); - new_texture->SetLabel("GlyphAtlas"); - if (!new_texture) { return nullptr; } @@ -445,39 +449,20 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( new_texture->SetLabel("GlyphAtlas"); std::shared_ptr cmd_buffer = context.CreateCommandBuffer(); - std::shared_ptr blit_pass; + std::shared_ptr blit_pass = cmd_buffer->CreateBlitPass(); // The R8/A8 textures used for certain glyphs is not supported as color - // attachments in most graphics drivers. To be safe, just do a CPU clear - // for these. - if (type == GlyphAtlas::Type::kAlphaBitmap) { - size_t byte_size = - new_texture->GetTextureDescriptor().GetByteSizeOfBaseMipLevel(); - BufferView buffer_view = - host_buffer.Emplace(nullptr, byte_size, DefaultUniformAlignment()); - - ::memset(buffer_view.buffer->OnGetContents() + buffer_view.range.offset, 0, - byte_size); - buffer_view.buffer->Flush(); - blit_pass = cmd_buffer->CreateBlitPass(); - blit_pass->AddCopy(buffer_view, new_texture); - } else { - // In all other cases, we can use a render pass to clear to a transparent - // color. - ColorAttachment attachment; - attachment.clear_color = Color::BlackTransparent(); - attachment.load_action = LoadAction::kClear; - attachment.store_action = StoreAction::kStore; - attachment.texture = new_texture; - - RenderTarget render_target; - render_target.SetColorAttachment(attachment, 0u); - - auto render_pass = cmd_buffer->CreateRenderPass(render_target); - render_pass->EncodeCommands(); - blit_pass = cmd_buffer->CreateBlitPass(); - } - FML_DCHECK(!!blit_pass); + // attachments in most graphics drivers. For other textures, most framebuffer + // attachments have a much smaller size limit than the max texture size. + size_t byte_size = + new_texture->GetTextureDescriptor().GetByteSizeOfBaseMipLevel(); + BufferView buffer_view = + host_buffer.Emplace(nullptr, byte_size, DefaultUniformAlignment()); + + ::memset(buffer_view.buffer->OnGetContents() + buffer_view.range.offset, 0, + byte_size); + buffer_view.buffer->Flush(); + blit_pass->AddCopy(buffer_view, new_texture); fml::ScopedCleanupClosure closure([&]() { blit_pass->EncodeCommands(context.GetResourceAllocator()); @@ -485,29 +470,29 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( }); // Blit the old texture to the top left of the new atlas. - if (last_atlas->GetTexture() && blit_old_atlas) { - blit_pass->AddCopy(last_atlas->GetTexture(), new_texture, - IRect::MakeSize(last_atlas->GetTexture()->GetSize()), + if (new_atlas->GetTexture() && blit_old_atlas) { + blit_pass->AddCopy(new_atlas->GetTexture(), new_texture, + IRect::MakeSize(new_atlas->GetTexture()->GetSize()), {0, 0}); } // Now append all remaining glyphs. This should never have any missing data... - last_atlas->SetTexture(std::move(new_texture)); + new_atlas->SetTexture(std::move(new_texture)); // --------------------------------------------------------------------------- // Step 3a: Record the positions in the glyph atlas of the newly added // glyphs. // --------------------------------------------------------------------------- for (size_t i = first_missing_index; i < glyph_positions.size(); i++) { - last_atlas->AddTypefaceGlyphPosition(new_glyphs[i], glyph_positions[i]); + new_atlas->AddTypefaceGlyphPosition(new_glyphs[i], glyph_positions[i]); } // --------------------------------------------------------------------------- // Step 4a: Draw new font-glyph pairs into the a host buffer and encode // the uploads into the blit pass. // --------------------------------------------------------------------------- - if (!UpdateAtlasBitmap(*last_atlas, blit_pass, host_buffer, - last_atlas->GetTexture(), new_glyphs, + if (!UpdateAtlasBitmap(*new_atlas, blit_pass, host_buffer, + new_atlas->GetTexture(), new_glyphs, first_missing_index, new_glyphs.size())) { return nullptr; } @@ -515,7 +500,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // Step 8b: Record the texture in the glyph atlas. // --------------------------------------------------------------------------- - return last_atlas; + return new_atlas; } } // namespace impeller diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index 713435b8428ee..a678e77bb23b9 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -6,14 +6,11 @@ #include #include -#include "impeller/typographer/rectangle_packer.h" namespace impeller { GlyphAtlasContext::GlyphAtlasContext(GlyphAtlas::Type type) - : atlas_(std::make_shared(type)), - atlas_size_(ISize(0, 0)), - rect_packer_(RectanglePacker::Factory(4096, 512)) {} + : atlas_(std::make_shared(type)), atlas_size_(ISize(0, 0)) {} GlyphAtlasContext::~GlyphAtlasContext() {} diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index 8163b508086c8..8b5f22c893ab0 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -160,6 +160,14 @@ class GlyphAtlasContext { /// @brief Retrieve the previous (if any) rect packer. std::shared_ptr GetRectPacker() const; + //---------------------------------------------------------------------------- + /// @brief A y-coordinate shift that must be applied to glyphs appended + /// to + /// the atlas. + /// + /// The rectangle packer is only initialized for unfilled regions + /// of the atlas. The area the rectangle packer covers is offset + /// from the origin by this height adjustment. int64_t GetHeightAdjustment() const; //---------------------------------------------------------------------------- diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index b8252bc9546c9..fd4fa861d26f5 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -346,6 +346,53 @@ TEST(TypographerTest, RectanglePackerFillsRows) { EXPECT_EQ(loc.y(), 16); } +TEST_P(TypographerTest, GlyphAtlasTextureWillGrowTilMaxTextureSize) { + auto host_buffer = HostBuffer::Create(GetContext()->GetResourceAllocator()); + auto context = TypographerContextSkia::Make(); + auto atlas_context = + context->CreateGlyphAtlasContext(GlyphAtlas::Type::kAlphaBitmap); + ASSERT_TRUE(context && context->IsValid()); + SkFont sk_font = flutter::testing::CreateTestFontOfSize(12); + auto blob = SkTextBlob::MakeFromString("A", sk_font); + ASSERT_TRUE(blob); + auto atlas = + CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, + GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, + *MakeTextFrameFromTextBlobSkia(blob)); + // Continually append new glyphs until the glyph size grows to the maximum. + // Note that the sizes here are more or less experimentally determined, but + // the important expectation is that the atlas size will shrink again after + // growing to the maximum size. + constexpr ISize expected_sizes[13] = { + {4096, 4096}, // + {4096, 4096}, // + {4096, 8192}, // + {4096, 8192}, // + {4096, 8192}, // + {4096, 8192}, // + {4096, 16384}, // + {4096, 16384}, // + {4096, 16384}, // + {4096, 16384}, // + {4096, 16384}, // + {4096, 16384}, // + {4096, 4096} // Shrinks! + }; + + for (int i = 0; i < 13; i++) { + SkFont sk_font = flutter::testing::CreateTestFontOfSize(50 + i); + auto blob = SkTextBlob::MakeFromString("A", sk_font); + + atlas = + CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, + GlyphAtlas::Type::kAlphaBitmap, 50 + i, atlas_context, + *MakeTextFrameFromTextBlobSkia(blob)); + ASSERT_TRUE(!!atlas); + EXPECT_EQ(atlas->GetTexture()->GetTextureDescriptor().size, + expected_sizes[i]); + } +} + } // namespace testing } // namespace impeller From d34694aa0c0877eb345b9135bfc71bb81f41a212 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 May 2024 13:11:48 -0700 Subject: [PATCH 08/10] disable blit for OpenGLES. --- .../typographer/backends/skia/typographer_context_skia.cc | 8 ++++++-- impeller/typographer/typographer_unittests.cc | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 427394ae9b985..46f784e4ec706 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -400,10 +400,14 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( context.GetResourceAllocator()->GetMaxTextureSizeSupported().height; // IF the current atlas size is as big as it can get, then "GC" and create an - // atlas with only the required glyphs. + // atlas with only the required glyphs. OpenGLES cannot reliably perform the + // blit required here, as 1) it requires attaching textures as read and write + // framebuffers which has substantially smaller size limits that max textures + // and 2) is missing a GLES 2.0 implementation and cap check. bool blit_old_atlas = true; std::shared_ptr new_atlas = last_atlas; - if (atlas_context->GetAtlasSize().height >= max_texture_height) { + if (atlas_context->GetAtlasSize().height >= max_texture_height || + context.GetBackendType() == Context::BackendType::kOpenGLES) { blit_old_atlas = false; first_missing_index = 0; glyph_positions.clear(); diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index fd4fa861d26f5..fb6e6fa654ca5 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -347,6 +347,10 @@ TEST(TypographerTest, RectanglePackerFillsRows) { } TEST_P(TypographerTest, GlyphAtlasTextureWillGrowTilMaxTextureSize) { + if (GetBackend() == PlaygroundBackend::kOpenGLES) { + GTEST_SKIP() << "Atlas growth isn't supported for OpenGLES currently."; + } + auto host_buffer = HostBuffer::Create(GetContext()->GetResourceAllocator()); auto context = TypographerContextSkia::Make(); auto atlas_context = From e31e49558fde1aae73a2b61c4ac7e88b502c3dce Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 15 May 2024 16:11:26 -0700 Subject: [PATCH 09/10] ++ --- .../backends/skia/typographer_context_skia.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 46f784e4ec706..77fded967c9ab 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -14,7 +14,6 @@ #include "flutter/fml/trace_event.h" #include "fml/closure.h" -#include "impeller/base/validation.h" #include "impeller/core/allocator.h" #include "impeller/core/buffer_view.h" #include "impeller/core/formats.h" @@ -214,12 +213,13 @@ static ISize ComputeNextAtlasSize( } else { rect_packer = RectanglePacker::Factory(kAtlasWidth, current_size.height); } - + glyph_positions.erase(glyph_positions.begin() + glyph_index_start, + glyph_positions.end()); atlas_context->UpdateRectPacker(rect_packer); - glyph_index_start = PairsFitInAtlasOfSize( - extra_pairs, current_size, glyph_positions, height_adjustment, - rect_packer, glyph_index_start); - if (glyph_index_start == extra_pairs.size()) { + auto next_index = PairsFitInAtlasOfSize(extra_pairs, current_size, + glyph_positions, height_adjustment, + rect_packer, glyph_index_start); + if (next_index == extra_pairs.size()) { return current_size; } current_size = ISize(current_size.width, current_size.height * 2); From a99269adecf6bc8d4fea26d01a2e46ad08a8dbf4 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 16 May 2024 13:33:36 -0700 Subject: [PATCH 10/10] chinmay review. --- .../backends/skia/typographer_context_skia.cc | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 77fded967c9ab..7593ef09c21ef 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -195,8 +195,8 @@ static ISize ComputeNextAtlasSize( int64_t max_texture_height) { // Because we can't grow the skyline packer horizontally, pick a reasonable // large width for all atlases. - static constexpr auto kAtlasWidth = 4096u; - static constexpr auto kMinAtlasHeight = 1024; + static constexpr int64_t kAtlasWidth = 4096; + static constexpr int64_t kMinAtlasHeight = 1024; ISize current_size = ISize(kAtlasWidth, kMinAtlasHeight); if (atlas_context->GetAtlasSize().height > current_size.height) { @@ -458,15 +458,18 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // The R8/A8 textures used for certain glyphs is not supported as color // attachments in most graphics drivers. For other textures, most framebuffer // attachments have a much smaller size limit than the max texture size. - size_t byte_size = - new_texture->GetTextureDescriptor().GetByteSizeOfBaseMipLevel(); - BufferView buffer_view = - host_buffer.Emplace(nullptr, byte_size, DefaultUniformAlignment()); - - ::memset(buffer_view.buffer->OnGetContents() + buffer_view.range.offset, 0, - byte_size); - buffer_view.buffer->Flush(); - blit_pass->AddCopy(buffer_view, new_texture); + { + TRACE_EVENT0("flutter", "ClearGlyphAtlas"); + size_t byte_size = + new_texture->GetTextureDescriptor().GetByteSizeOfBaseMipLevel(); + BufferView buffer_view = + host_buffer.Emplace(nullptr, byte_size, DefaultUniformAlignment()); + + ::memset(buffer_view.buffer->OnGetContents() + buffer_view.range.offset, 0, + byte_size); + buffer_view.buffer->Flush(); + blit_pass->AddCopy(buffer_view, new_texture); + } fml::ScopedCleanupClosure closure([&]() { blit_pass->EncodeCommands(context.GetResourceAllocator());