[Impeller] treat glyph atlas texture as source of truth, remove copy of SkBitmap. #52567

jonahwilliams · 2024-05-05T03:50:02Z

Work towards part of flutter/flutter#138798

Allow updating single glyphs in the glyph atlas, without replacing the entire bitmap. Required to efficiently append/update to large atlases.

…uth.

jonahwilliams · 2024-05-08T17:16:30Z

impeller/typographer/backends/skia/typographer_context_skia.cc

    const ISize& max_texture_size) {
-  static constexpr auto kMinAtlasSize = 8u;
-  static constexpr auto kMinAlphaBitmapSize = 1024u;
+  static constexpr auto kMinAtlasSize = 4096u;


Because we don't have to upload the whole bitmap for each change, we can afford a larger texture.

backed this out for now because I'd need to rework all of the tests.

jonahwilliams · 2024-05-08T18:30:00Z

impeller/typographer/typographer_unittests.cc

  ASSERT_EQ(old_packer, new_packer);
 }

-TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) {


This test case is deleted because we never need to change the type of an atlas at runtime, we always create distinct alpha and color atlases

jonahwilliams · 2024-05-08T18:30:41Z

lib/gpu/shader_library.cc

 #include "flutter/lib/ui/window/platform_configuration.h"
 #include "fml/mapping.h"
 #include "fml/memory/ref_ptr.h"
+#include "impeller/base/validation.h"


This TU was depending on this include transitively through the atlas code

…xture_sot

jonahwilliams · 2024-05-09T20:08:16Z

Still working on last test failure.

chinmaygarde · 2024-05-09T21:50:56Z

impeller/typographer/backends/skia/typographer_context_skia.cc

+  explicit HostBufferAllocator(HostBuffer& host_buffer)
+      : host_buffer_(host_buffer) {}
+
+  BufferView TakeBufferView() {


[[nodiscard]] here perhaps?

chinmaygarde · 2024-05-09T21:51:17Z

impeller/typographer/backends/skia/typographer_context_skia.cc

+    return std::move(buffer_view_);
+  }
+
+  bool allocPixelRef(SkBitmap* bitmap) {


Is this an override? Maybe add the override and decorate it with the comment for an override.

chinmaygarde · 2024-05-09T21:52:56Z

impeller/typographer/backends/skia/typographer_context_skia.cc

+    BufferView buffer_view = host_buffer_.Emplace(nullptr, required_bytes,
+                                                  DefaultUniformAlignment());
+
+    struct ImpellerPixelRef final : public SkPixelRef {


I am not sure why this subclass is necessary.

Its not! removed

jonahwilliams · 2024-05-10T02:40:18Z

Still has some rendering bugs

jonahwilliams · 2024-05-10T17:16:30Z

testing/dart/assets_test.dart

-    // Therefore each buffer will contain identical contents.
-    expect(ahemImage, equals(bundledFontImage));
-  });
+  // TESTING


flutter-dashboard · 2024-05-10T18:02:25Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #52567 at sha 55a88b0

jonahwilliams · 2024-05-10T18:06:20Z

looks like we're still shifted a bit.

flutter-dashboard · 2024-05-10T19:02:02Z

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #52567 at sha 50b9e44

flutter-dashboard · 2024-05-10T21:08:03Z

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #52567 at sha 7382403

…move copy of SkBitmap. (flutter/engine#52567)

…148156) flutter/engine@ba8e0d3...fad88cb 2024-05-10 [email protected] Roll Skia from f69e8967610e to b6186e1b3995 (2 revisions) (flutter/engine#52726) 2024-05-10 [email protected] Simplify GN pools, use in more places (flutter/engine#52721) 2024-05-10 [email protected] Write `dart:ui` golden-file tests testing `FilterQuality.*` (flutter/engine#52687) 2024-05-10 [email protected] Revert "DisplayListBuilder internal reorganization with better rendering op overlap detection" (flutter/engine#52725) 2024-05-10 [email protected] deletes canvas recorder (flutter/engine#52722) 2024-05-10 [email protected] Roll Skia from d1118d56853c to f69e8967610e (1 revision) (flutter/engine#52723) 2024-05-10 [email protected] [Impeller] treat glyph atlas texture as source of truth, remove copy of SkBitmap. (flutter/engine#52567) 2024-05-10 [email protected] Roll Dart SDK from 01121c008f4d to 13fc7db956c7 (2 revisions) (flutter/engine#52716) 2024-05-10 [email protected] Roll Skia from 11d892ce49b6 to d1118d56853c (1 revision) (flutter/engine#52717) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

jonahwilliams added 2 commits May 4, 2024 16:22

merge in 52555

a89731c

[Impeller] remove host SkBitmap / make atlas texture the source of tr…

44c6607

…uth.

github-actions bot added the e: impeller label May 5, 2024

jonahwilliams changed the title ~~Texture sot~~ [Impeller] treat glyph atlas texture as source of truth, remove copy of SkBitmap. May 5, 2024

jonahwilliams added 3 commits May 5, 2024 10:25

fix goldens.

ee942db

Merge branch 'main' of github.com:flutter/engine into texture_sot

4082ca0

++

b44b36e

jonahwilliams commented May 8, 2024

View reviewed changes

jonahwilliams marked this pull request as ready for review May 8, 2024 17:45

jonahwilliams requested a review from chinmaygarde May 8, 2024 18:00

++

1563f2b

jonahwilliams force-pushed the texture_sot branch from ba7ec3d to 1563f2b Compare May 8, 2024 18:28

jonahwilliams commented May 8, 2024

View reviewed changes

jonahwilliams added 4 commits May 8, 2024 12:38

add missing include.

ff8064e

add missing include.

0d5f6d7

Merge branch 'texture_sot' of github.com:jonahwilliams/engine into te…

7e8cd7d

…xture_sot

revert size changes.

0b69d97

fix round out and clamp instead.

40d43fc

chinmaygarde reviewed May 9, 2024

View reviewed changes

jonahwilliams added 4 commits May 9, 2024 15:23

pixel alignment.

cea4d5d

++

bcde2d9

++

d2cbb95

++

34184f6

chinmaygarde approved these changes May 9, 2024

View reviewed changes

jonahwilliams added 3 commits May 9, 2024 16:33

++

8f358f9

++

57e32fd

++

f1f6950

jonahwilliams added 4 commits May 9, 2024 19:58

++

b1e932e

++

4a0dcf1

Merge branch 'main' of github.com:flutter/engine into texture_sot

9380649

++

b4d2edd

jonahwilliams commented May 10, 2024

View reviewed changes

skip failing tests so I can see goldens.

55a88b0

flutter-dashboard bot added the will affect goldens label May 10, 2024

jonahwilliams added 2 commits May 10, 2024 12:46

divide by scale factor.

dabe7fa

++

63de4db

jonahwilliams force-pushed the texture_sot branch from 50b9e44 to 63de4db Compare May 10, 2024 19:49

re-enable tests.

7382403

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label May 10, 2024

auto-submit bot merged commit 2824230 into flutter:main May 10, 2024

engine-flutter-autoroll mentioned this pull request May 10, 2024

Roll Flutter Engine from ba8e0d3e2f23 to 28242309332e (3 revisions) flutter/flutter#148152

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 10, 2024

282423093 [Impeller] treat glyph atlas texture as source of truth, re…

9d42e1a

…move copy of SkBitmap. (flutter/engine#52567)

engine-flutter-autoroll mentioned this pull request May 10, 2024

Roll Flutter Engine from ba8e0d3e2f23 to cab5a2aab6e4 (5 revisions) flutter/flutter#148154

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 10, 2024

282423093 [Impeller] treat glyph atlas texture as source of truth, re…

c3cdb8d

…move copy of SkBitmap. (flutter/engine#52567)

engine-flutter-autoroll mentioned this pull request May 11, 2024

Roll Flutter Engine from ba8e0d3e2f23 to b94215ce0a1f (7 revisions) flutter/flutter#148155

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 11, 2024

282423093 [Impeller] treat glyph atlas texture as source of truth, re…

ce6f045

…move copy of SkBitmap. (flutter/engine#52567)

engine-flutter-autoroll mentioned this pull request May 11, 2024

Roll Flutter Engine from ba8e0d3e2f23 to fad88cb16d03 (9 revisions) flutter/flutter#148156

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 11, 2024

282423093 [Impeller] treat glyph atlas texture as source of truth, re…

b4d9117

…move copy of SkBitmap. (flutter/engine#52567)

[Impeller] treat glyph atlas texture as source of truth, remove copy of SkBitmap. #52567

[Impeller] treat glyph atlas texture as source of truth, remove copy of SkBitmap. #52567

Uh oh!

Conversation

jonahwilliams commented May 5, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams May 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented May 9, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented May 10, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented May 10, 2024

Uh oh!

jonahwilliams commented May 10, 2024

Uh oh!

flutter-dashboard bot commented May 10, 2024

Uh oh!

flutter-dashboard bot commented May 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonahwilliams May 8, 2024 •

edited

Loading