remove usage of Dart_New for paragraph/libtxt #16837
Conversation
lib/ui/text.dart
Outdated
| @@ -1914,6 +1890,20 @@ class Paragraph extends NativeFieldWrapperClass2 { | |||
| void layout(ParagraphConstraints constraints) => _layout(constraints.width); | |||
| void _layout(double width) native 'Paragraph_layout'; | |||
|
|
|||
| List<TextBox> _decodeTextBoxes(Float32List encoded) { | |||
| final List<TextBox> boxes = List<TextBox>(encoded[0].toInt()); | |||
There was a problem hiding this comment.
The count of text boxes is redundant with the length of the encoded Float32List
lib/ui/text.dart
Outdated
| List<TextBox> _decodeTextBoxes(Float32List encoded) { | ||
| final List<TextBox> boxes = List<TextBox>(encoded[0].toInt()); | ||
| for (int index = 0; index < boxes.length; index += 1) { | ||
| final int position = (index * 5) + 1; |
There was a problem hiding this comment.
Avoid the cost of the multiply by advancing the index by 5 on each iteration
lib/ui/text.dart
Outdated
| final Float64List encoded = _computeLineMetrics(); | ||
| final List<LineMetrics> metrics = List<LineMetrics>(encoded[0].toInt()); | ||
| for (int index = 0; index < metrics.length; index += 1) { | ||
| final int position = (index * 5) + 1; |
There was a problem hiding this comment.
Advance the position by 9 instead of 5 for LineMetrics
There was a problem hiding this comment.
Done, and added some tests that didn't exist that would have caught this.
lib/ui/text/paragraph.cc
Outdated
| for (unsigned long i = 0; i < metrics.size(); i++) { | ||
| auto position = (i * 9) + 1; | ||
| const txt::LineMetrics& line = metrics[i]; | ||
| result[position + 0] = static_cast<float>(line.hard_break); |
There was a problem hiding this comment.
cast this to double to match the type of result
|
Also noticed that we could make LineMetrics const constructable. Not sure how crucial that is since the real use case for it couldn't use it. At any rate I added hashCode and == and toString overrides for LineMetrics, they were helpful to me in writing tests. |
|
If web has a lineMetrics you should add the logic there too, plus const construction if you decide on it. |
|
@jonahwilliams - strangely, the web I don't want to make this const right now because I'm not sure where it would be useful and it's entirely possible it turns out to be breaking for someone that's constructing it in a non const way where it could have been const. We can do that in a separate PR if desired. |
|
I'm going to merge this on the idea that the test needs the same logic to pass without these changes - if it's a problem we should fix it separately. |
Part of flutter/flutter#50997
TL;DR is that Dart_New is slow and we should try to avoid it.
After this it should just be images.