From 960f3f347cfeca357e614536aa6477bf1f3ef532 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Sharma?= <737941+loic-sharma@users.noreply.github.com> Date: Wed, 14 Sep 2022 11:17:49 -0700 Subject: [PATCH] [Web] Draw tab characters as single space (flutter/engine#36107) --- engine/src/flutter/lib/web_ui/dev/felt.bat | 5 ++-- .../flutter/lib/web_ui/dev/felt_windows.bat | 9 +++--- .../src/engine/canvaskit/canvaskit_api.dart | 1 + .../web_ui/lib/src/engine/canvaskit/text.dart | 1 + .../lib/web_ui/test/canvaskit/common.dart | 18 +++++++++++ .../lib/web_ui/test/canvaskit/text_test.dart | 30 ++++++++++++++++++- 6 files changed, 55 insertions(+), 9 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/dev/felt.bat b/engine/src/flutter/lib/web_ui/dev/felt.bat index 581e52aea2e..7a91c67f049 100644 --- a/engine/src/flutter/lib/web_ui/dev/felt.bat +++ b/engine/src/flutter/lib/web_ui/dev/felt.bat @@ -24,7 +24,6 @@ SET OUT_DIR=%ENGINE_SRC_DIR%\out SET HOST_DEBUG_UNOPT_DIR=%OUT_DIR%\host_debug_unopt SET DART_SDK_DIR=%HOST_DEBUG_UNOPT_DIR%\dart-sdk SET DART_BIN=%DART_SDK_DIR%\bin\dart -SET PUB_BIN=%DART_SDK_DIR%\bin\pub SET FLUTTER_DIR=%ENGINE_SRC_DIR%\flutter SET WEB_UI_DIR=%FLUTTER_DIR%\lib\web_ui SET DEV_DIR=%WEB_UI_DIR%\dev @@ -56,11 +55,11 @@ IF %needsHostDebugUnoptRebuild%==1 ( cd %WEB_UI_DIR% IF NOT EXIST "%SNAPSHOT_PATH%" ( ECHO Precompiling felt snapshot - CALL %PUB_BIN% get + CALL %DART_SDK_DIR%\bin\dart pub get %DART_BIN% --snapshot="%SNAPSHOT_PATH%" --packages="%WEB_UI_DIR%\.dart_tool\package_config.json" %FELT_PATH% ) -IF %1==test ( +IF "%1"=="test" ( %DART_SDK_DIR%\bin\dart --packages="%WEB_UI_DIR%\.dart_tool\package_config.json" "%SNAPSHOT_PATH%" %* --browser=chrome ) ELSE ( %DART_SDK_DIR%\bin\dart --packages="%WEB_UI_DIR%\.dart_tool\package_config.json" "%SNAPSHOT_PATH%" %* diff --git a/engine/src/flutter/lib/web_ui/dev/felt_windows.bat b/engine/src/flutter/lib/web_ui/dev/felt_windows.bat index f79bd49e0c5..a70567ea3d9 100644 --- a/engine/src/flutter/lib/web_ui/dev/felt_windows.bat +++ b/engine/src/flutter/lib/web_ui/dev/felt_windows.bat @@ -28,7 +28,6 @@ SET DEV_DIR="%WEB_UI_DIR%dev" SET OUT_DIR="%ENGINE_SRC_DIR%out" SET HOST_DEBUG_UNOPT_DIR="%ENGINE_SRC_DIR%out\host_debug_unopt" SET DART_SDK_DIR=%ENGINE_SRC_DIR%out\host_debug_unopt\dart-sdk -SET PUB_DIR="%DART_SDK_DIR%\bin\pub" SET SCRIPT_PATH="%DEV_DIR%felt.dart" SET STAMP_PATH="%DART_TOOL_DIR%felt.snapshot.stamp" SET GN="%FLUTTER_DIR%tools\gn" @@ -48,18 +47,18 @@ IF %orTempValue%==0 ( CALL python %GN% --unoptimized --full-dart-sdk CALL ninja -C %HOST_DEBUG_UNOPT_DIR%) -:: TODO(yjbanov): The batch script does not support snanphot option. +:: TODO(yjbanov): The batch script does not support snapshot option. :: Support snapshot option. CALL :installdeps -IF %1==test (%DART_SDK_DIR%\bin\dart "%DEV_DIR%\felt.dart" %* --browser=chrome) ELSE ( %DART_SDK_DIR%\bin\dart "%DEV_DIR%\felt.dart" %* ) +IF "%1"=="test" (%DART_SDK_DIR%\bin\dart %DEV_DIR%\felt.dart %* --browser=chrome) ELSE ( %DART_SDK_DIR%\bin\dart %DEV_DIR%\felt.dart %* ) EXIT /B %ERRORLEVEL% :installdeps ECHO "Running \`pub get\` in 'engine/src/flutter/web_sdk/web_engine_tester'" cd "%FLUTTER_DIR%web_sdk\web_engine_tester" -CALL %PUB_DIR% get +CALL %DART_SDK_DIR%\bin\dart pub get ECHO "Running \`pub get\` in 'engine/src/flutter/lib/web_ui'" cd %WEB_UI_DIR% -CALL %PUB_DIR% get +CALL %DART_SDK_DIR%\bin\dart pub get EXIT /B 0 diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart index 996355850bd..2d8e8490dcb 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart @@ -1900,6 +1900,7 @@ extension SkParagraphStylePropertiesExtension on SkParagraphStyleProperties { external set ellipsis(String? value); external set textStyle(SkTextStyleProperties? value); external set strutStyle(SkStrutStyleProperties? strutStyle); + external set replaceTabCharacters(bool? bool); } @JS() diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart index 3a5efc1fb50..55da65bab6b 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart @@ -178,6 +178,7 @@ class CkParagraphStyle implements ui.ParagraphStyle { toSkStrutStyleProperties(strutStyle, textHeightBehavior); } + properties.replaceTabCharacters = true; properties.textStyle = toSkTextStyleProperties( fontFamily, fontSize, height, fontWeight, fontStyle); diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/common.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/common.dart index 8bcd58c8068..68d88c0396d 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/common.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/common.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:typed_data'; import 'package:test/test.dart'; @@ -195,6 +196,23 @@ Future matchPictureGolden(String goldenFile, CkPicture picture, await matchGoldenFile(goldenFile, region: region); } +Future matchImage(ui.Image left, ui.Image right) async { + if (left.width != right.width || left.height != right.height) { + return false; + } + int getPixel(ByteData data, int x, int y) => data.getUint32((x + y * left.width) * 4); + final ByteData leftData = (await left.toByteData())!; + final ByteData rightData = (await right.toByteData())!; + for (int y = 0; y < left.height; y++) { + for (int x = 0; x < left.width; x++) { + if (getPixel(leftData, x, y) != getPixel(rightData, x, y)) { + return false; + } + } + } + return true; +} + /// Sends a platform message to create a Platform View with the given id and viewType. Future createPlatformView(int id, String viewType) { final Completer completer = Completer(); diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/text_test.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/text_test.dart index ec5621731a1..87efad258b6 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/text_test.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/text_test.dart @@ -4,6 +4,7 @@ import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; +import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart' as ui; import 'common.dart'; @@ -65,5 +66,32 @@ void testMain() { // because the directionality of the 'h' is LTR. expect(boxes.single.direction, equals(ui.TextDirection.ltr)); }); - }); + + test('Renders tab as space instead of tofu', () async { + // CanvasKit renders a tofu if the font does not have a glyph for a + // character. However, Flutter opts-in to a CanvasKit feature to render + // tabs as a single space. + // See: https://github.com/flutter/flutter/issues/79153 + Future drawText(String text) { + const ui.Rect bounds = ui.Rect.fromLTRB(0, 0, 100, 100); + final CkPictureRecorder recorder = CkPictureRecorder(); + final CkCanvas canvas = recorder.beginRecording(bounds); + final CkParagraph paragraph = makeSimpleText(text); + + canvas.drawParagraph(paragraph, ui.Offset.zero); + final ui.Picture picture = recorder.endRecording(); + return picture.toImage(100, 100); + } + + // The backspace character, \b, does not have a corresponding glyph and + // is rendered as a tofu. + final ui.Image tabImage = await drawText('>\t<'); + final ui.Image spaceImage = await drawText('> <'); + final ui.Image tofuImage = await drawText('>\b<'); + + expect(await matchImage(tabImage, spaceImage), isTrue); + expect(await matchImage(tabImage, tofuImage), isFalse); + }); + // TODO(hterkelsen): https://github.com/flutter/flutter/issues/71520 + }, skip: isSafari || isFirefox); }