From 2944a0213c1868b0583141e9bc9531da05f24414 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Wed, 8 Mar 2023 17:11:03 -0500 Subject: [PATCH 1/4] [web] Better way to detect CanvasKit variant --- .../src/engine/canvaskit/canvaskit_api.dart | 82 ++++++------------- lib/web_ui/lib/src/engine/canvaskit/text.dart | 2 +- .../src/engine/canvaskit/text_fragmenter.dart | 3 +- .../test/canvaskit/canvaskit_api_test.dart | 4 +- 4 files changed, 30 insertions(+), 61 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart index d588da5a54c39..c632ae75bd892 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart @@ -28,22 +28,6 @@ import 'renderer.dart'; /// Entrypoint into the CanvasKit API. late CanvasKit canvasKit; -late CanvasKitVariant _canvasKitVariant; - -/// Which variant of CanvasKit we are using. -CanvasKitVariant get canvasKitVariant => _canvasKitVariant; -set canvasKitVariant(CanvasKitVariant value) { - if (value == CanvasKitVariant.auto) { - throw ArgumentError.value( - value, - 'value', - 'CanvasKitVariant.auto is not a valid value for canvasKitVariant', - ); - } - _canvasKitVariant = value; -} - - /// Sets the [CanvasKit] object on `window` so we can use `@JS()` to bind to /// static APIs. /// @@ -1884,6 +1868,7 @@ extension SkParagraphBuilderNamespaceExtension on SkParagraphBuilderNamespace { SkParagraphStyle paragraphStyle, TypefaceFontProvider? fontManager, ); + external bool RequiresClientICU(); } @JS() @@ -2699,47 +2684,26 @@ void patchCanvasKitModule(DomHTMLScriptElement canvasKitScript) { } } -String get _canvasKitBaseUrl => configuration.canvasKitBaseUrl; - const String _kFullCanvasKitJsFileName = 'canvaskit.js'; const String _kChromiumCanvasKitJsFileName = 'chromium/canvaskit.js'; -// TODO(mdebbar): Replace this with a Record once it's supported in Dart. -class _CanvasKitVariantUrl { - const _CanvasKitVariantUrl(this.url, this.variant) - : assert( - variant != CanvasKitVariant.auto, - 'CanvasKitVariant.auto cannot have a url', - ); - - final String url; - final CanvasKitVariant variant; - - static _CanvasKitVariantUrl chromium = _CanvasKitVariantUrl( - '$_canvasKitBaseUrl$_kChromiumCanvasKitJsFileName', - CanvasKitVariant.chromium, - ); - - static _CanvasKitVariantUrl full = _CanvasKitVariantUrl( - '$_canvasKitBaseUrl$_kFullCanvasKitJsFileName', - CanvasKitVariant.full, - ); -} - -List<_CanvasKitVariantUrl> get _canvasKitUrls { +String get _canvasKitBaseUrl => configuration.canvasKitBaseUrl; +List get _canvasKitJsFileNames { switch (configuration.canvasKitVariant) { case CanvasKitVariant.auto: - return <_CanvasKitVariantUrl>[ - if (browserSupportsCanvaskitChromium) _CanvasKitVariantUrl.chromium, - _CanvasKitVariantUrl.full, + return [ + if (browserSupportsCanvaskitChromium) _kChromiumCanvasKitJsFileName, + _kFullCanvasKitJsFileName, ]; case CanvasKitVariant.full: - return <_CanvasKitVariantUrl>[_CanvasKitVariantUrl.full]; + return [_kFullCanvasKitJsFileName]; case CanvasKitVariant.chromium: - return <_CanvasKitVariantUrl>[_CanvasKitVariantUrl.chromium]; + return [_kChromiumCanvasKitJsFileName]; } } - +Iterable get _canvasKitJsUrls { + return _canvasKitJsFileNames.map((String filename) => '$_canvasKitBaseUrl$filename'); +} @visibleForTesting String canvasKitWasmModuleUrl(String file, String canvasKitBase) => canvasKitBase + file; @@ -2749,23 +2713,29 @@ String canvasKitWasmModuleUrl(String file, String canvasKitBase) => /// Downloads the CanvasKit JavaScript, then calls `CanvasKitInit` to download /// and intialize the CanvasKit wasm. Future downloadCanvasKit() async { - await _downloadOneOf(_canvasKitUrls); + await _downloadOneOf(_canvasKitJsUrls); - return CanvasKitInit(CanvasKitInitOptions( + final CanvasKit canvasKit = await CanvasKitInit(CanvasKitInitOptions( locateFile: allowInterop(canvasKitWasmModuleUrl), )); + + if (canvasKit.ParagraphBuilder.RequiresClientICU() && !browserSupportsCanvaskitChromium) { + throw Exception( + 'The CanvasKit variant you are using only works on Chromium browsers. ' + 'Please use a different CanvasKit variant, or use a Chromium browser.', + ); + } + + return canvasKit; } -/// Finds the first entry in [urls] that can be downloaded successfully, and +/// Finds the first URL in [urls] that can be downloaded successfully, and /// downloads it. /// /// If none of the URLs can be downloaded, throws an [Exception]. -/// -/// Also sets [canvasKitVariant] to the variant of CanvasKit that was downloaded. -Future _downloadOneOf(Iterable<_CanvasKitVariantUrl> urls) async { - for (final _CanvasKitVariantUrl entry in urls) { - if (await _downloadCanvasKitJs(entry.url)) { - canvasKitVariant = entry.variant; +Future _downloadOneOf(Iterable urls) async { + for (final String url in urls) { + if (await _downloadCanvasKitJs(url)) { return; } } diff --git a/lib/web_ui/lib/src/engine/canvaskit/text.dart b/lib/web_ui/lib/src/engine/canvaskit/text.dart index dfd272ca3bf4d..e885bd7da4fd5 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/text.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/text.dart @@ -984,7 +984,7 @@ class CkParagraphBuilder implements ui.ParagraphBuilder { /// Builds the CkParagraph with the builder and deletes the builder. SkParagraph _buildSkParagraph() { - if (canvasKitVariant == CanvasKitVariant.chromium) { + if (canvasKit.ParagraphBuilder.RequiresClientICU()) { injectClientICU(_paragraphBuilder); } final SkParagraph result = _paragraphBuilder.build(); diff --git a/lib/web_ui/lib/src/engine/canvaskit/text_fragmenter.dart b/lib/web_ui/lib/src/engine/canvaskit/text_fragmenter.dart index ef55f170a212c..3acc029aba3d5 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/text_fragmenter.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/text_fragmenter.dart @@ -7,7 +7,6 @@ import 'dart:typed_data'; import '../dom.dart'; import '../text/line_breaker.dart'; import 'canvaskit_api.dart'; -import 'renderer.dart'; /// Injects required ICU data into the [builder]. /// @@ -15,7 +14,7 @@ import 'renderer.dart'; /// without ICU data. void injectClientICU(SkParagraphBuilder builder) { assert( - canvasKitVariant == CanvasKitVariant.chromium, + canvasKit.ParagraphBuilder.RequiresClientICU(), 'This method should only be used with the CanvasKit Chromium variant.', ); diff --git a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart index 82602bd87c075..39ef01da27299 100644 --- a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart +++ b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart @@ -1624,7 +1624,7 @@ void _paragraphTests() { builder.pushStyle( canvasKit.TextStyle(SkTextStyleProperties()..halfLeading = true)); builder.pop(); - if (canvasKitVariant == CanvasKitVariant.chromium) { + if (canvasKit.ParagraphBuilder.RequiresClientICU()) { injectClientICU(builder); } final SkParagraph paragraph = builder.build(); @@ -1742,7 +1742,7 @@ void _paragraphTests() { ); builder.addText('hello'); - if (canvasKitVariant == CanvasKitVariant.chromium) { + if (canvasKit.ParagraphBuilder.RequiresClientICU()) { injectClientICU(builder); } From 995ef10c7699e35fd313b431ecd6e5f700ef855c Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Thu, 9 Mar 2023 14:50:32 -0500 Subject: [PATCH 2/4] compatibility with old CanvasKit --- lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart index c632ae75bd892..644ce9a79c10f 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart @@ -1868,7 +1868,13 @@ extension SkParagraphBuilderNamespaceExtension on SkParagraphBuilderNamespace { SkParagraphStyle paragraphStyle, TypefaceFontProvider? fontManager, ); - external bool RequiresClientICU(); + + bool RequiresClientICU() { + if (!js_util.hasProperty(this, 'RequiresClientICU')) { + return false; + } + return js_util.callMethod(this, 'RequiresClientICU', const [],) as bool; + } } @JS() From 3fb078f96b709c8ef50decdcd2aa0b1cdec8c65e Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Thu, 9 Mar 2023 16:05:44 -0500 Subject: [PATCH 3/4] fix @eyebrowsoffire's tests --- lib/web_ui/lib/src/engine/browser_detection.dart | 4 +--- lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart | 6 +++++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/web_ui/lib/src/engine/browser_detection.dart b/lib/web_ui/lib/src/engine/browser_detection.dart index 66801b61bc7ce..4073b82b0c0f2 100644 --- a/lib/web_ui/lib/src/engine/browser_detection.dart +++ b/lib/web_ui/lib/src/engine/browser_detection.dart @@ -268,6 +268,4 @@ int _detectWebGLVersion() { } /// Whether the current browser supports the Chromium variant of CanvasKit. -const bool browserSupportsCanvaskitChromium = false; -// TODO(mdebbar): Uncomment this to enable real detection of browser support. -// final bool browserSupportsCanvaskitChromium = domIntl.v8BreakIterator != null; +final bool browserSupportsCanvaskitChromium = domIntl.v8BreakIterator != null; diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart index 644ce9a79c10f..3dc3236783352 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart @@ -28,6 +28,10 @@ import 'renderer.dart'; /// Entrypoint into the CanvasKit API. late CanvasKit canvasKit; +/// TODO(mdebbar): Turn this on when CanvasKit Chromium is ready. +/// https://github.com/flutter/flutter/issues/122329 +const bool _enableCanvasKitChromiumInAutoMode = false; + /// Sets the [CanvasKit] object on `window` so we can use `@JS()` to bind to /// static APIs. /// @@ -2698,7 +2702,7 @@ List get _canvasKitJsFileNames { switch (configuration.canvasKitVariant) { case CanvasKitVariant.auto: return [ - if (browserSupportsCanvaskitChromium) _kChromiumCanvasKitJsFileName, + if (_enableCanvasKitChromiumInAutoMode) _kChromiumCanvasKitJsFileName, _kFullCanvasKitJsFileName, ]; case CanvasKitVariant.full: From 1bd24546cc3074347d8af9da3d1e2ff8f50b5509 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Thu, 9 Mar 2023 16:20:40 -0500 Subject: [PATCH 4/4] fix a lint --- lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart index 3dc3236783352..dbd01511304d6 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart @@ -28,8 +28,8 @@ import 'renderer.dart'; /// Entrypoint into the CanvasKit API. late CanvasKit canvasKit; -/// TODO(mdebbar): Turn this on when CanvasKit Chromium is ready. -/// https://github.com/flutter/flutter/issues/122329 +// TODO(mdebbar): Turn this on when CanvasKit Chromium is ready. +// https://github.com/flutter/flutter/issues/122329 const bool _enableCanvasKitChromiumInAutoMode = false; /// Sets the [CanvasKit] object on `window` so we can use `@JS()` to bind to