[web] feature-detect and use ImageDecoder for all image decoding by yjbanov · Pull Request #29419 · flutter/engine

yjbanov · 2021-10-29T23:58:49Z

Feature-detect that the browser supports ImageDecoder.
If it does, use it for image decoding instead of the WASM codecs shipped with CanvasKit.
Bonus: when running felt test --debug, auto-open Chrome DevTools and maximize the window (it is annoying to have to do this manually every time).

The benefit of ImageDecoder is that it decodes asynchronously on a separate thread, so it doesn't block the frames while decoding. Once this API is supported in all browsers, we can remove the WASM codecs and reduce the size of CanvasKit by a few 100s of KB.

Fixes flutter/flutter#63397

dalecurtis · 2021-11-01T17:16:01Z

lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart

+      );
+    }
+
+    if (!await _isImageTypeSupported(contentType)) {


If you don't already have an async step to hide this under, you can just create the image decoder and get the exception there instead of adding any async delay here.

dalecurtis · 2021-11-01T17:16:36Z

lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart

+      // color space, typically SRGB, which is what we want.
+      colorSpaceConversion: 'default',
+
+      // Flutter doesn't give the developer to customize this, so if this is an


"the developer a way..."

dalecurtis · 2021-11-01T17:17:09Z

lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart

+    // wait until the image is fully decoded.
+    // package:js bindings don't work with getters that return a Promise, which
+    // is why js_util is used instead.
+    await js_util.promiseToFuture<void>(js_util.getProperty(webDecoder, 'completed'));


completed is for loading, not decoding. No decoding starts until decode() is called. This will ensure the frame count is accurate though.

dalecurtis · 2021-11-01T17:18:04Z

lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart

+      preferAnimation: true,
+    ));
+
+    await js_util.promiseToFuture<void>(webDecoder.tracks.ready);


This is all that's needed unless you want to ensure frame count is fully known.

For some reason, until this morning (when I was forced to update my Chrome), if I didn't also await ImageDecoder.completed I would get different values for VideoFrame.format. It would flip between BGRX and I420. I assumed there was some race between some internal work that ImageDecoder does and the call to decode() (e.g. progressive loading of images).

With the latest version of Chrome I cannot reproduce this any more, but I'm not sure what behavior to expect. Since I changed the code to load the VideoFrame straight to texImage2D, it probably doesn't matter. We should be able to handle both. The only thing that might be surprising to the user (and perhaps to our goldens test infrastructure) is if the image is sometimes decoded at full resolution and sometimes partially.

Ah, Chrome will opportunistically use YUV decoding, but it depends on if all the frame data is available. So waiting for completed will ensure it's YUV if the image format allows it and Chrome thinks it can decode to it.

In cases where a YUV capable image is decoded to BGRX it's still full resolution, it shouldn't muddle any pixel tests, but I'm not sure off the top of my head.

dalecurtis · 2021-11-01T17:20:04Z

lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart

+    // surface, if necessary, or reuse the existing one.
+    final CkSurface ckSurface = SurfaceFactory.instance.baseSurface.createOrUpdateSurface(ui.window.physicalSize);
+    final SkSurface skSurface = ckSurface.surface;
+    final SkImage? skImage = skSurface.makeImageFromTextureSource(


You can also call copyTo to get the raw pixel bytes. But if your destination is the gpu drawImage/texImage straight from the VideoFrame is definitely better.

Yes, preferably it should go straight to the GPU. However, in a follow-up PR I'll add support for target size, and since not all codecs support resizing, we'll need the pixels in the CPU memory, which is when copyTo will come handy.

dalecurtis · 2021-11-01T17:21:06Z

lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart

+  /// type. It should therefore contain the most popular file formats at the
+  /// top, and less popular towards the bottom.
+  static const List<ImageFileFormat> values = <ImageFileFormat>[
+    // PNG


AVIF is also supported by Chrome if you want.

I do, but I don't know how to detect it :)

It's slightly complicated if you want to do precise detection, but 'ftyp' in the first 16 bytes is good enough to detect an ISO-BMFF image type -- you'll fail later during configure if it's not avif and instead heic or a video.

Complex way: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/image-decoders/avif/avif_image_decoder.cc;l=504;drc=fd8802b593110ea18a97ef044f8a40dd24a622ec

Fantastic! Thanks for the link. I'll add it.

lib/web_ui/lib/src/engine/canvaskit/image.dart

harryterkelsen

LGTM

dalecurtis

lgtm % a couple of nits

dalecurtis · 2021-11-04T22:29:29Z

lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart

@@ -0,0 +1,436 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.


Our project uses 2013 everywhere. Using a future date is problematic, but using an older date is fine.

dalecurtis · 2021-11-04T22:31:02Z

lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart

+      );
+    }
+
+    final _ImageDecoder webDecoder = _ImageDecoder(_ImageDecoderOptions(


I think you want this in the try/catch below.

dalecurtis · 2021-11-04T22:32:41Z

lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart

+      frame.displayWidth,
+      frame.displayHeight,
+    );
+


If you're done with frame at this point it's good practice to call frame.close() to free up memory.

mdebbar

LGTM.

The Linux Fuchsia failure looks like an infra failure, so I triggered a re-run.

EDIT: The re-run I triggered never happened. I tried multiple times but the re-run never runs for some reason. Maybe because I'm not the author of the PR?

yjbanov · 2021-11-11T21:47:39Z

Update: found a bug where the image is lost when moved across GL contexts. Waiting for a CanvasKit-side fix before merging this.

yjbanov · 2021-11-18T00:27:22Z

CanvasKit fix landed, so I will merge this as soon as it goes green.

skia-gold · 2021-11-18T01:00:21Z

Gold has detected about 1 new digest(s) on patchset 1.
View them at https://flutter-engine-gold.skia.org/cl/github/29419

skia-gold · 2021-11-18T01:30:21Z

Gold has detected about 1 new digest(s) on patchset 1.
View them at https://flutter-engine-gold.skia.org/cl/github/29419

yjbanov · 2021-11-18T18:00:51Z

The luci-engine status seems to be incorrect again.

…ing (flutter/engine#29419)

flutter-dashboard bot added the platform-web Code specifically for the web engine label Oct 29, 2021

google-cla bot added the cla: yes label Oct 29, 2021

dalecurtis reviewed Nov 1, 2021

View reviewed changes

mdebbar reviewed Nov 1, 2021

View reviewed changes

lib/web_ui/lib/src/engine/canvaskit/image.dart Show resolved Hide resolved

yjbanov mentioned this pull request Nov 2, 2021

CanvasKit: move image decoding to web workers flutter/flutter#63397

Closed

yjbanov force-pushed the web-codecs branch 3 times, most recently from bdb0a5f to 1c6be9a Compare November 2, 2021 20:03

yjbanov marked this pull request as ready for review November 2, 2021 22:45

yjbanov force-pushed the web-codecs branch 2 times, most recently from b5d373a to 2ebde7c Compare November 3, 2021 21:38

yjbanov requested review from harryterkelsen and mdebbar November 4, 2021 00:50

harryterkelsen approved these changes Nov 4, 2021

View reviewed changes

dalecurtis reviewed Nov 4, 2021

View reviewed changes

yjbanov force-pushed the web-codecs branch from 2ebde7c to 3cd03ca Compare November 4, 2021 23:58

mdebbar approved these changes Nov 5, 2021

View reviewed changes

yjbanov force-pushed the web-codecs branch from 3cd03ca to fdc1b5c Compare November 6, 2021 00:50

CaseyHillers changed the base branch from master to main November 15, 2021 18:13

fluttergithubbot force-pushed the master branch from b1a8771 to 2962099 Compare November 15, 2021 18:30

yjbanov force-pushed the web-codecs branch 4 times, most recently from 6987dd1 to 0875758 Compare November 18, 2021 00:15

yjbanov changed the base branch from master to main November 18, 2021 00:18

yjbanov force-pushed the web-codecs branch from 0875758 to cd3a37e Compare November 18, 2021 00:34

[web] feature-detect and use ImageDecoder for all image decoding

701d358

yjbanov force-pushed the web-codecs branch from cd3a37e to 701d358 Compare November 18, 2021 01:01

yjbanov merged commit 9258e0c into flutter:main Nov 18, 2021

engine-flutter-autoroll mentioned this pull request Nov 19, 2021

Roll Engine from d3dcd41c9490 to 64b13b8eff25 (29 revisions) flutter/flutter#93941

Merged

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

9258e0c [web] feature-detect and use ImageDecoder for all image decod…

d844d7d

…ing (flutter/engine#29419)

zambetpentru mentioned this pull request Jan 28, 2022

Images become corrupted when using CanvasKit flutter/flutter#97103

Closed

		@@ -0,0 +1,436 @@
		// Copyright 2013 The Flutter Authors. All rights reserved.

Conversation

yjbanov commented Oct 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

dalecurtis Nov 1, 2021 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

harryterkelsen left a comment

Choose a reason for hiding this comment

Uh oh!

dalecurtis left a comment

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!

Choose a reason for hiding this comment

Uh oh!

mdebbar left a comment • 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!

yjbanov commented Nov 11, 2021

Uh oh!

yjbanov commented Nov 18, 2021

Uh oh!

skia-gold commented Nov 18, 2021

Uh oh!

skia-gold commented Nov 18, 2021

Uh oh!

yjbanov commented Nov 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yjbanov commented Oct 29, 2021 •

edited

Loading

dalecurtis Nov 1, 2021 •

edited

Loading

mdebbar left a comment •

edited

Loading