Make GetRectsForRange provide more detailed/nuanced metrics through RectStyle enum. #6335

GaryQian · 2018-09-25T20:24:00Z

This PR adds the advanced GetRectsForRange functionality to LibTxt, but does not yet enable it in the framework because this would change the current behavior.

RectStyle provides an OR-able enum that defines a number of properties that the rects will have.

GaryQian · 2018-09-25T20:24:58Z

third_party/txt/src/txt/paragraph.cc

                          glyph_positions.back().x_pos.end),
            line_number, metrics, run.direction());
-      }
+      }  // for each in glyph_blobs


A brief description of the for loops that this brace encloses. This method is long and this helps make sense of it.

third_party/txt/src/txt/paragraph.cc

…ctStyle enum.

Hixie · 2018-09-26T23:01:03Z

third_party/txt/src/txt/paragraph.cc

+    } else {
+      // Set each box to the max height of each line to ensure continuity.
+      float min_top = DBL_MAX;
+      float max_bottom = -1;


Any value smaller than zero should do, since the bottom of it cannot be above the top, and the highest the top can be is zero.

why not zero, then?

third_party/txt/src/txt/paragraph.h

Hixie · 2018-09-26T23:02:22Z

Is the plan to expose this in the dart:ui API?

GaryQian · 2018-09-26T23:07:26Z

Yes, we would make a similar enum system in the Dart-side Paragraph class' getRectsForRange(), and default the optional enum to the current behavior. Currently, I have it defaulted in the C++ layer and have not exposed it at all to dart.

GaryQian · 2018-10-01T23:31:26Z

@jason-simmons @Hixie Any feedback/additional suggestions?

Hixie · 2018-10-01T23:37:05Z

i'll defer to jason.

flutter/engine@b59c864...3a9c22a git log b59c864..3a9c22a --no-merges --oneline 3a9c22a Allow GetRectsForRange to provide more detailed/nuanced metrics through RectStyle enum. (flutter/engine#6335) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.

…gh RectStyle enum. (flutter#6335)

flutter/engine@f6af1f2...44a5149 git log f6af1f2..44a5149 --no-merges --oneline 44a5149 Roll src/third_party/skia 7dae882574d2..cc4dbfcfbd8a (15 commits) (flutter/engine#6405) de32c65 Roll src/third_party/skia 3b8b11e1f912..7dae882574d2 (15 commits) (flutter/engine#6403) 3a9c22a Allow GetRectsForRange to provide more detailed/nuanced metrics through RectStyle enum. (flutter/engine#6335) b59c864 Rename the Android fragment support library JAR (flutter/engine#6400) 4213ac1 Add an Android fragment support library to third_party (flutter/engine#6384) a785b25 do not count Hidden nodes at the beginning of the scrollable (flutter/engine#6381) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@f6af1f2...dc2634d git log f6af1f2..dc2634d --no-merges --oneline dc2634d Force lf-line endings for so that source offsets match across platforms. (flutter/engine#6408) 44a5149 Roll src/third_party/skia 7dae882574d2..cc4dbfcfbd8a (15 commits) (flutter/engine#6405) de32c65 Roll src/third_party/skia 3b8b11e1f912..7dae882574d2 (15 commits) (flutter/engine#6403) 3a9c22a Allow GetRectsForRange to provide more detailed/nuanced metrics through RectStyle enum. (flutter/engine#6335) b59c864 Rename the Android fragment support library JAR (flutter/engine#6400) 4213ac1 Add an Android fragment support library to third_party (flutter/engine#6384) a785b25 do not count Hidden nodes at the beginning of the scrollable (flutter/engine#6381) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@f6af1f2...fa719e3 git log f6af1f2..fa719e3 --no-merges --oneline fa719e3 Roll src/third_party/skia cc4dbfcfbd8a..6719fcc43b1e (17 commits) (flutter/engine#6409) dc2634d Force lf-line endings for so that source offsets match across platforms. (flutter/engine#6408) 44a5149 Roll src/third_party/skia 7dae882574d2..cc4dbfcfbd8a (15 commits) (flutter/engine#6405) de32c65 Roll src/third_party/skia 3b8b11e1f912..7dae882574d2 (15 commits) (flutter/engine#6403) 3a9c22a Allow GetRectsForRange to provide more detailed/nuanced metrics through RectStyle enum. (flutter/engine#6335) b59c864 Rename the Android fragment support library JAR (flutter/engine#6400) 4213ac1 Add an Android fragment support library to third_party (flutter/engine#6384) a785b25 do not count Hidden nodes at the beginning of the scrollable (flutter/engine#6381) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@f6af1f2...9ccc966 git log f6af1f2..9ccc966 --no-merges --oneline 9ccc966 Share engine layers with the framework (flutter/engine#6406) fa719e3 Roll src/third_party/skia cc4dbfcfbd8a..6719fcc43b1e (17 commits) (flutter/engine#6409) dc2634d Force lf-line endings for so that source offsets match across platforms. (flutter/engine#6408) 44a5149 Roll src/third_party/skia 7dae882574d2..cc4dbfcfbd8a (15 commits) (flutter/engine#6405) de32c65 Roll src/third_party/skia 3b8b11e1f912..7dae882574d2 (15 commits) (flutter/engine#6403) 3a9c22a Allow GetRectsForRange to provide more detailed/nuanced metrics through RectStyle enum. (flutter/engine#6335) b59c864 Rename the Android fragment support library JAR (flutter/engine#6400) 4213ac1 Add an Android fragment support library to third_party (flutter/engine#6384) a785b25 do not count Hidden nodes at the beginning of the scrollable (flutter/engine#6381) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@f6af1f2...2de87a8 git log f6af1f2..2de87a8 --no-merges --oneline 2de87a8 Roll src/third_party/skia 6719fcc43b1e..deb512045e6a (7 commits) (flutter/engine#6411) 68a42e3 Add nullability annotations to MethodChannel/MethodCall. (flutter/engine#6393) 71ba20a Dart SDK roll for 2018-10-02 to d2c5a24fd9ead97a7f18d02786e679293cc3709e (flutter/engine#6410) 9ccc966 Share engine layers with the framework (flutter/engine#6406) fa719e3 Roll src/third_party/skia cc4dbfcfbd8a..6719fcc43b1e (17 commits) (flutter/engine#6409) dc2634d Force lf-line endings for so that source offsets match across platforms. (flutter/engine#6408) 44a5149 Roll src/third_party/skia 7dae882574d2..cc4dbfcfbd8a (15 commits) (flutter/engine#6405) de32c65 Roll src/third_party/skia 3b8b11e1f912..7dae882574d2 (15 commits) (flutter/engine#6403) 3a9c22a Allow GetRectsForRange to provide more detailed/nuanced metrics through RectStyle enum. (flutter/engine#6335) b59c864 Rename the Android fragment support library JAR (flutter/engine#6400) 4213ac1 Add an Android fragment support library to third_party (flutter/engine#6384) a785b25 do not count Hidden nodes at the beginning of the scrollable (flutter/engine#6381) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@f6af1f2...74662ab git log f6af1f2..74662ab --no-merges --oneline 74662ab Revert "Share engine layers with the framework" (flutter/engine#6412) 2de87a8 Roll src/third_party/skia 6719fcc43b1e..deb512045e6a (7 commits) (flutter/engine#6411) 68a42e3 Add nullability annotations to MethodChannel/MethodCall. (flutter/engine#6393) 71ba20a Dart SDK roll for 2018-10-02 to d2c5a24fd9ead97a7f18d02786e679293cc3709e (flutter/engine#6410) 9ccc966 Share engine layers with the framework (flutter/engine#6406) fa719e3 Roll src/third_party/skia cc4dbfcfbd8a..6719fcc43b1e (17 commits) (flutter/engine#6409) dc2634d Force lf-line endings for so that source offsets match across platforms. (flutter/engine#6408) 44a5149 Roll src/third_party/skia 7dae882574d2..cc4dbfcfbd8a (15 commits) (flutter/engine#6405) de32c65 Roll src/third_party/skia 3b8b11e1f912..7dae882574d2 (15 commits) (flutter/engine#6403) 3a9c22a Allow GetRectsForRange to provide more detailed/nuanced metrics through RectStyle enum. (flutter/engine#6335) b59c864 Rename the Android fragment support library JAR (flutter/engine#6400) 4213ac1 Add an Android fragment support library to third_party (flutter/engine#6384) a785b25 do not count Hidden nodes at the beginning of the scrollable (flutter/engine#6381) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.

GaryQian requested a review from jason-simmons September 25, 2018 20:24

googlebot added the cla: yes label Sep 25, 2018

GaryQian commented Sep 25, 2018

View reviewed changes

GaryQian force-pushed the textrects branch from 89617eb to 0dddc16 Compare September 25, 2018 20:40

GaryQian requested review from cbracken and chinmaygarde September 25, 2018 20:56

jason-simmons reviewed Sep 25, 2018

View reviewed changes

third_party/txt/src/txt/paragraph.cc Show resolved Hide resolved

Let GetRectsForRange provide more detailed/nuanced metrics through Re…

ed0b191

…ctStyle enum.

GaryQian force-pushed the textrects branch from 0dddc16 to ed0b191 Compare September 26, 2018 00:23

GaryQian mentioned this pull request Sep 26, 2018

Text selection vertical extent should be defined per line (not per glyph) flutter/flutter#21997

Open

Hixie reviewed Sep 26, 2018

View reviewed changes

third_party/txt/src/txt/paragraph.h Show resolved Hide resolved

0 instead of -1

d06b879

Remove Unused enums from RectStyle

3c1396d

jason-simmons approved these changes Oct 1, 2018

View reviewed changes

GaryQian merged commit 3a9c22a into flutter:master Oct 2, 2018

engine-flutter-autoroll mentioned this pull request Oct 2, 2018

Roll engine b59c86493159..3a9c22afde67 (1 commits) flutter/flutter#22541

Closed

GaryQian added a commit to GaryQian/engine that referenced this pull request Oct 2, 2018

Allow GetRectsForRange to provide more detailed/nuanced metrics throu…

bfe2b33

…gh RectStyle enum. (flutter#6335)

engine-flutter-autoroll mentioned this pull request Oct 2, 2018

Roll engine f6af1f20ba92..44a514943206 (6 commits) flutter/flutter#22567

Closed

engine-flutter-autoroll mentioned this pull request Oct 2, 2018

Roll engine f6af1f20ba92..dc2634dcf70e (7 commits) flutter/flutter#22577

Closed

engine-flutter-autoroll mentioned this pull request Oct 2, 2018

Roll engine f6af1f20ba92..fa719e3e7a51 (8 commits) flutter/flutter#22582

Closed

engine-flutter-autoroll mentioned this pull request Oct 2, 2018

Roll engine f6af1f20ba92..9ccc96625f0a (9 commits) flutter/flutter#22585

Closed

engine-flutter-autoroll mentioned this pull request Oct 3, 2018

Roll engine f6af1f20ba92..2de87a80f550 (12 commits) flutter/flutter#22597

Closed

engine-flutter-autoroll mentioned this pull request Oct 3, 2018

Roll engine f6af1f20ba92..74662ab69523 (13 commits) flutter/flutter#22599

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Make GetRectsForRange provide more detailed/nuanced metrics through RectStyle enum. #6335

Make GetRectsForRange provide more detailed/nuanced metrics through RectStyle enum. #6335

Uh oh!

GaryQian commented Sep 25, 2018

Uh oh!

GaryQian Sep 25, 2018

Uh oh!

Uh oh!

Hixie Sep 26, 2018

Uh oh!

GaryQian Sep 26, 2018

Uh oh!

Hixie Sep 27, 2018

Uh oh!

Uh oh!

Hixie commented Sep 26, 2018

Uh oh!

GaryQian commented Sep 26, 2018

Uh oh!

GaryQian commented Oct 1, 2018

Uh oh!

Hixie commented Oct 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Make GetRectsForRange provide more detailed/nuanced metrics through RectStyle enum. #6335

Make GetRectsForRange provide more detailed/nuanced metrics through RectStyle enum. #6335

Uh oh!

Conversation

GaryQian commented Sep 25, 2018

Uh oh!

GaryQian Sep 25, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Hixie Sep 26, 2018

Choose a reason for hiding this comment

Uh oh!

GaryQian Sep 26, 2018

Choose a reason for hiding this comment

Uh oh!

Hixie Sep 27, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Hixie commented Sep 26, 2018

Uh oh!

GaryQian commented Sep 26, 2018

Uh oh!

GaryQian commented Oct 1, 2018

Uh oh!

Hixie commented Oct 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants