Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[web] Fix canvas2d leaks in text measurement #38640

Merged
merged 5 commits into from
Jan 6, 2023
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
11 changes: 11 additions & 0 deletions lib/web_ui/lib/src/engine/dom.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'dart:typed_data';

import 'package:js/js.dart';
import 'package:js/js_util.dart' as js_util;
import 'package:meta/meta.dart';

/// This file contains static interop classes for interacting with the DOM and
/// some helpers. All of the classes in this file are named after their
Expand Down Expand Up @@ -584,7 +585,16 @@ class DomPerformanceMeasure extends DomPerformanceEntry {}
@staticInterop
class DomCanvasElement extends DomHTMLElement {}

@visibleForTesting
int debugCanvasCount = 0;

@visibleForTesting
void debugResetCanvasCount() {
debugCanvasCount = 0;
}

DomCanvasElement createDomCanvasElement({int? width, int? height}) {
debugCanvasCount++;
final DomCanvasElement canvas =
domWindow.document.createElement('canvas') as DomCanvasElement;
if (width != null) {
Expand Down Expand Up @@ -625,6 +635,7 @@ abstract class DomCanvasImageSource {}
class DomCanvasRenderingContext2D {}

extension DomCanvasRenderingContext2DExtension on DomCanvasRenderingContext2D {
external DomCanvasElement? get canvas;
external Object? get fillStyle;
external set fillStyle(Object? style);
external String get font;
Expand Down
45 changes: 27 additions & 18 deletions lib/web_ui/lib/src/engine/text/layout_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ import 'paragraph.dart';
import 'ruler.dart';
import 'text_direction.dart';

/// A single canvas2d context to use for all text measurements.
@visibleForTesting
final DomCanvasRenderingContext2D textContext =
// We don't use this canvas to draw anything, so let's make it as small as
// possible to save memory.
createDomCanvasElement(width: 0, height: 0).context2D;

/// The last font used in the [textContext].
String? _lastContextFont;

/// Performs layout on a [CanvasParagraph].
///
/// It uses a [DomCanvasElement] to measure text.
Expand All @@ -24,9 +34,6 @@ class TextLayoutService {

final CanvasParagraph paragraph;

final DomCanvasRenderingContext2D context =
createDomCanvasElement().context2D;

// *** Results of layout *** //

// Look at the Paragraph class for documentation of the following properties.
Expand All @@ -53,7 +60,7 @@ class TextLayoutService {
ui.Rect get paintBounds => _paintBounds;
ui.Rect _paintBounds = ui.Rect.zero;

late final Spanometer spanometer = Spanometer(paragraph, context);
late final Spanometer spanometer = Spanometer(paragraph);

late final LayoutFragmenter layoutFragmenter =
LayoutFragmenter(paragraph.plainText, paragraph.spans);
Expand Down Expand Up @@ -882,10 +889,9 @@ class LineBuilder {
/// it's set, the [Spanometer] updates the underlying [context] so that
/// subsequent measurements use the correct styles.
class Spanometer {
Spanometer(this.paragraph, this.context);
Spanometer(this.paragraph);

final CanvasParagraph paragraph;
final DomCanvasRenderingContext2D context;

static final RulerHost _rulerHost = RulerHost();

Expand All @@ -904,21 +910,31 @@ class Spanometer {
_rulers.clear();
}

String _cssFontString = '';

double? get letterSpacing => currentSpan.style.letterSpacing;

TextHeightRuler? _currentRuler;
ParagraphSpan? _currentSpan;

ParagraphSpan get currentSpan => _currentSpan!;
set currentSpan(ParagraphSpan? span) {
// Update the font string if it's different from the last applied font
// string.
//
// Also, we need to update the font string even if the span isn't changing.
// That's because `textContext` is shared across all spanometers.
if (span != null) {
final String newCssFontString = span.style.cssFontString;
if (_lastContextFont != newCssFontString) {
_lastContextFont = newCssFontString;
textContext.font = newCssFontString;
}
}

if (span == _currentSpan) {
return;
}
_currentSpan = span;

// No need to update css font string when `span` is null.
if (span == null) {
_currentRuler = null;
return;
Expand All @@ -933,13 +949,6 @@ class Spanometer {
_rulers[heightStyle] = ruler;
}
_currentRuler = ruler;

// Update the font string if it's different from the previous span.
final String cssFontString = span.style.cssFontString;
if (_cssFontString != cssFontString) {
_cssFontString = cssFontString;
context.font = cssFontString;
}
}

/// Whether the spanometer is ready to take measurements.
Expand All @@ -955,7 +964,7 @@ class Spanometer {
double get height => _currentRuler!.height;

double measureText(String text) {
return measureSubstring(context, text, 0, text.length);
return measureSubstring(textContext, text, 0, text.length);
}

double measureRange(int start, int end) {
Expand Down Expand Up @@ -1047,7 +1056,7 @@ class Spanometer {
assert(end >= currentSpan.start && end <= currentSpan.end);

return measureSubstring(
context,
textContext,
paragraph.plainText,
start,
end,
Expand Down
60 changes: 60 additions & 0 deletions lib/web_ui/test/text/layout_service_plain_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -691,4 +691,64 @@ Future<void> testMain() async {
l('i', 9, 10, hardBreak: true, width: 10.0, left: 40.0),
]);
});

test('uses a single minimal canvas', () {
debugResetCanvasCount();

plain(ahemStyle, 'Lorem').layout(constrain(double.infinity));
plain(ahemStyle, 'ipsum dolor').layout(constrain(150.0));
// Try different styles too.
plain(EngineParagraphStyle(fontWeight: ui.FontWeight.bold), 'sit amet').layout(constrain(300.0));

expect(textContext.canvas!.width, isZero);
expect(textContext.canvas!.height, isZero);
// This number is 0 instead of 1 because the canvas is created at the top
// level as a global variable. So by the time this test runs, the canvas
// would have been created already.
//
// So we just make sure that no new canvas is created after the above layout
// calls.
expect(debugCanvasCount, 0);
});

test('does not leak styles across spanometers', () {
// This prevents the Ahem font from being forced in all paragraphs.
ui.debugEmulateFlutterTesterEnvironment = false;

final CanvasParagraph p1 = plain(
EngineParagraphStyle(
fontSize: 20.0,
fontFamily: 'FontFamily1',
),
'Lorem',
)..layout(constrain(double.infinity));
// After the layout, the canvas should have the above style applied.
expect(textContext.font, contains('20px'));
expect(textContext.font, contains('FontFamily1'));

final CanvasParagraph p2 = plain(
EngineParagraphStyle(
fontSize: 40.0,
fontFamily: 'FontFamily2',
),
'ipsum dolor',
)..layout(constrain(double.infinity));
// After the layout, the canvas should have the above style applied.
expect(textContext.font, contains('40px'));
expect(textContext.font, contains('FontFamily2'));

p1.getBoxesForRange(0, 2);
// getBoxesForRange performs some text measurements. Let's make sure that it
// applied the correct style.
expect(textContext.font, contains('20px'));
expect(textContext.font, contains('FontFamily1'));

p2.getBoxesForRange(0, 4);
// getBoxesForRange performs some text measurements. Let's make sure that it
// applied the correct style.
expect(textContext.font, contains('40px'));
expect(textContext.font, contains('FontFamily2'));

ui.debugEmulateFlutterTesterEnvironment = true;
});
}