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.

Sign up for GitHub

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

Revert "[web] Correct getPositionForOffset for multi-line paragraphs (#16206)" #16268

Merged
merged 1 commit into from
Jan 31, 2020
Merged

Revert "[web] Correct getPositionForOffset for multi-line paragraphs (#16206)" #16268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 1 addition & 4 deletions lib/web_ui/lib/src/engine/text/measurement.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,10 +373,7 @@ class DomTextMeasurementService extends TextMeasurementService {
@override
ui.TextPosition getTextPositionForOffset(EngineParagraph paragraph,
ui.ParagraphConstraints constraints, ui.Offset offset) {
assert(
paragraph._measurementResult.lines == null,
'should only be called when the faster lines-based approach is not possible',
);
assert(paragraph._plainText == null, 'should only be called for multispan');

final ParagraphGeometricStyle style = paragraph._geometricStyle;
final ParagraphRuler ruler =
Expand Down
48 changes: 6 additions & 42 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 @@ -378,53 +378,17 @@ class EngineParagraph implements ui.Paragraph {

@override
ui.TextPosition getPositionForOffset(ui.Offset offset) {
final List<EngineLineMetrics> lines = _measurementResult.lines;
if (lines == null) {
if (_plainText == null) {
return getPositionForMultiSpanOffset(offset);
}

final int lineNumber = offset.dy ~/ _measurementResult.lineHeight;

// [offset] is below all the lines.
if (lineNumber >= lines.length) {
return ui.TextPosition(
offset: _plainText.length,
affinity: ui.TextAffinity.upstream,
);
}

final EngineLineMetrics lineMetrics = lines[lineNumber];
final double lineLeft = lineMetrics.left;
final double lineRight = lineLeft + lineMetrics.width;

// [offset] is to the left of the line.
if (offset.dx <= lineLeft) {
return ui.TextPosition(
offset: lineMetrics.startIndex,
affinity: ui.TextAffinity.downstream,
);
}

// [offset] is to the right of the line.
if (offset.dx >= lineRight) {
return ui.TextPosition(
offset: lineMetrics.endIndex,
affinity: ui.TextAffinity.upstream,
);
}

// If we reach here, it means the [offset] is somewhere within the line. The
// code below will do a binary search to find where exactly the [offset]
// falls within the line.

final double dx = offset.dx - _alignOffset;
final TextMeasurementService instance = _measurementService;

int low = lineMetrics.startIndex;
int high = lineMetrics.endIndex;
int low = 0;
int high = _plainText.length;
do {
final int current = (low + high) ~/ 2;
final double width = instance.measureSubstringWidth(this, lineMetrics.startIndex, current);
final double width = instance.measureSubstringWidth(this, 0, current);
if (width < dx) {
low = current;
} else if (width > dx) {
Expand All @@ -439,8 +403,8 @@ class EngineParagraph implements ui.Paragraph {
return ui.TextPosition(offset: high, affinity: ui.TextAffinity.upstream);
}

final double lowWidth = instance.measureSubstringWidth(this, lineMetrics.startIndex, low);
final double highWidth = instance.measureSubstringWidth(this, lineMetrics.startIndex, high);
final double lowWidth = instance.measureSubstringWidth(this, 0, low);
final double highWidth = instance.measureSubstringWidth(this, 0, high);

if (dx - lowWidth < highWidth - dx) {
// The offset is closer to the low index.
Expand Down
150 changes: 1 addition & 149 deletions lib/web_ui/test/paragraph_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import 'package:ui/ui.dart';
import 'package:test/test.dart';

void testEachMeasurement(String description, VoidCallback body, {bool skip}) {
test('$description (dom measurement)', () async {
test(description, () async {
try {
TextMeasurementService.initialize(rulerCacheCapacity: 2);
return body();
Expand Down Expand Up @@ -130,154 +130,6 @@ void main() async {
}, // TODO(nurhan): https://github.com/flutter/flutter/issues/46638
skip: (browserEngine == BrowserEngine.firefox));

testEachMeasurement('getPositionForOffset single-line', () {
final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(
fontFamily: 'Ahem',
fontStyle: FontStyle.normal,
fontWeight: FontWeight.normal,
fontSize: 10,
textDirection: TextDirection.ltr,
));
builder.addText('abcd efg');
final Paragraph paragraph = builder.build();
paragraph.layout(const ParagraphConstraints(width: 1000));

// At the beginning of the line.
expect(
paragraph.getPositionForOffset(Offset(0, 5)),
TextPosition(offset: 0, affinity: TextAffinity.downstream),
);
// Below the line.
expect(
paragraph.getPositionForOffset(Offset(0, 12)),
TextPosition(offset: 8, affinity: TextAffinity.upstream),
);
// Above the line.
expect(
paragraph.getPositionForOffset(Offset(0, -5)),
TextPosition(offset: 0, affinity: TextAffinity.downstream),
);
// At the end of the line.
expect(
paragraph.getPositionForOffset(Offset(80, 5)),
TextPosition(offset: 8, affinity: TextAffinity.upstream),
);
// On the left side of "b".
expect(
paragraph.getPositionForOffset(Offset(14, 5)),
TextPosition(offset: 1, affinity: TextAffinity.downstream),
);
// On the right side of "b".
expect(
paragraph.getPositionForOffset(Offset(16, 5)),
TextPosition(offset: 2, affinity: TextAffinity.upstream),
);
});

test('getPositionForOffset multi-line', () {
// [Paragraph.getPositionForOffset] for multi-line text doesn't work well
// with dom-based measurement.
TextMeasurementService.enableExperimentalCanvasImplementation = true;
TextMeasurementService.initialize(rulerCacheCapacity: 2);

final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(
fontFamily: 'Ahem',
fontStyle: FontStyle.normal,
fontWeight: FontWeight.normal,
fontSize: 10,
textDirection: TextDirection.ltr,
));
builder.addText('abcd\n');
builder.addText('abcdefg\n');
builder.addText('ab');
final Paragraph paragraph = builder.build();
paragraph.layout(const ParagraphConstraints(width: 1000));

// First line: "abcd\n"

// At the beginning of the first line.
expect(
paragraph.getPositionForOffset(Offset(0, 5)),
TextPosition(offset: 0, affinity: TextAffinity.downstream),
);
// Above the first line.
expect(
paragraph.getPositionForOffset(Offset(0, -5)),
TextPosition(offset: 0, affinity: TextAffinity.downstream),
);
// At the end of the first line.
expect(
paragraph.getPositionForOffset(Offset(50, 5)),
TextPosition(offset: 5, affinity: TextAffinity.upstream),
);
// On the left side of "b" in the first line.
expect(
paragraph.getPositionForOffset(Offset(14, 5)),
TextPosition(offset: 1, affinity: TextAffinity.downstream),
);
// On the right side of "b" in the first line.
expect(
paragraph.getPositionForOffset(Offset(16, 5)),
TextPosition(offset: 2, affinity: TextAffinity.upstream),
);


// Second line: "abcdefg\n"

// At the beginning of the second line.
expect(
paragraph.getPositionForOffset(Offset(0, 15)),
TextPosition(offset: 5, affinity: TextAffinity.downstream),
);
// At the end of the second line.
expect(
paragraph.getPositionForOffset(Offset(100, 15)),
TextPosition(offset: 13, affinity: TextAffinity.upstream),
);
// On the left side of "e" in the second line.
expect(
paragraph.getPositionForOffset(Offset(44, 15)),
TextPosition(offset: 9, affinity: TextAffinity.downstream),
);
// On the right side of "e" in the second line.
expect(
paragraph.getPositionForOffset(Offset(46, 15)),
TextPosition(offset: 10, affinity: TextAffinity.upstream),
);


// Last (third) line: "ab"

// At the beginning of the last line.
expect(
paragraph.getPositionForOffset(Offset(0, 25)),
TextPosition(offset: 13, affinity: TextAffinity.downstream),
);
// At the end of the last line.
expect(
paragraph.getPositionForOffset(Offset(40, 25)),
TextPosition(offset: 15, affinity: TextAffinity.upstream),
);
// Below the last line.
expect(
paragraph.getPositionForOffset(Offset(0, 32)),
TextPosition(offset: 15, affinity: TextAffinity.upstream),
);
// On the left side of "b" in the last line.
expect(
paragraph.getPositionForOffset(Offset(12, 25)),
TextPosition(offset: 14, affinity: TextAffinity.downstream),
);
// On the right side of "a" in the last line.
expect(
paragraph.getPositionForOffset(Offset(9, 25)),
TextPosition(offset: 14, affinity: TextAffinity.upstream),
);

TextMeasurementService.clearCache();
TextMeasurementService.enableExperimentalCanvasImplementation = false;
});

testEachMeasurement('getBoxesForRange returns a box', () {
final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(
fontFamily: 'Ahem',
Expand Down