Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] roll CanvasKit 0.36.1 (attempt 3) #36005

Merged
merged 1 commit into from
Sep 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ vars = {

# WARNING: DO NOT EDIT canvaskit_cipd_instance MANUALLY
# See `lib/web_ui/README.md` for how to roll CanvasKit to a new version.
'canvaskit_cipd_instance': 'yrsfF-vTvu4jzBBm1o6tDl70dky-l4G29Dnj75UvRDgC',
'canvaskit_cipd_instance': '8wh6J7ZXGCgo1vvCQIqkmskYQZfjhcXQG1YCKFNrzRUC',

# Do not download the Emscripten SDK by default.
# This prevents us from downloading the Emscripten toolchain for builds
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/dev/canvaskit_lock.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Specifies the version of CanvasKit to use for Flutter Web apps.
#
# See `lib/web_ui/README.md` for how to update this file.
canvaskit_version: "0.35.0"
canvaskit_version: "0.36.1"
8 changes: 7 additions & 1 deletion lib/web_ui/lib/src/engine/assets.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,15 @@ import 'dart:convert';
import 'dart:typed_data';

import 'dom.dart';
import 'text/font_collection.dart';
import 'util.dart';

const String ahemFontFamily = 'Ahem';
const String ahemFontUrl = '/assets/fonts/ahem.ttf';
const String robotoFontFamily = 'Roboto';
const String robotoTestFontUrl = '/assets/fonts/Roboto-Regular.ttf';
const String robotoVariableFontFamily = 'RobotoVariable';
const String robotoVariableTestFontUrl = '/assets/fonts/RobotoSlab-VariableFont_wght.ttf';

/// This class downloads assets over the network.
///
/// The assets are resolved relative to [assetsDir] inside the directory
Expand Down
5 changes: 0 additions & 5 deletions lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1853,11 +1853,6 @@ extension SkPictureExtension on SkPicture {
class SkParagraphBuilderNamespace {}

extension SkParagraphBuilderNamespaceExtension on SkParagraphBuilderNamespace {
external SkParagraphBuilder Make(
SkParagraphStyle paragraphStyle,
SkFontMgr? fontManager,
);

external SkParagraphBuilder MakeFromFontProvider(
SkParagraphStyle paragraphStyle,
TypefaceFontProvider? fontManager,
Expand Down
16 changes: 9 additions & 7 deletions lib/web_ui/lib/src/engine/canvaskit/fonts.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ import 'font_fallbacks.dart';
const String _robotoUrl =
'https://fonts.gstatic.com/s/roboto/v20/KFOmCnqEu92Fr1Me5WZLCzYlKw.ttf';

// URL for the Ahem font, only used in tests.
const String _ahemUrl = '/assets/fonts/ahem.ttf';

/// Manages the fonts used in the Skia-based backend.
class SkiaFontCollection implements FontCollection {
final Set<String> _registeredFontFamilies = <String>{};
Expand Down Expand Up @@ -170,13 +167,19 @@ class SkiaFontCollection implements FontCollection {
/// different URLs.
@override
void debugRegisterTestFonts() {
if (!_isFontFamilyRegistered('Ahem')) {
_registerFont(_ahemUrl, 'Ahem');
if (!_isFontFamilyRegistered(ahemFontFamily)) {
_registerFont(ahemFontUrl, ahemFontFamily);
}
if (!_isFontFamilyRegistered(robotoFontFamily)) {
_registerFont(robotoTestFontUrl, robotoFontFamily);
}
if (!_isFontFamilyRegistered(robotoVariableFontFamily)) {
_registerFont(robotoVariableTestFontUrl, robotoVariableFontFamily);
}

// Ahem must be added to font fallbacks list regardless of where it was
// downloaded from.
FontFallbackData.instance.globalFontFallbacks.add('Ahem');
FontFallbackData.instance.globalFontFallbacks.add(ahemFontFamily);
}

void _registerFont(String url, String family) {
Expand Down Expand Up @@ -221,7 +224,6 @@ class SkiaFontCollection implements FontCollection {
.then<ByteBuffer>((dynamic x) => x as ByteBuffer);
}

SkFontMgr? skFontMgr;
TypefaceFontProvider? fontProvider;

@override
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import 'package:js/js.dart';
/// The version of CanvasKit used by the web engine by default.
// DO NOT EDIT THE NEXT LINE OF CODE MANUALLY
// See `lib/web_ui/README.md` for how to roll CanvasKit to a new version.
const String _canvaskitVersion = '0.35.0';
const String _canvaskitVersion = '0.36.1';

/// The Web Engine configuration for the current application.
FlutterConfiguration get configuration => _configuration ??= FlutterConfiguration(_jsConfiguration);
Expand Down
7 changes: 0 additions & 7 deletions lib/web_ui/lib/src/engine/text/font_collection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,6 @@ import '../safe_browser_api.dart';
import '../util.dart';
import 'layout_service.dart';

const String ahemFontFamily = 'Ahem';
const String ahemFontUrl = '/assets/fonts/ahem.ttf';
const String robotoFontFamily = 'Roboto';
const String robotoTestFontUrl = '/assets/fonts/Roboto-Regular.ttf';
const String robotoVariableFontFamily = 'RobotoVariable';
const String robotoVariableTestFontUrl = '/assets/fonts/RobotoSlab-VariableFont_wght.ttf';

/// This class is responsible for registering and loading fonts.
///
/// Once an asset manager has been set in the framework, call
Expand Down
131 changes: 91 additions & 40 deletions lib/web_ui/test/canvaskit/canvaskit_api_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:test/test.dart';

import 'package:ui/src/engine.dart';
import 'package:ui/ui.dart' as ui;
import 'package:web_engine_tester/golden_tester.dart';

import '../matchers.dart';
import 'common.dart';
Expand Down Expand Up @@ -49,7 +50,9 @@ void testMain() {
_matrix4x4CompositionTests();
_toSkRectTests();
_skVerticesTests();
_paragraphTests();
group('SkParagraph', () {
_paragraphTests();
});
group('SkPath', () {
_pathTests();
});
Expand Down Expand Up @@ -1447,31 +1450,36 @@ void _textStyleTests() {
}

void _paragraphTests() {
setUpAll(() async {
CanvasKitRenderer.instance.fontCollection.debugRegisterTestFonts();
await CanvasKitRenderer.instance.fontCollection.ensureFontsLoaded();
});

// This test is just a kitchen sink that blasts CanvasKit with all paragraph
// properties all at once, making sure CanvasKit doesn't choke on anything.
// In particular, this tests that our JS bindings are correct, such as that
// arguments are of acceptable types and passed in the correct order.
test('SkParagraph API kitchensink', () {
test('kitchensink', () async {
final SkParagraphStyleProperties props = SkParagraphStyleProperties();
props.textAlign = canvasKit.TextAlign.Center;
props.textAlign = canvasKit.TextAlign.Left;
props.textDirection = canvasKit.TextDirection.RTL;
props.heightMultiplier = 3;
props.textHeightBehavior = canvasKit.TextHeightBehavior.All;
props.maxLines = 4;
props.ellipsis = '___';
props.textStyle = SkTextStyleProperties()
..backgroundColor = Float32List.fromList(<double>[1, 2, 3, 4])
..color = Float32List.fromList(<double>[5, 6, 7, 8])
..foregroundColor = Float32List.fromList(<double>[9, 10, 11, 12])
..backgroundColor = Float32List.fromList(<double>[0.2, 0, 0, 0.5])
..color = Float32List.fromList(<double>[0, 1, 0, 1])
..foregroundColor = Float32List.fromList(<double>[1, 0, 1, 1])
..decoration = 0x2
..decorationThickness = 2.0
..decorationColor = Float32List.fromList(<double>[13, 14, 15, 16])
..decorationStyle = canvasKit.DecorationStyle.Dotted
..textBaseline = canvasKit.TextBaseline.Ideographic
..fontSize = 24
..fontSize = 48
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why these changes in fontSize, heightMultiplier and others? (I understand the colors were maybe invalid before, but the rest?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this test a golden test. Previously the text we rendered was complete nonsense. Now it is displaying something reasonable: https://flutter-engine-gold.skia.org/diff?test=paragraph_kitchen_sink&left=c8f1c90d490826fdbcaf4e2a4762fc76&right=eb3fc116f84bdf3e401fe0345ccde4f3

..letterSpacing = 5
..wordSpacing = 10
..heightMultiplier = 2.5
..heightMultiplier = 1.3
..halfLeading = true
..locale = 'en_CA'
..fontFamilies = <String>['Roboto', 'serif']
Expand All @@ -1486,23 +1494,24 @@ void _paragraphTests() {
SkFontFeature()
..name = 'tnum'
..value = 1,
];
]
;
props.strutStyle = SkStrutStyleProperties()
..fontFamilies = <String>['Roboto', 'Noto']
..fontStyle = (SkFontStyle()
..slant = canvasKit.FontSlant.Italic
..weight = canvasKit.FontWeight.Bold)
..fontSize = 23
..heightMultiplier = 5
..fontSize = 72
..heightMultiplier = 1.5
..halfLeading = true
..leading = 6
..leading = 0
..strutEnabled = true
..forceStrutHeight = false;

final SkParagraphStyle paragraphStyle = canvasKit.ParagraphStyle(props);
final SkParagraphBuilder builder = canvasKit.ParagraphBuilder.Make(
final SkParagraphBuilder builder = canvasKit.ParagraphBuilder.MakeFromFontProvider(
paragraphStyle,
CanvasKitRenderer.instance.fontCollection.skFontMgr,
CanvasKitRenderer.instance.fontCollection.fontProvider,
);

builder.addText('Hello');
Expand All @@ -1513,51 +1522,93 @@ void _paragraphTests() {
canvasKit.TextBaseline.Ideographic,
4.0,
);
builder
.pushStyle(canvasKit.TextStyle(SkTextStyleProperties()..fontSize = 12));
builder.pushStyle(canvasKit.TextStyle(SkTextStyleProperties()
..color = Float32List.fromList(<double>[1, 0, 0, 1])
..fontSize = 24
..fontFamilies = <String>['Roboto', 'serif']
));
builder.addText('World');
builder.pop();
builder.pushPaintStyle(
canvasKit.TextStyle(SkTextStyleProperties()..fontSize = 12),
SkPaint(),
SkPaint());
canvasKit.TextStyle(SkTextStyleProperties()
..color = Float32List.fromList(<double>[1, 0, 0, 1])
..fontSize = 60
..fontFamilies = <String>['Roboto', 'serif']
),
SkPaint()..setColorInt(0xFF0000FF),
SkPaint()..setColorInt(0xFFFF0000),
);
builder.addText('!');
builder.pop();
builder.pushStyle(
canvasKit.TextStyle(SkTextStyleProperties()..halfLeading = true));
builder.pop();
final SkParagraph paragraph = builder.build();
paragraph.layout(55);
expect(paragraph.getAlphabeticBaseline(),
within<double>(distance: 0.5, from: 20.7));
paragraph.layout(500);

final DomCanvasElement canvas = createDomCanvasElement(
width: 400,
height: 160,
);
domDocument.body!.append(canvas);

// TODO(yjbanov): WebGL screenshot tests do not work on Firefox - https://github.com/flutter/flutter/issues/109265
if (!isFirefox) {
final SkSurface surface = canvasKit.MakeWebGLCanvasSurface(canvas);
final SkCanvas skCanvas = surface.getCanvas();
skCanvas.drawColorInt(0xFFCCCCCC, toSkBlendMode(ui.BlendMode.srcOver));
skCanvas.drawParagraph(paragraph, 20, 20);
skCanvas.drawRect(
Float32List.fromList(<double>[20, 20, 20 + paragraph.getMaxIntrinsicWidth(), 20 + paragraph.getHeight()]),
SkPaint()
..setStyle(toSkPaintStyle(ui.PaintingStyle.stroke))
..setStrokeWidth(1)
..setColorInt(0xFF00FF00),
);
surface.flush();

await matchGoldenFile(
'paragraph_kitchen_sink.png',
region: const ui.Rect.fromLTRB(0, 0, 400, 160),
maxDiffRatePercent: 0.0,
write: true,
);
}

void expectAlmost(double actual, double expected) {
expect(actual, within<double>(distance: actual / 100, from: expected));
}
Comment on lines +1578 to +1580
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need your own implementation to validate this?

There's already moreOrLessEquals and closeTo that might help here (and probably more!).

expect(actual, moreOrLessEquals(expected, epsilon: actual/100));

(Also not sure about using actual as part of the epsilon value... shouldn't it be fixed to guarantee accuracy to X decimal points? Like 0.1 or maybe 1?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moreOrLessEquals is from package:flutter_test, which we don't have access to. closeTo is interesting. Right now within is standard in the code base, so I think for now I'll prefer to continue relying on it.


expectAlmost(paragraph.getAlphabeticBaseline(), 85.5);
expect(paragraph.didExceedMaxLines(), isFalse);
expect(paragraph.getHeight(), 25);
expect(paragraph.getIdeographicBaseline(),
within<double>(distance: 0.5, from: 25));
expect(paragraph.getLongestLine(), 50);
expect(paragraph.getMaxIntrinsicWidth(), 50);
expect(paragraph.getMinIntrinsicWidth(), 50);
expect(paragraph.getMaxWidth(), 55);
expectAlmost(paragraph.getHeight(), 108);
expectAlmost(paragraph.getIdeographicBaseline(), 108);
expectAlmost(paragraph.getLongestLine(), 263);
expectAlmost(paragraph.getMaxIntrinsicWidth(), 263);
expectAlmost(paragraph.getMinIntrinsicWidth(), 135);
expectAlmost(paragraph.getMaxWidth(), 500);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, the width can be +-5 of 500 (I assume pixels?). Isn't that delta starting to be "too large" that could start masking actual measurement/rounding errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole test is just a smoke test. More precise text measurement tests are in dedicated files. So I'm not worried about the precision here. TBH I'd probably be happy if we just checked that the returned value is not null. That we are also checking for a specific value is a bonus.

expect(
paragraph.getRectsForRange(1, 3, canvasKit.RectHeightStyle.Tight,
canvasKit.RectWidthStyle.Max),
<double>[]);
paragraph.getRectsForRange(1, 3, canvasKit.RectHeightStyle.Tight, canvasKit.RectWidthStyle.Max).single,
hasLength(4),
);
expect(paragraph.getRectsForPlaceholders(), hasLength(1));
expect(paragraph.getLineMetrics(), hasLength(1));

final SkLineMetrics lineMetrics =
paragraph.getLineMetrics().cast<SkLineMetrics>().single;
expect(lineMetrics.ascent, within<double>(distance: 0.5, from: 20.7));
expect(lineMetrics.descent, within<double>(distance: 0.2, from: 4.3));
expectAlmost(lineMetrics.ascent, 55.6);
expectAlmost(lineMetrics.descent, 14.8);
expect(lineMetrics.isHardBreak, isTrue);
expect(lineMetrics.baseline, within<double>(distance: 0.5, from: 20.7));
expect(lineMetrics.height, 25);
expect(lineMetrics.left, 2.5);
expect(lineMetrics.width, 50);
expectAlmost(lineMetrics.baseline, 85.5);
expectAlmost(lineMetrics.height, 108);
expectAlmost(lineMetrics.left, 2.5);
expectAlmost(lineMetrics.width, 263);
expect(lineMetrics.lineNumber, 0);

expect(paragraph.getGlyphPositionAtCoordinate(5, 5).affinity,
canvasKit.Affinity.Downstream);
expect(
paragraph.getGlyphPositionAtCoordinate(5, 5).affinity,
canvasKit.Affinity.Upstream,
);

// "Hello"
for (int i = 0; i < 5; i++) {
Expand Down