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

Reverts "Remove migration flag and unused header files" #50229

Merged
merged 1 commit into from
Feb 1, 2024
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: 2 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -5859,6 +5859,7 @@ ORIGIN: ../../../flutter/lib/ui/text/asset_manager_font_provider.cc + ../../../f
ORIGIN: ../../../flutter/lib/ui/text/asset_manager_font_provider.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/text/font_collection.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/text/font_collection.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/text/line_metrics.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/text/paragraph.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/text/paragraph.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/ui/text/paragraph_builder.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -8721,6 +8722,7 @@ FILE: ../../../flutter/lib/ui/text/asset_manager_font_provider.cc
FILE: ../../../flutter/lib/ui/text/asset_manager_font_provider.h
FILE: ../../../flutter/lib/ui/text/font_collection.cc
FILE: ../../../flutter/lib/ui/text/font_collection.h
FILE: ../../../flutter/lib/ui/text/line_metrics.h
FILE: ../../../flutter/lib/ui/text/paragraph.cc
FILE: ../../../flutter/lib/ui/text/paragraph.h
FILE: ../../../flutter/lib/ui/text/paragraph_builder.cc
Expand Down
1 change: 1 addition & 0 deletions lib/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ source_set("ui") {
"text/asset_manager_font_provider.h",
"text/font_collection.cc",
"text/font_collection.h",
"text/line_metrics.h",
"text/paragraph.cc",
"text/paragraph.h",
"text/paragraph_builder.cc",
Expand Down
22 changes: 20 additions & 2 deletions lib/ui/text.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3355,6 +3355,23 @@ abstract class ParagraphBuilder {
/// [Paragraph].
factory ParagraphBuilder(ParagraphStyle style) = _NativeParagraphBuilder;

/// Whether the rounding hack enabled by default in SkParagraph and TextPainter
/// is disabled.
///
/// Do not rely on this getter as it exists for migration purposes only and
/// will soon be removed.
@Deprecated('''
The shouldDisableRoundingHack flag is for internal migration purposes only and should not be used.
''')
static bool get shouldDisableRoundingHack => _shouldDisableRoundingHack;
static bool _shouldDisableRoundingHack = true;
/// Do not call this method as it is for migration purposes only and will soon
/// be removed.
// ignore: use_setters_to_change_properties
static void setDisableRoundingHack(bool disableRoundingHack) {
_shouldDisableRoundingHack = disableRoundingHack;
}

/// The number of placeholders currently in the paragraph.
int get placeholderCount;

Expand Down Expand Up @@ -3472,10 +3489,11 @@ base class _NativeParagraphBuilder extends NativeFieldWrapperClass1 implements P
style._height ?? 0,
style._ellipsis ?? '',
_encodeLocale(style._locale),
!ParagraphBuilder.shouldDisableRoundingHack,
);
}

@Native<Void Function(Handle, Handle, Handle, Handle, Handle, Double, Double, Handle, Handle)>(symbol: 'ParagraphBuilder::Create')
@Native<Void Function(Handle, Handle, Handle, Handle, Handle, Double, Double, Handle, Handle, Bool)>(symbol: 'ParagraphBuilder::Create')
external void _constructor(
Int32List encoded,
ByteData? strutData,
Expand All @@ -3485,7 +3503,7 @@ base class _NativeParagraphBuilder extends NativeFieldWrapperClass1 implements P
double height,
String ellipsis,
String locale,
);
bool applyRoundingHack);

@override
int get placeholderCount => _placeholderCount;
Expand Down
62 changes: 62 additions & 0 deletions lib/ui/text/line_metrics.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// 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.

#ifndef FLUTTER_LIB_UI_TEXT_LINE_METRICS_H_
#define FLUTTER_LIB_UI_TEXT_LINE_METRICS_H_

#include "third_party/dart/runtime/include/dart_api.h"
#include "third_party/tonic/converter/dart_converter.h"

namespace flutter {

struct LineMetrics {
const bool* hard_break;

// The final computed ascent and descent for the line. This can be impacted by
// the strut, height, scaling, as well as outlying runs that are very tall.
//
// The top edge is `baseline - ascent` and the bottom edge is `baseline +
// descent`. Ascent and descent are provided as positive numbers. Raw numbers
// for specific runs of text can be obtained in run_metrics_map. These values
// are the cumulative metrics for the entire line.
const double* ascent;
const double* descent;
const double* unscaled_ascent;
// Height of the line.
const double* height;
// Width of the line.
const double* width;
// The left edge of the line. The right edge can be obtained with `left +
// width`
const double* left;
// The y position of the baseline for this line from the top of the paragraph.
const double* baseline;
// Zero indexed line number.
const size_t* line_number;

LineMetrics();

LineMetrics(const bool* hard_break,
const double* ascent,
const double* descent,
const double* unscaled_ascent,
const double* height,
const double* width,
const double* left,
const double* baseline,
const size_t* line_number)
: hard_break(hard_break),
ascent(ascent),
descent(descent),
unscaled_ascent(unscaled_ascent),
height(height),
width(width),
left(left),
baseline(baseline),
line_number(line_number) {}
};

} // namespace flutter

#endif // FLUTTER_LIB_UI_TEXT_LINE_METRICS_H_
1 change: 1 addition & 0 deletions lib/ui/text/paragraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "flutter/fml/message_loop.h"
#include "flutter/lib/ui/dart_wrapper.h"
#include "flutter/lib/ui/painting/canvas.h"
#include "flutter/lib/ui/text/line_metrics.h"
#include "flutter/third_party/txt/src/txt/paragraph.h"

namespace flutter {
Expand Down
9 changes: 6 additions & 3 deletions lib/ui/text/paragraph_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,12 @@ void ParagraphBuilder::Create(Dart_Handle wrapper,
double fontSize,
double height,
const std::u16string& ellipsis,
const std::string& locale) {
const std::string& locale,
bool applyRoundingHack) {
UIDartState::ThrowIfUIOperationsProhibited();
auto res = fml::MakeRefCounted<ParagraphBuilder>(
encoded_handle, strutData, fontFamily, strutFontFamilies, fontSize,
height, ellipsis, locale);
height, ellipsis, locale, applyRoundingHack);
res->AssociateWithDartWrapper(wrapper);
}

Expand Down Expand Up @@ -230,7 +231,8 @@ ParagraphBuilder::ParagraphBuilder(
double fontSize,
double height,
const std::u16string& ellipsis,
const std::string& locale) {
const std::string& locale,
bool applyRoundingHack) {
int32_t mask = 0;
txt::ParagraphStyle style;
{
Expand Down Expand Up @@ -291,6 +293,7 @@ ParagraphBuilder::ParagraphBuilder(
if (mask & kPSLocaleMask) {
style.locale = locale;
}
style.apply_rounding_hack = applyRoundingHack;

FontCollection& font_collection = UIDartState::Current()
->platform_configuration()
Expand Down
6 changes: 4 additions & 2 deletions lib/ui/text/paragraph_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class ParagraphBuilder : public RefCountedDartWrappable<ParagraphBuilder> {
double fontSize,
double height,
const std::u16string& ellipsis,
const std::string& locale);
const std::string& locale,
bool applyRoundingHack);

~ParagraphBuilder() override;

Expand Down Expand Up @@ -76,7 +77,8 @@ class ParagraphBuilder : public RefCountedDartWrappable<ParagraphBuilder> {
double fontSize,
double height,
const std::u16string& ellipsis,
const std::string& locale);
const std::string& locale,
bool applyRoundingHack);

std::unique_ptr<txt::ParagraphBuilder> m_paragraph_builder_;
};
Expand Down
1 change: 1 addition & 0 deletions lib/web_ui/lib/src/engine/canvaskit/renderer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ class CanvasKitRenderer implements Renderer {
strutStyle: strutStyle,
ellipsis: ellipsis,
locale: locale,
applyRoundingHack: !ui.ParagraphBuilder.shouldDisableRoundingHack,
);

@override
Expand Down
5 changes: 4 additions & 1 deletion lib/web_ui/lib/src/engine/canvaskit/text.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class CkParagraphStyle implements ui.ParagraphStyle {
ui.StrutStyle? strutStyle,
String? ellipsis,
ui.Locale? locale,
bool applyRoundingHack = true,
}) : skParagraphStyle = toSkParagraphStyle(
textAlign,
textDirection,
Expand All @@ -46,6 +47,7 @@ class CkParagraphStyle implements ui.ParagraphStyle {
strutStyle,
ellipsis,
locale,
applyRoundingHack,
),
_textAlign = textAlign,
_textDirection = textDirection,
Expand Down Expand Up @@ -161,6 +163,7 @@ class CkParagraphStyle implements ui.ParagraphStyle {
ui.StrutStyle? strutStyle,
String? ellipsis,
ui.Locale? locale,
bool applyRoundingHack,
) {
final SkParagraphStyleProperties properties = SkParagraphStyleProperties();

Expand Down Expand Up @@ -197,7 +200,7 @@ class CkParagraphStyle implements ui.ParagraphStyle {
properties.replaceTabCharacters = true;
properties.textStyle = toSkTextStyleProperties(
fontFamily, fontSize, height, fontWeight, fontStyle);
properties.applyRoundingHack = false;
properties.applyRoundingHack = applyRoundingHack;

return canvasKit.ParagraphStyle(properties);
}
Expand Down
11 changes: 11 additions & 0 deletions lib/web_ui/lib/src/engine/text/canvas_paragraph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@ class CanvasParagraph implements ui.Paragraph {

@override
void layout(ui.ParagraphConstraints constraints) {
// When constraint width has a decimal place, we floor it to avoid getting
// a layout width that's higher than the constraint width.
//
// For example, if constraint width is `30.8` and the text has a width of
// `30.5` then the TextPainter in the framework will ceil the `30.5` width
// which will result in a width of `40.0` that's higher than the constraint
// width.
if (!ui.ParagraphBuilder.shouldDisableRoundingHack) {
constraints = ui.ParagraphConstraints(width: constraints.width.floorToDouble());
}

if (constraints == _lastUsedConstraints) {
return;
}
Expand Down
7 changes: 7 additions & 0 deletions lib/web_ui/lib/text.dart
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,13 @@ abstract class ParagraphBuilder {
factory ParagraphBuilder(ParagraphStyle style) =>
engine.renderer.createParagraphBuilder(style);

static bool get shouldDisableRoundingHack => _shouldDisableRoundingHack;
static bool _shouldDisableRoundingHack = true;
// ignore: use_setters_to_change_properties
static void setDisableRoundingHack(bool disableRoundingHack) {
_shouldDisableRoundingHack = disableRoundingHack;
}

void pushStyle(TextStyle style);
void pop();
void addText(String text);
Expand Down
30 changes: 29 additions & 1 deletion lib/web_ui/test/canvaskit/text_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,9 @@ void testMain() {
expect(bottomRight?.writingDirection, ui.TextDirection.ltr);
});

test('rounding hack disabled', () {
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);
Expand All @@ -224,6 +226,32 @@ void testMain() {
}
});

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);
}
15 changes: 15 additions & 0 deletions lib/web_ui/test/html/text/canvas_paragraph_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,21 @@ Future<void> testMain() async {
expect(paragraph.longestLine, 50.0);
});

test('$CanvasParagraph.width should be a whole integer when shouldDisableRoundingHack is false', () {
if (ui.ParagraphBuilder.shouldDisableRoundingHack) {
ui.ParagraphBuilder.setDisableRoundingHack(false);
addTearDown(() => ui.ParagraphBuilder.setDisableRoundingHack(true));
}
// The paragraph width is only rounded to a whole integer if
// shouldDisableRoundingHack is false.
assert(!ui.ParagraphBuilder.shouldDisableRoundingHack);
final ui.Paragraph paragraph = plain(ahemStyle, 'abc');
paragraph.layout(const ui.ParagraphConstraints(width: 30.8));

expect(paragraph.width, 30);
expect(paragraph.height, 10);
});

test('Render after dispose', () async {
final ui.Paragraph paragraph = plain(ahemStyle, 'abc');
paragraph.layout(const ui.ParagraphConstraints(width: 30.8));
Expand Down
7 changes: 6 additions & 1 deletion lib/web_ui/test/html/text_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,12 @@ Future<void> testMain() async {
expect(bottomRight?.graphemeClusterLayoutBounds, const Rect.fromLTWH(0.0, 0.0, 10.0, 10.0));
}, skip: domIntl.v8BreakIterator == null); // Intended: Intl.v8breakiterator is needed for correctly breaking grapheme clusters.

test('disable rounding hack', () {
test('Can disable rounding hack', () {
if (!ParagraphBuilder.shouldDisableRoundingHack) {
ParagraphBuilder.setDisableRoundingHack(true);
addTearDown(() => ParagraphBuilder.setDisableRoundingHack(false));
}
assert(ParagraphBuilder.shouldDisableRoundingHack);
const double fontSize = 1;
const String text = '12345';
const double letterSpacing = 0.25;
Expand Down
34 changes: 33 additions & 1 deletion testing/dart/paragraph_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,42 @@ void main() {
}
});

test('rounding hack disabled', () {
test('can set disableRoundingHack to false in tests', () {
bool assertsEnabled = false;
assert(() {
assertsEnabled = true;
return true;
}());
if (!assertsEnabled){
return;
}
const double fontSize = 1.25;
const String text = '12345';
assert((fontSize * text.length).truncate() != fontSize * text.length);
// ignore: deprecated_member_use
final bool roundingHackWasDisabled = ParagraphBuilder.shouldDisableRoundingHack;
if (roundingHackWasDisabled) {
ParagraphBuilder.setDisableRoundingHack(false);
}
// ignore: deprecated_member_use
assert(!ParagraphBuilder.shouldDisableRoundingHack);
final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(fontSize: fontSize));
builder.addText(text);
final Paragraph paragraph = builder.build()
..layout(const ParagraphConstraints(width: text.length * fontSize));
expect(paragraph.computeLineMetrics().length, greaterThan(1));

if (roundingHackWasDisabled) {
ParagraphBuilder.setDisableRoundingHack(true);
}
});

test('rounding hack disabled by default', () {
const double fontSize = 1.25;
const String text = '12345';
assert((fontSize * text.length).truncate() != fontSize * text.length);
// ignore: deprecated_member_use
expect(ParagraphBuilder.shouldDisableRoundingHack, isTrue);
final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(fontSize: fontSize));
builder.addText(text);
final Paragraph paragraph = builder.build()
Expand Down
2 changes: 1 addition & 1 deletion third_party/txt/src/skia/paragraph_builder_skia.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ skt::ParagraphStyle ParagraphBuilderSkia::TxtToSkia(const ParagraphStyle& txt) {

skia.turnHintingOff();
skia.setReplaceTabCharacters(true);
skia.setApplyRoundingHack(false);
skia.setApplyRoundingHack(txt.apply_rounding_hack);

return skia;
}
Expand Down
Loading