++

Author: jonahwilliams

impeller/typographer/backends/skia/typographer_context_skia.cc

@@ -5,6 +5,7 @@
 #include "impeller/typographer/backends/skia/typographer_context_skia.h"
 
 #include <cstddef>
+#include <memory>
 #include <numeric>
 #include <utility>
 #include <vector>
@@ -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<GlyphAtlas> 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<GlyphAtlas> 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<GlyphAtlas>(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<GlyphAtlas> 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,89 +439,68 @@ std::shared_ptr<GlyphAtlas> TypographerContextSkia::CreateGlyphAtlas(
   }
   descriptor.size = atlas_size;
   descriptor.storage_mode = StorageMode::kDevicePrivate;
-  descriptor.usage = TextureUsage::kShaderRead | TextureUsage::kRenderTarget;
+  descriptor.usage = TextureUsage::kShaderRead;
   std::shared_ptr<Texture> new_texture =
       context.GetResourceAllocator()->CreateTexture(descriptor);
-  new_texture->SetLabel("GlyphAtlas");
-
   if (!new_texture) {
     return nullptr;
   }
 
   new_texture->SetLabel("GlyphAtlas");
 
   std::shared_ptr<CommandBuffer> cmd_buffer = context.CreateCommandBuffer();
-  std::shared_ptr<BlitPass> blit_pass;
+  std::shared_ptr<BlitPass> 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());
     context.GetCommandQueue()->Submit({std::move(cmd_buffer)});
   });
 
   // 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;
   }
   // ---------------------------------------------------------------------------
   // Step 8b: Record the texture in the glyph atlas.
   // ---------------------------------------------------------------------------
 
-  return last_atlas;
+  return new_atlas;
 }
 
 }  // namespace impeller

impeller/typographer/glyph_atlas.cc

@@ -6,14 +6,11 @@
 
 #include <numeric>
 #include <utility>
-#include "impeller/typographer/rectangle_packer.h"
 
 namespace impeller {
 
 GlyphAtlasContext::GlyphAtlasContext(GlyphAtlas::Type type)
-    : atlas_(std::make_shared<GlyphAtlas>(type)),
-      atlas_size_(ISize(0, 0)),
-      rect_packer_(RectanglePacker::Factory(4096, 512)) {}
+    : atlas_(std::make_shared<GlyphAtlas>(type)), atlas_size_(ISize(0, 0)) {}
 
 GlyphAtlasContext::~GlyphAtlasContext() {}
 

impeller/typographer/glyph_atlas.h

@@ -160,6 +160,14 @@ class GlyphAtlasContext {
   /// @brief      Retrieve the previous (if any) rect packer.
   std::shared_ptr<RectanglePacker> 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;
 
   //----------------------------------------------------------------------------

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