Skip to content

Commit

Permalink
Fix a few issues rendering text with Skwasm.
Browse files Browse the repository at this point in the history
This fixes flutter/flutter#141001
This also fixes flutter/flutter#143743

* We need to always call `setStrutEnabled(true)` on `StrutStyle`.
* `getLineMetricsAt` had reversed ternary logic.
* Ported unit tests from CanvasKit over to UI to cover Skwasm and HTML
* The HTML renderer should return 0 line metrics for an empty paragraph.
  • Loading branch information
eyebrowsoffire committed Feb 28, 2024
1 parent 49f5d96 commit 7014f7c
Show file tree
Hide file tree
Showing 8 changed files with 177 additions and 129 deletions.
6 changes: 5 additions & 1 deletion lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paragraph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ class SkwasmParagraph extends SkwasmObjectWrapper<RawParagraph> implements ui.Pa
@override
ui.LineMetrics? getLineMetricsAt(int index) {
final LineMetricsHandle lineMetrics = paragraphGetLineMetricsAtIndex(handle, index);
return lineMetrics == nullptr ? SkwasmLineMetrics._(lineMetrics) : null;
return lineMetrics != nullptr ? SkwasmLineMetrics._(lineMetrics) : null;
}
}

Expand Down Expand Up @@ -746,6 +746,10 @@ class SkwasmParagraphStyle extends SkwasmObjectWrapper<RawParagraphStyle> implem
skStringFree(localeHandle);
}
paragraphStyleSetTextStyle(handle, textStyleHandle);

if (ui.ParagraphBuilder.shouldDisableRoundingHack) {
paragraphStyleSetApplyRoundingHack(handle, false);
}
return SkwasmParagraphStyle._(
handle,
textStyle,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,6 @@ external void paragraphStyleSetStrutStyle(ParagraphStyleHandle handle, StrutStyl

@Native<Void Function(ParagraphStyleHandle, TextStyleHandle)>(symbol: 'paragraphStyle_setTextStyle', isLeaf: true)
external void paragraphStyleSetTextStyle(ParagraphStyleHandle handle, TextStyleHandle textStyle);

@Native<Void Function(ParagraphStyleHandle, Bool)>(symbol: 'paragraphStyle_setApplyRoundingHack', isLeaf: true)
external void paragraphStyleSetApplyRoundingHack(ParagraphStyleHandle handle, bool applyRoundingHack);
3 changes: 3 additions & 0 deletions lib/web_ui/lib/src/engine/text/layout_fragmenter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ class LayoutFragmenter extends TextFragmenter {

@override
List<LayoutFragment> fragment() {
if (text.isEmpty) {
return <LayoutFragment>[];
}
final List<LayoutFragment> fragments = <LayoutFragment>[];

int fragmentStart = 0;
Expand Down
5 changes: 5 additions & 0 deletions lib/web_ui/skwasm/text/paragraph_style.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,8 @@ SKWASM_EXPORT void paragraphStyle_setTextStyle(ParagraphStyle* style,
TextStyle* textStyle) {
style->setTextStyle(*textStyle);
}

SKWASM_EXPORT void paragraphStyle_setApplyRoundingHack(ParagraphStyle* style,
bool applyRoundingHack) {
style->setApplyRoundingHack(applyRoundingHack);
}
4 changes: 3 additions & 1 deletion lib/web_ui/skwasm/text/strut_style.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
using namespace skia::textlayout;

SKWASM_EXPORT StrutStyle* strutStyle_create() {
return new StrutStyle();
auto style = new StrutStyle();
style->setStrutEnabled(true);
return style;
}

SKWASM_EXPORT void strutStyle_dispose(StrutStyle* style) {
Expand Down
127 changes: 0 additions & 127 deletions lib/web_ui/test/canvaskit/text_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -125,133 +125,6 @@ void testMain() {
}
});
});

test('empty paragraph', () {
const double fontSize = 10.0;
final ui.Paragraph paragraph = ui.ParagraphBuilder(CkParagraphStyle(
fontSize: fontSize,
)).build();
paragraph.layout(const ui.ParagraphConstraints(width: double.infinity));

expect(paragraph.getLineMetricsAt(0), isNull);
expect(paragraph.numberOfLines, 0);
expect(paragraph.getLineNumberAt(0), isNull);
});

test('Basic line related metrics', () {
const double fontSize = 10;
final ui.ParagraphBuilder builder = ui.ParagraphBuilder(CkParagraphStyle(
fontStyle: ui.FontStyle.normal,
fontWeight: ui.FontWeight.normal,
fontSize: fontSize,
maxLines: 1,
ellipsis: 'BBB',
))..addText('A' * 100);
final ui.Paragraph paragraph = builder.build();
paragraph.layout(const ui.ParagraphConstraints(width: 100.0));

expect(paragraph.numberOfLines, 1);

expect(paragraph.getLineMetricsAt(-1), isNull);
expect(paragraph.getLineMetricsAt(0)?.lineNumber, 0);
expect(paragraph.getLineMetricsAt(1), isNull);

expect(paragraph.getLineNumberAt(-1), isNull);
expect(paragraph.getLineNumberAt(0), 0);
expect(paragraph.getLineNumberAt(6), 0);
// The last 3 characters on the first line are ellipsized with BBB.
expect(paragraph.getLineMetricsAt(7), isNull);
});

test('Basic glyph metrics', () {
const double fontSize = 10;
final ui.ParagraphBuilder builder = ui.ParagraphBuilder(CkParagraphStyle(
fontStyle: ui.FontStyle.normal,
fontWeight: ui.FontWeight.normal,
fontFamily: 'FlutterTest',
fontSize: fontSize,
maxLines: 1,
ellipsis: 'BBB',
))..addText('A' * 100);
final ui.Paragraph paragraph = builder.build();
paragraph.layout(const ui.ParagraphConstraints(width: 100.0));

expect(paragraph.getGlyphInfoAt(-1), isNull);

// The last 3 characters on the first line are ellipsized with BBB.
expect(paragraph.getGlyphInfoAt(0)?.graphemeClusterCodeUnitRange, const ui.TextRange(start: 0, end: 1));
expect(paragraph.getGlyphInfoAt(6)?.graphemeClusterCodeUnitRange, const ui.TextRange(start: 6, end: 7));
expect(paragraph.getGlyphInfoAt(7), isNull);
expect(paragraph.getGlyphInfoAt(200), isNull);
});

test('Basic glyph metrics - hit test', () {
const double fontSize = 10.0;
final ui.ParagraphBuilder builder = ui.ParagraphBuilder(CkParagraphStyle(
fontSize: fontSize,
fontFamily: 'FlutterTest',
))..addText('Test\nTest');
final ui.Paragraph paragraph = builder.build();
paragraph.layout(const ui.ParagraphConstraints(width: double.infinity));

final ui.GlyphInfo? bottomRight = paragraph.getClosestGlyphInfoForOffset(const ui.Offset(99.0, 99.0));
final ui.GlyphInfo? last = paragraph.getGlyphInfoAt(8);
expect(bottomRight, equals(last));
expect(bottomRight, isNot(paragraph.getGlyphInfoAt(0)));

expect(bottomRight?.graphemeClusterLayoutBounds, const ui.Rect.fromLTWH(30, 10, 10, 10));
expect(bottomRight?.graphemeClusterCodeUnitRange, const ui.TextRange(start: 8, end: 9));
expect(bottomRight?.writingDirection, ui.TextDirection.ltr);
});

test('rounding hack disabled by default', () {
expect(ui.ParagraphBuilder.shouldDisableRoundingHack, isTrue);

const double fontSize = 1.25;
const String text = '12345';
assert((fontSize * text.length).truncate() != fontSize * text.length);
final ui.ParagraphBuilder builder = ui.ParagraphBuilder(
ui.ParagraphStyle(fontSize: fontSize, fontFamily: 'FlutterTest'),
);
builder.addText(text);
final ui.Paragraph paragraph = builder.build()
..layout(const ui.ParagraphConstraints(width: text.length * fontSize));

expect(paragraph.maxIntrinsicWidth, text.length * fontSize);
switch (paragraph.computeLineMetrics()) {
case [ui.LineMetrics(width: final double width)]:
expect(width, text.length * fontSize);
case final List<ui.LineMetrics> metrics:
expect(metrics, hasLength(1));
}
});

test('setDisableRoundinghHack to false works in tests', () {
bool assertsEnabled = false;
assert(() {
assertsEnabled = true;
return true;
}());
if (!assertsEnabled){
return;
}

if (ui.ParagraphBuilder.shouldDisableRoundingHack) {
ui.ParagraphBuilder.setDisableRoundingHack(false);
addTearDown(() => ui.ParagraphBuilder.setDisableRoundingHack(true));
}

assert(!ui.ParagraphBuilder.shouldDisableRoundingHack);
const double fontSize = 1.25;
const String text = '12345';
assert((fontSize * text.length).truncate() != fontSize * text.length);
final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle(fontSize: fontSize, fontFamily: 'FlutterTest'));
builder.addText(text);
final ui.Paragraph paragraph = builder.build()
..layout(const ui.ParagraphConstraints(width: text.length * fontSize));
expect(paragraph.computeLineMetrics().length, greaterThan(1));
});

// TODO(hterkelsen): https://github.com/flutter/flutter/issues/71520
}, skip: isSafari || isFirefox);
}
148 changes: 148 additions & 0 deletions lib/web_ui/test/ui/line_metrics_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

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/test_initialization.dart';
import 'utils.dart';

void main() {
internalBootstrapBrowserTest(() => testMain);
}

Future<void> testMain() async {
setUpUnitTests(
withImplicitView: true,
setUpTestViewDimensions: false,
);

test('empty paragraph', () {
const double fontSize = 10.0;
final ui.Paragraph paragraph = ui.ParagraphBuilder(ui.ParagraphStyle(
fontSize: fontSize,
)).build();
paragraph.layout(const ui.ParagraphConstraints(width: double.infinity));

expect(paragraph.getLineMetricsAt(0), isNull);
expect(paragraph.numberOfLines, 0);
expect(paragraph.getLineNumberAt(0), isNull);
});

test('Basic line related metrics', () {
const double fontSize = 10;
final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle(
fontStyle: ui.FontStyle.normal,
fontWeight: ui.FontWeight.normal,
fontSize: fontSize,
maxLines: 1,
ellipsis: 'BBB',
))..addText('A' * 100);
final ui.Paragraph paragraph = builder.build();
paragraph.layout(const ui.ParagraphConstraints(width: 100.0));

expect(paragraph.numberOfLines, 1);

expect(paragraph.getLineMetricsAt(-1), isNull);
expect(paragraph.getLineMetricsAt(0)?.lineNumber, 0);
expect(paragraph.getLineMetricsAt(1), isNull);

expect(paragraph.getLineNumberAt(-1), isNull);
expect(paragraph.getLineNumberAt(0), 0);
expect(paragraph.getLineNumberAt(6), 0);
// The last 3 characters on the first line are ellipsized with BBB.
expect(paragraph.getLineMetricsAt(7), isNull);
});

test('Basic glyph metrics', () {
const double fontSize = 10;
final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle(
fontStyle: ui.FontStyle.normal,
fontWeight: ui.FontWeight.normal,
fontFamily: 'FlutterTest',
fontSize: fontSize,
maxLines: 1,
ellipsis: 'BBB',
))..addText('A' * 100);
final ui.Paragraph paragraph = builder.build();
paragraph.layout(const ui.ParagraphConstraints(width: 100.0));

expect(paragraph.getGlyphInfoAt(-1), isNull);

// The last 3 characters on the first line are ellipsized with BBB.
expect(paragraph.getGlyphInfoAt(0)?.graphemeClusterCodeUnitRange, const ui.TextRange(start: 0, end: 1));
expect(paragraph.getGlyphInfoAt(6)?.graphemeClusterCodeUnitRange, const ui.TextRange(start: 6, end: 7));
expect(paragraph.getGlyphInfoAt(7), isNull);
expect(paragraph.getGlyphInfoAt(200), isNull);
});

test('Basic glyph metrics - hit test', () {
const double fontSize = 10.0;
final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle(
fontSize: fontSize,
fontFamily: 'FlutterTest',
))..addText('Test\nTest');
final ui.Paragraph paragraph = builder.build();
paragraph.layout(const ui.ParagraphConstraints(width: double.infinity));

final ui.GlyphInfo? bottomRight = paragraph.getClosestGlyphInfoForOffset(const ui.Offset(99.0, 99.0));
final ui.GlyphInfo? last = paragraph.getGlyphInfoAt(8);
expect(bottomRight, equals(last));
expect(bottomRight, isNot(paragraph.getGlyphInfoAt(0)));

expect(bottomRight?.graphemeClusterLayoutBounds, const ui.Rect.fromLTWH(30, 10, 10, 10));
expect(bottomRight?.graphemeClusterCodeUnitRange, const ui.TextRange(start: 8, end: 9));
expect(bottomRight?.writingDirection, ui.TextDirection.ltr);
});

test('rounding hack disabled by default', () {
expect(ui.ParagraphBuilder.shouldDisableRoundingHack, isTrue);

const double fontSize = 1.25;
const String text = '12345';
assert((fontSize * text.length).truncate() != fontSize * text.length);
final ui.ParagraphBuilder builder = ui.ParagraphBuilder(
ui.ParagraphStyle(fontSize: fontSize, fontFamily: 'FlutterTest'),
);
builder.addText(text);
final ui.Paragraph paragraph = builder.build()
..layout(const ui.ParagraphConstraints(width: text.length * fontSize));

expect(paragraph.maxIntrinsicWidth, text.length * fontSize);
switch (paragraph.computeLineMetrics()) {
case [ui.LineMetrics(width: final double width)]:
expect(width, text.length * fontSize);
case final List<ui.LineMetrics> metrics:
expect(metrics, hasLength(1));
}
}, skip: isHtml); // The rounding hack doesn't apply to the html renderer

test('setDisableRoundingHack to false works in tests', () {
bool assertsEnabled = false;
assert(() {
assertsEnabled = true;
return true;
}());
if (!assertsEnabled){
return;
}

if (ui.ParagraphBuilder.shouldDisableRoundingHack) {
ui.ParagraphBuilder.setDisableRoundingHack(false);
addTearDown(() => ui.ParagraphBuilder.setDisableRoundingHack(true));
}

assert(!ui.ParagraphBuilder.shouldDisableRoundingHack);
const double fontSize = 1.25;
const String text = '12345';
assert((fontSize * text.length).truncate() != fontSize * text.length);
final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle(fontSize: fontSize, fontFamily: 'FlutterTest'));
builder.addText(text);
final ui.Paragraph paragraph = builder.build()
..layout(const ui.ParagraphConstraints(width: text.length * fontSize));
expect(paragraph.computeLineMetrics().length, greaterThan(1));
}, skip: isHtml); // The rounding hack doesn't apply to the html renderer
}
10 changes: 10 additions & 0 deletions lib/web_ui/test/ui/text_golden_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,16 @@ Future<void> testMain() async {
);
});

test('strut style - override height', () async {
await testTextStyle(
'strut style',
paragraphStrutStyle: ui.StrutStyle(
forceStrutHeight: true,
height: 2,
),
);
});

test('sample Chinese text', () async {
await testSampleText(
'chinese',
Expand Down

0 comments on commit 7014f7c

Please sign in to comment.