Add image generator registry #25987

bdero · 2021-05-06T20:30:49Z

This CL has the first part of a refactor for #17356 , which adds an engine registry for installing new ImageGenerators. In order to allow for optional support for subpixel image decoding, a Flutter-specific abstraction is needed (as SkImageGenerator does not offer an interface that can support this (whereas SkCodecImageGenerator does -- but platform-specific codecs and embedder-installed codecs won't require wrapping SkCodecs).

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.
The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

MrBirb · 2021-05-06T21:50:34Z

I suppose you meant this #17356

bdero · 2021-05-06T23:08:55Z

@MrBirb Yes indeed, thanks!

bdero · 2021-05-10T18:18:22Z

@chinmaygarde The build is passing other than tree, so I think this is ready for a first pass.

chinmaygarde

This is great. I could find no major issues. Just minor nits.

lib/ui/painting/image_generator.h

lib/ui/painting/image_generator_registry.h

chinmaygarde · 2021-05-10T19:00:16Z

lib/ui/painting/image_generator_registry.h

+
+/// Keeps a priority-ordered registry of image generator builders to be used
+/// when decoding images. This object must be created, accessed, and collected
+/// on the UI thread (typically the engine or its runtime controller).


This bit of clarification on threading is great. Thanks.

lib/ui/painting/image_generator_registry.h

lib/ui/painting/image_generator_registry_unittests.cc

lib/ui/painting/image_generator.h

lib/ui/painting/image_generator_registry.cc

dnfield · 2021-05-12T03:26:58Z

runtime/dart_isolate.h

@@ -176,6 +176,12 @@ class DartIsolate : public UIDartState {
  /// @param[in]  io_manager                  The i/o manager.
  /// @param[in]  unref_queue                 The Skia unref queue.
  /// @param[in]  image_decoder               The image decoder.
+  /// @param[in]  image_generator_registry    Cascading registry of image


Every time we add a new parameter to this my soul dies a little bit.

If you have any interest, it would be nice to resolve flutter/flutter#71620 so we don't have to keep updating the 10 or however many files this ends up touching every time we do this :)

Oh yeah, I agree this would be nice to make passing around all this shared state simpler. I'd prefer to do it in a simpler follow-up PR if you don't mind.

Yeah, definitely a separate PR

dnfield

Overall seems to make sense, thanks for getting this moving!

bdero · 2021-05-17T18:45:00Z

This is ready for another review pass. (cc @chinmaygarde & @dnfield)

* 98068f1 fuchsia: Only send different ViewProperties (flutter/engine#26170) * 5e487ba Roll Skia from c1f641104531 to 469531234936 (4 revisions) (flutter/engine#26197) * 4cab492 Add image generator registry (flutter/engine#25987) * c753c24 Roll Fuchsia Linux SDK from l6XmTSLnt... to AUWVgDkx6... (flutter/engine#26201) * 75a7812 Reland outputs a json version of the package manifest. (flutter/engine#26163) * b5dc598 Roll Skia from 469531234936 to 5b38536d76ae (7 revisions) (flutter/engine#26202) * fb91366 [icu] Upgrade ICU to 69.1, the same commit used by Chromimum latest (flutter/engine#26200) * 62e0951 Prepare to move --delete-tostring-package-uri option to Dart SDK (flutter/engine#26196) * 156c2be Web ImageFilter.matrix support (flutter/engine#25982) * c30de25 Roll Skia from 5b38536d76ae to 2fed9f62d29a (7 revisions) (flutter/engine#26206) * 7ef42f1 Roll Fuchsia Mac SDK from KmSY84b_E... to 7WNQRHsHN... (flutter/engine#26207) * 8bba964 Add more diagrams for deferred components docs, fix alignment of existing one (flutter/engine#26209) * 391e22d Roll Dart SDK from 67be110b5ba8 to 510f26486328 (3 revisions) (flutter/engine#26212) * fa62ce0 Roll Skia from 2fed9f62d29a to e8cd0a54041e (2 revisions) (flutter/engine#26213) * 2029cba Roll Skia from e8cd0a54041e to 3d854bade6de (4 revisions) (flutter/engine#26216) * 9cd3c64 Roll Fuchsia Linux SDK from AUWVgDkx6... to boele8geO... (flutter/engine#26217) * 6ceab15 Roll Dart SDK from 510f26486328 to 13e329e614f2 (1 revision) (flutter/engine#26218) * f90698a Roll Fuchsia Mac SDK from 7WNQRHsHN... to F5TX4MaEg... (flutter/engine#26220) * b9d03fe Roll Skia from 3d854bade6de to 66125eac158d (1 revision) (flutter/engine#26221) * 849ce7e Roll Dart SDK from 13e329e614f2 to 2eea032403e2 (1 revision) (flutter/engine#26222) * ca89d69 Roll Skia from 66125eac158d to 5696bcd0f7f8 (1 revision) (flutter/engine#26223) * db7a3dc fuchsia: Fix occlusion_hint handling (flutter/engine#26208) * b875d99 Roll Skia from 5696bcd0f7f8 to bdfd77616177 (5 revisions) (flutter/engine#26224) * b67caca Roll Skia from bdfd77616177 to fb8d20befa8f (1 revision) (flutter/engine#26225) * 2398dea Revert Dart SDK to 67be110b5ba8fc84af5e7136389e52a13ccbc9d5 (flutter/engine#26232)

bdero requested a review from chinmaygarde May 6, 2021 20:30

google-cla bot added the cla: yes label May 6, 2021

bdero force-pushed the bdero/codec-registry branch 2 times, most recently from 2aeb496 to b466828 Compare May 6, 2021 21:12

bdero force-pushed the bdero/codec-registry branch from b466828 to 6e88015 Compare May 7, 2021 21:21

chinmaygarde reviewed May 10, 2021

View reviewed changes

bdero force-pushed the bdero/codec-registry branch from 6e88015 to e56ad99 Compare May 11, 2021 22:23

dnfield reviewed May 12, 2021

View reviewed changes

lib/ui/painting/image_generator.h Show resolved Hide resolved

dnfield reviewed May 12, 2021

View reviewed changes

lib/ui/painting/image_generator_registry.cc Outdated Show resolved Hide resolved

dnfield reviewed May 12, 2021

View reviewed changes

bdero force-pushed the bdero/codec-registry branch from 042732b to 5d648fc Compare May 14, 2021 22:21

bdero mentioned this pull request May 15, 2021

Register platform-specific decoders from from their respective platform embedders flutter/flutter#82603

Open

bdero and others added 9 commits May 17, 2021 10:41

Add image generator registry and lookup with priority

cf5f280

Minor review fixes

f1574ef

Add docstrings

802a558

Add more docstrings

5b741f8

Add extra var for sorting when priorities are equal

2ab297a

Use ordered set

b89fbe3

Add support for multiframe codecs directly in ImageGenerator

6f29553

Add todo for installing image decoder factories though embedders

237fcd5

Use unsigned int instead of uint

867a570

bdero force-pushed the bdero/codec-registry branch from 469453e to 8667f36 Compare May 17, 2021 17:43

Fix name in windows generator factory

3063a98

bdero force-pushed the bdero/codec-registry branch from 8667f36 to 3063a98 Compare May 17, 2021 17:55

chinmaygarde approved these changes May 17, 2021

View reviewed changes

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

4cab492 Add image generator registry (flutter/engine#25987)

88901e3

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to 8bba964f9247 (12 revisions) flutter/flutter#82775

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

610b5c6

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to 391e22dcb8a1 (13 revisions) flutter/flutter#82778

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

16c110f

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to fa62ce08b9b3 (14 revisions) flutter/flutter#82782

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

9be605a

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to 2029cbafc01c (15 revisions) flutter/flutter#82797

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

61aa0ea

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to 9cd3c64914a3 (16 revisions) flutter/flutter#82805

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

e3c22ad

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to 6ceab157df12 (17 revisions) flutter/flutter#82806

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

e5aa7d0

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to f90698a89695 (18 revisions) flutter/flutter#82813

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

1757012

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to b9d03fed2d09 (19 revisions) flutter/flutter#82819

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

7d4bd9f

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to ca89d69130b6 (21 revisions) flutter/flutter#82830

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

67b02e5

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to db7a3dc09157 (22 revisions) flutter/flutter#82837

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

3a3da2d

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to b875d99dcf46 (23 revisions) flutter/flutter#82842

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

361c699

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to b67caca64330 (24 revisions) flutter/flutter#82848

Closed

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

4cab492 Add image generator registry (flutter/engine#25987)

5fc2ca2

engine-flutter-autoroll mentioned this pull request May 18, 2021

Roll Engine from 3cda7f5807db to 2398dea918a6 (25 revisions) flutter/flutter#82858

Merged

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

4cab492 Add image generator registry (flutter/engine#25987)

3739f18

bdero mentioned this pull request Nov 1, 2021

Fix bug where ImageGenerator factories with the same priority get dropped in release mode #29254

Merged

PeterHatch mentioned this pull request Feb 2, 2023

Add API for adding image decoders flutter/flutter#119845

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add image generator registry #25987

Add image generator registry #25987

bdero commented May 6, 2021 •

edited

Loading

MrBirb commented May 6, 2021

bdero commented May 6, 2021

bdero commented May 10, 2021

chinmaygarde left a comment

chinmaygarde May 10, 2021

dnfield May 12, 2021

bdero May 17, 2021

dnfield May 17, 2021

dnfield left a comment

bdero commented May 17, 2021

Add image generator registry #25987

Add image generator registry #25987

Conversation

bdero commented May 6, 2021 • edited Loading

Pre-launch Checklist

MrBirb commented May 6, 2021

bdero commented May 6, 2021

bdero commented May 10, 2021

chinmaygarde left a comment

Choose a reason for hiding this comment

chinmaygarde May 10, 2021

Choose a reason for hiding this comment

dnfield May 12, 2021

Choose a reason for hiding this comment

bdero May 17, 2021

Choose a reason for hiding this comment

dnfield May 17, 2021

Choose a reason for hiding this comment

dnfield left a comment

Choose a reason for hiding this comment

bdero commented May 17, 2021

bdero commented May 6, 2021 •

edited

Loading