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
Make Rect and RRect use 64 bit doubles, and make them const-able #8565
Conversation
blRadiusY: radiusY, | ||
brRadiusX: radiusX, | ||
brRadiusY: radiusY, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than a ._raw constructor you could also just inline the initializers in each constructor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _raw
constructor asserts everything is not null.
@@ -1438,38 +1451,38 @@ class RRect { | |||
if (point.dx < left || point.dx >= right || point.dy < top || point.dy >= bottom) | |||
return false; // outside bounding box | |||
|
|||
_scaleRadii(); | |||
final RRect scaled = _scaleRadii(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we're not caching that work anymore, this may have perf implications.
we should make sure we have a benchmark that covers the case here (of calling contains
many times on the same rrects).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flutter/flutter#30985 will track that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
: assert(left != null), | ||
assert(top != null), | ||
assert(right != null), | ||
assert(bottom != null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as last time, remember to mark the const
linter warnings as ignore until we've rolled internally and embedders have been updated.
I will be opening a manual Flutter roll to land this. |
…ble (flutter#8565)" This reverts commit c123152.
flutter/engine@934772d...0523870 git log 934772d..0523870 --no-merges --oneline 0523870 Add tests from framework (flutter/engine#8692) ed1f3fd Change Vertices.indices to use a Uint16 list to more accurately reflect Skia's API (flutter/engine#8657) 4f2fd84 Revert Rect/RRect 64 bit (flutter/engine#8690) 6dc5dca Revert "Remove unused Settings::ToString. (#8642)" (flutter/engine#8689) 9fa7336 fix toString (flutter/engine#8688) c123152 Make Rect and RRect use 64 bit doubles, and make them const-able (flutter/engine#8565) 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 (liyuqian@google.com), and stop the roller if necessary.
This relands work done by @HansMuller in #8524 with a couple changes:
double
instead of aFloat64List
, so we can make at least some constructorsconst
RRect
.This will need to be manually rolled into the framework to update the Semantics traversal test, due to precision differences in the Rect thatimpact that test (as documented in flutter/flutter#30942). That PR updates a few places that can be made to work with both 32 and 64 bit doubles.
Fixes flutter/flutter#27320