Auto detect mode to determine which rendering backend to use. #21852

angjieli · 2020-10-14T23:29:39Z

Description

Support auto detect mode to determine which rendering backend to use.

If auto detect is enabled (set by environment variable FLUTTER_WEB_AUTO_DETECT), developers will be allowed to set window.flutterWebRenderer to either canvaskit or html to select the rendering backend.
If window.flutterWebRenderer is not set, flutter engine will use canvaskit for desktop device while using html for mobile device.
If window.flutterWebRenderer is set to an invalid value (not canvaskit nor html), it will default to html.

If auto detect is disabled, it will check the value of environment variable FLUTTER_WEB_USE_SKIA. If true, use canvaksit. Otherwise, use html.

Tests

Updated some test to reflect this change.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the contributor guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the C++, Objective-C, Java style guides for the engine.
I read the tree hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation.
All existing and new tests are passing.
I am willing to follow-up on review comments in a timely manner.

Reviewer Checklist

I have submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: https://flutter.dev/go/breaking-changes-template Replace this with a link to a pull request that adds the migration guide to https://flutter.dev/docs/release/breaking-changes

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

yjbanov · 2020-10-16T23:33:52Z

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

-/// EXPERIMENTAL: Enable the Skia-based rendering backend.
-const bool experimentalUseSkia =
+@JS('window.flutterWebRenderer')
+external String? get customRenderer;


nit: "custom renderer" sounds like this is the renderer itself. I'd call this requestedRendererType.

Also, let's leave dartdocs explaining what it's for.

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

yjbanov · 2020-10-16T23:36:38Z

Added @jonahwilliams to reviewer list to have a look at the gn changes.

jonahwilliams

gn changes LGTM

nturgut

The failing test on Cirrus is fixed yesterday, we should sync to head though.

nturgut · 2020-10-20T17:47:46Z

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

+    _autoDetect ? _detectRenderer() : _useSkia;
+
+/// Returns true if CanvasKit is used.
+/// Otherwise, returns false.


very nit: I believe there is one empty line between two sentences in doc comments. https://dart.dev/guides/language/effective-dart/documentation#do-separate-the-first-sentence-of-a-doc-comment-into-its-own-paragraph

nturgut · 2020-10-20T17:54:26Z

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

+  if (requestedRendererType != null) {
+    return requestedRendererType! == 'canvaskit';
+  }
+  // If requestedRendererType is not specified, use CanvasKit for desktop and


@yjbanov @ferhatb if I'm not mistaken we are setting Canvaskit as default for desktop. I am personally not sure if we are ready for this step as testing goes:

Majority of our unit tests are build with canvaskit.

We have coverage in many critical areas with integration tests on Desktop browsers (text editing, font loading, keyboard, platform selection, image loading, semantics (in draft), scroll (in pr), pointer events (in pr), tree shaking, platform messages). None of them runs with Canvaskit though.

Shall we prioritize running them with Canvaskit then?

Good point. We should definitely ramp up testing in CanvasKit mode. This PR is not changing the default yet (--web-renderer=auto will default to CanvasKit on desktop, but --web-renderer itself defaults to html so overall the defaults are not changing), so tests can proceed on their own schedule.

In terms of test prioritization, testing things in CanvasKit mode where the choice of renderer has known effects is a good place to start from. From your list the affected areas are: font loading, image loading. Text editing is also affected but I'm not sure our existing tests will catch the distinction, so we may need new tests. Tree shaking is affected; we use devicelab for that, so it would be best to reuse the existing tests. Other things affected by renderer choice: rendering quality, platform views. There's also performance, but our benchmarks already cover both HTML and CanvasKit.

@angjieli tl;dr there's no action items for this PR w.r.t. running existing tests in CanvasKit mode

nturgut · 2020-10-20T17:55:29Z

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

+///
+/// Using flutter tools option "--web-render=cavanskit" would set the value to
+/// true.
+/// Using flutter tools option "--web-render=html" would set the value to false.


very nit: typo for canvaskit

nturgut · 2020-10-20T18:02:14Z

@ferhatb @yjbanov I have an idea for increasing test coverage. When we are running framework tests with local engine, we can run them both with canvaskit and html backend. I'll do that by setting FLUTTER_WEB_AUTO_DETECT in a new luci builder. It will be a very easy change in flutter/infra code. Please let me know if this sound good to you?

yjbanov

yjbanov · 2020-10-22T20:17:48Z

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

+  if (requestedRendererType != null) {
+    return requestedRendererType! == 'canvaskit';
+  }
+  // If requestedRendererType is not specified, use CanvasKit for desktop and


@angjieli tl;dr there's no action items for this PR w.r.t. running existing tests in CanvasKit mode

…flutter/engine#21852)

* 000bf4b Roll Skia from 2d2f82c00aeb to 5c7bb326a7b3 (33 revisions) (flutter/engine#22059) * ae92dbf Roll Fuchsia Linux SDK from lPMs_KwnU... to gqS_DIjN4... (flutter/engine#22057) * 92cd74e Roll Fuchsia Mac SDK from pZ9FgVZTK... to WLxBkBnZa... (flutter/engine#22055) * e51c710 Roll Dart SDK from a3d902d8598e to 9f907e198970 (2 revisions) (flutter/engine#22058) * 326b202 Reland fuchsia external view embedder will be shared with platform view (flutter/engine#22008) * a9a9a2f Roll Skia from 5c7bb326a7b3 to 65674e4c2e56 (3 revisions) (flutter/engine#22060) * 1233fe4 Revert "Revert "Explicitly make the X connection for EGL. (#21831)" (#21851)" (flutter/engine#21871) * aed8e01 Fixes Edge trigger route change announcement (flutter/engine#21975) * 6bc70e4 Reland: Migration to PlatformDispatcher and multi-window (flutter/engine#21932) * 5ca5e26 Add FlEventChannel (flutter/engine#21316) * 77b0052 Roll Skia from 65674e4c2e56 to 01b05e5b830b (3 revisions) (flutter/engine#22062) * 3d27fd5 Support loading assets from Android dynamic feature modules (flutter/engine#21504) * 742dfbe support uri intent launcher in android (flutter/engine#21275) * cde1e3f Auto detect mode to determine which rendering backend to use. (flutter/engine#21852) * 329ccf7 Roll Skia from 01b05e5b830b to 53281c712159 (1 revision) (flutter/engine#22065) * cde78c1 Add a golden scenario test for fallback font rendering on iOS take 2 (flutter/engine#22033) * 4f4599b Roll Dart SDK from 9f907e198970 to 37ccceacad41 (3 revisions) (flutter/engine#22069) * f0b10c5 [web] Prevent using DOM nodes for canvas with large number of draws (flutter/engine#22064) * a86ba57 Roll Fuchsia Mac SDK from WLxBkBnZa... to zDfaxkqlv... (flutter/engine#22073) * 645198a Roll Fuchsia Linux SDK from gqS_DIjN4... to vuKxZmSVj... (flutter/engine#22074) * 0b26570 Revert dart rolls (flutter/engine#22078)

happyharis · 2020-10-31T05:30:01Z

Is there a way to check whether the HTML is being rendered only, or canvas kit is being rendered only?

angjieli · 2020-11-05T17:59:11Z

Is there a way to check whether the HTML is being rendered only, or canvas kit is being rendered only?

IIRC, you should be able to tell the difference by examining the compiled javascript code. @yjbanov feel free to correct me if I am wrong

…r#21852)

slavap · 2021-01-05T02:00:29Z

@yjbanov @angjieli
How can I get access to engine.useCanvasKit from regular Flutter application?
Right now I cannot figure out from the code which renderer is used to adjust my code accordingly, so I have to explicitly specify --web-renderer html

angjieli · 2021-01-05T21:54:55Z

@yjbanov @angjieli
How can I get access to engine.useCanvasKit from regular Flutter application?
Right now I cannot figure out from the code which renderer is used to adjust my code accordingly, so I have to explicitly specify --web-renderer html

Once you have launched the application in auto mode, you can enter window.flutterWebRenderer=html in your browser's console before the application is launched. By doing so, your application will be rendered using html.

slavap · 2021-01-05T22:46:41Z

@angjieli No, my question was - how to detect which mode was set by auto setting, I don't want to explicitly force anything.

angjieli · 2021-01-05T22:51:05Z

@angjieli No, my question was - how to detect which mode was set by auto setting, I don't want to explicitly force anything.

Oh, misread the question. There is no good way to check the rendering backend at this point. IIRC, the info is not exposed to flutter application.

slavap · 2021-01-05T23:20:41Z

@angjieli
That's bad, it has to be exposed to Flutter app, preferable as constant. We have kIsWeb, probably adding kIsWebHtml and kIsWebCanvasKit will cover all cases.

angjieli · 2021-01-05T23:22:11Z

@angjieli
That's bad, it has to be exposed to Flutter app, preferable as constant. We have kIsWeb, probably adding kIsWebHtml and kIsWebCanvasKit will cover all cases.

Yeah. Feel free to file an issue and we can fix it.

Auto detect mode to determine which rendering backend to use.

8dbef3c

googlebot added the cla: yes label Oct 14, 2020

Null-safe fix.

f5340b8

angjieli requested a review from yjbanov October 15, 2020 15:41

chinmaygarde added the platform-web Code specifically for the web engine label Oct 15, 2020

yjbanov reviewed Oct 16, 2020

View reviewed changes

yjbanov requested a review from jonahwilliams October 16, 2020 23:36

angjieli added 2 commits October 16, 2020 17:27

Add dartdoc.

013bca2

Remove unnecessary trailing space.

baf3b71

jonahwilliams approved these changes Oct 19, 2020

View reviewed changes

format BUILD.gn.

baf0432

nturgut approved these changes Oct 20, 2020

View reviewed changes

Merge branch 'master' into webrenderer

bbe1bb4

Fix typo and doc issue.

f8bb10d

angjieli requested a review from yjbanov October 22, 2020 15:46

yjbanov approved these changes Oct 22, 2020

View reviewed changes

angjieli and others added 2 commits October 22, 2020 14:57

Merge branch 'master' into webrenderer

85aad6f

Fix platform_dispatcher.

7b34923

angjieli added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Oct 22, 2020

fluttergithubbot merged commit cde1e3f into flutter:master Oct 22, 2020

angjieli mentioned this pull request Oct 23, 2020

Support --web-renderer options which would allow user to specify which rendering backend to use. flutter/flutter#68848

Merged

13 tasks

engine-flutter-autoroll mentioned this pull request Oct 23, 2020

Roll Engine from defa8be2b106 to cde1e3fca8dc (14 revisions) flutter/flutter#68849

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 23, 2020

cde1e3f Auto detect mode to determine which rendering backend to use. (…

7149b5b

…flutter/engine#21852)

engine-flutter-autoroll mentioned this pull request Oct 23, 2020

Roll Engine from defa8be2b106 to 4f4599b269c5 (17 revisions) flutter/flutter#68852

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 23, 2020

cde1e3f Auto detect mode to determine which rendering backend to use. (…

7b43c23

…flutter/engine#21852)

engine-flutter-autoroll mentioned this pull request Oct 23, 2020

Roll Engine from defa8be2b106 to f0b10c54bf68 (18 revisions) flutter/flutter#68859

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 23, 2020

cde1e3f Auto detect mode to determine which rendering backend to use. (…

fa18184

…flutter/engine#21852)

engine-flutter-autoroll mentioned this pull request Oct 23, 2020

Roll Engine from defa8be2b106 to 645198aa72c1 (20 revisions) flutter/flutter#68863

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 23, 2020

cde1e3f Auto detect mode to determine which rendering backend to use. (…

e7faa28

…flutter/engine#21852)

engine-flutter-autoroll mentioned this pull request Oct 23, 2020

Roll Engine from defa8be2b106 to 0b26570e933f (21 revisions) flutter/flutter#68897

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 23, 2020

cde1e3f Auto detect mode to determine which rendering backend to use. (…

12de484

…flutter/engine#21852)

illia-romanenko mentioned this pull request Nov 4, 2020

Skia deployment - enable auto renderer Flank/flank-dashboard#773

Closed

3 tasks

chaselatta pushed a commit to chaselatta/engine that referenced this pull request Nov 30, 2020

Auto detect mode to determine which rendering backend to use. (flutte…

6443af9

…r#21852)

creativecreatorormaybenot mentioned this pull request Dec 18, 2020

[web] Why does FLUTTER_WEB_AUTO_DETECT not use CanvasKit on mobile? flutter/flutter#72597

Closed

muhleder mentioned this pull request Dec 26, 2020

When I use this command, Image not showing peiffer-innovations/websafe_svg#15

Closed

slavap mentioned this pull request Jan 5, 2021

[web] Need kIsWebHtml & kIsWebCanvasKit, need ability to find out current renderer from app code set by "Auto detect mode". flutter/flutter#73369

Closed

deakjahn mentioned this pull request Jan 12, 2021

Not working with web-renderer=canvaskit deakjahn/flutter_dropzone#20

Closed

illia-romanenko mentioned this pull request Jan 18, 2021

Build POC of changing renderer using Debug Menu Flank/flank-dashboard#987

Closed

1 task

Auto detect mode to determine which rendering backend to use. #21852

Auto detect mode to determine which rendering backend to use. #21852

Uh oh!

Conversation

angjieli commented Oct 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tests

Checklist

Reviewer Checklist

Breaking Change

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

yjbanov commented Oct 16, 2020

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

nturgut 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!

nturgut commented Oct 20, 2020

Uh oh!

yjbanov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

happyharis commented Oct 31, 2020

Uh oh!

angjieli commented Nov 5, 2020

Uh oh!

slavap commented Jan 5, 2021

Uh oh!

angjieli commented Jan 5, 2021

Uh oh!

slavap commented Jan 5, 2021

Uh oh!

angjieli commented Jan 5, 2021

Uh oh!

slavap commented Jan 5, 2021

Uh oh!

angjieli commented Jan 5, 2021

Uh oh!

Uh oh!

angjieli commented Oct 14, 2020 •

edited

Loading