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

Add line boundary information to LineMetrics. #13727

Merged
merged 5 commits into from
Nov 12, 2019
Merged

Add line boundary information to LineMetrics. #13727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f38dfdc
Add line boundary information to Paragraph
gspencergoog Nov 7, 2019
d44bbcd
Add web API
gspencergoog Nov 7, 2019
3f534dc
Changing return type for getWordBoundary to TextRange
gspencergoog Nov 9, 2019
48502a1
Move to TextPosition, remove extra fields on LineMetrics (for now)
gspencergoog Nov 11, 2019
a249d84
Update comments.
gspencergoog Nov 12, 2019
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
37 changes: 23 additions & 14 deletions lib/ui/text.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1866,23 +1866,32 @@ class Paragraph extends NativeFieldWrapperClass2 {
}
List<int> _getPositionForOffset(double dx, double dy) native 'Paragraph_getPositionForOffset';

/// Returns the [start, end] of the word at the given offset. Characters not
/// part of a word, such as spaces, symbols, and punctuation, have word breaks
/// on both sides. In such cases, this method will return [offset, offset+1].
/// Word boundaries are defined more precisely in Unicode Standard Annex #29
/// http://www.unicode.org/reports/tr29/#Word_Boundaries
List<int> getWordBoundary(dynamic position) {
// TODO(gspencergoog): have this take only a TextPosition once the framework
// code is calling it with that.
if (position is TextPosition) {
return _getWordBoundary(position.offset);
} else {
final int offset = position;
return _getWordBoundary(offset);
}
/// Returns the [TextRange] of the word at the given [TextPosition].
///
/// Characters not part of a word, such as spaces, symbols, and punctuation,
/// have word breaks on both sides. In such cases, this method will return
/// [offset, offset+1]. Word boundaries are defined more precisely in Unicode
/// Standard Annex #29 http://www.unicode.org/reports/tr29/#Word_Boundaries
TextRange getWordBoundary(TextPosition position) {
final List<int> boundary = _getWordBoundary(position.offset);
return TextRange(start: boundary[0], end: boundary[1]);
}
List<int> _getWordBoundary(int offset) native 'Paragraph_getWordBoundary';

/// Returns the [TextRange] of the line at the given [TextPosition].
///
/// The newline (if any) is returned as part of the range.
///
/// Not valid until after layout.
///
/// This can potentially be expensive, since it needs to compute the line
/// metrics, so use it sparingly.
TextRange getLineBoundary(TextPosition position) {
final List<int> boundary = _getLineBoundary(position.offset);
return TextRange(start: boundary[0], end: boundary[1]);
}
List<int> _getLineBoundary(int offset) native 'Paragraph_getLineBoundary';
Copy link
Contributor

@mdebbar mdebbar Nov 11, 2019

Choose a reason for hiding this comment

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

Is there a reason this is implemented in C++ and not in Dart directly? (Implementing it in Dart would reduce unnecessary work since web can also use the same implementation).

Copy link
Contributor Author

@gspencergoog gspencergoog Nov 11, 2019

Choose a reason for hiding this comment

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

I don't have the history on why that is, but I assume the reason is that we didn't want to have to implement, test, and maintain the code, and it already existed in the text library. If that's the case, then I suspect that with the advent of web, it makes sense to make the investment and implement, test, and maintain it (since we'll need to do that anyhow for web).

It's possible that it is somehow linked to the text processing library that we use, though, so I'll defer to the engine team on that.

cc/ @jason-simmons @GaryQian

Copy link
Contributor

@GaryQian GaryQian Nov 11, 2019

Choose a reason for hiding this comment

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

In this case, it would be far more efficient to compute it in C++ instead of serializing and passing all the LineMetrics to dart, decoding, and processing it there.

This also keeps implementations consistent throughout the Paragraph API. Methods such as GetRectsForRange requires a significant computation that can only be done at engine level, and splitting implementation between dart and C++ would make it harder to maintain.


// Redirecting the paint function in this way solves some dependency problems
// in the C++ code. If we straighten out the C++ dependencies, we can remove
// this indirection.
Expand Down
18 changes: 18 additions & 0 deletions lib/ui/text/paragraph.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, Paragraph);
V(Paragraph, layout) \
V(Paragraph, paint) \
V(Paragraph, getWordBoundary) \
V(Paragraph, getLineBoundary) \
V(Paragraph, getRectsForRange) \
V(Paragraph, getRectsForPlaceholders) \
V(Paragraph, getPositionForOffset) \
Expand Down Expand Up @@ -134,6 +135,23 @@ Dart_Handle Paragraph::getWordBoundary(unsigned offset) {
return result;
}

Dart_Handle Paragraph::getLineBoundary(unsigned offset) {
std::vector<txt::LineMetrics> metrics = m_paragraph->GetLineMetrics();
int line_start = -1;
int line_end = -1;
for (txt::LineMetrics& line : metrics) {
if (offset >= line.start_index && offset < line.end_index) {
line_start = line.start_index;
line_end = line.end_index;
break;
}
}
Dart_Handle result = Dart_NewListOf(Dart_CoreType_Int, 2);
Dart_ListSetAt(result, 0, ToDart(line_start));
Dart_ListSetAt(result, 1, ToDart(line_end));
return result;
}

std::vector<LineMetrics> Paragraph::computeLineMetrics() {
std::vector<LineMetrics> result;
std::vector<txt::LineMetrics> metrics = m_paragraph->GetLineMetrics();
Expand Down
1 change: 1 addition & 0 deletions lib/ui/text/paragraph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class Paragraph : public RefCountedDartWrappable<Paragraph> {
std::vector<TextBox> getRectsForPlaceholders();
Dart_Handle getPositionForOffset(double dx, double dy);
Dart_Handle getWordBoundary(unsigned offset);
Dart_Handle getLineBoundary(unsigned offset);
std::vector<LineMetrics> computeLineMetrics();

size_t GetAllocationSize() override;
Expand Down
31 changes: 13 additions & 18 deletions lib/web_ui/lib/src/engine/text/paragraph.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,27 +281,22 @@ class EngineParagraph implements ui.Paragraph {
}

@override
List<int> getWordBoundary(dynamic position) {
// TODO(gspencergoog): have this take only a TextPosition once the framework
// code is calling it with that.
if (position is ui.TextPosition) {
ui.TextPosition textPosition = position;
if (_plainText == null) {
return <int>[textPosition.offset, textPosition.offset];
}

final int start = WordBreaker.prevBreakIndex(_plainText, textPosition.offset);
final int end = WordBreaker.nextBreakIndex(_plainText, textPosition.offset);
return <int>[start, end];
}

ui.TextRange getWordBoundary(ui.TextPosition position) {
ui.TextPosition textPosition = position;
if (_plainText == null) {
return <int>[position, position];
return ui.TextRange(start: textPosition.offset, end: textPosition.offset);
}

final int start = WordBreaker.prevBreakIndex(_plainText, position);
final int end = WordBreaker.nextBreakIndex(_plainText, position);
return <int>[start, end];
final int start = WordBreaker.prevBreakIndex(_plainText, textPosition.offset);
final int end = WordBreaker.nextBreakIndex(_plainText, textPosition.offset);
return ui.TextRange(start: start, end: end);
}

@override
ui.TextRange getLineBoundary(ui.TextPosition position) {
// TODO(flutter_web): https://github.com/flutter/flutter/issues/39537
// Depends upon LineMetrics measurement.
return null;
}

@override
Expand Down
30 changes: 24 additions & 6 deletions lib/web_ui/lib/src/ui/text.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1237,12 +1237,23 @@ abstract class Paragraph {
/// within the text.
TextPosition getPositionForOffset(Offset offset);

/// Returns the [start, end] of the word at the given offset. Characters not
/// part of a word, such as spaces, symbols, and punctuation, have word breaks
/// on both sides. In such cases, this method will return [offset, offset+1].
/// Word boundaries are defined more precisely in Unicode Standard Annex #29
/// http://www.unicode.org/reports/tr29/#Word_Boundaries
List<int> getWordBoundary(dynamic position);
/// Returns the [TextRange] of the word at the given [TextPosition].
///
/// Characters not part of a word, such as spaces, symbols, and punctuation,
/// have word breaks on both sides. In such cases, this method will return
/// [offset, offset+1]. Word boundaries are defined more precisely in Unicode
/// Standard Annex #29 http://www.unicode.org/reports/tr29/#Word_Boundaries
TextRange getWordBoundary(TextPosition position);

/// Returns the [TextRange] of the line at the given [TextPosition].
///
/// The newline (if any) is returned as part of the range.
///
/// Not valid until after layout.
///
/// This can potentially be expensive, since it needs to compute the line
Copy link
Contributor

@GaryQian GaryQian Nov 11, 2019

Choose a reason for hiding this comment

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

The line metrics are computed as part of layout, so as long as it is used after layout, this should not be particularly expensive. We currently don't do any additional computation beyond that of layout to calculate the LineMetrics.

What should be mentioned though is that this is invalid before layout.

Copy link
Contributor

@GaryQian GaryQian Nov 11, 2019

Choose a reason for hiding this comment

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

Actually, this warning can remain, as it depends on the implementation. Web or SkShaper may have more expensive implementations. It just so happens LibTxt computes this in layout.

/// metrics, so use it sparingly.
TextRange getLineBoundary(TextPosition position);

/// Returns a list of text boxes that enclose all placeholders in the paragraph.
///
Expand All @@ -1252,6 +1263,13 @@ abstract class Paragraph {
/// where positive y values indicate down.
List<TextBox> getBoxesForPlaceholders();

/// Returns the full list of [LineMetrics] that describe in detail the various
/// metrics of each laid out line.
///
/// Not valid until after layout.
///
/// This can potentially return a large amount of data, so it is not recommended
/// to repeatedly call this. Instead, cache the results.
List<LineMetrics> computeLineMetrics();
}

Expand Down