[Impeller] Make Rect origin and size fields private #49168
Conversation
|
The new additions to the Rect class all have unit tests. The changes in the rest of the file should be simply replacing an old reference with its replacement. More than 90% of the time Please evaluate as part of your reviews if you think there are additional tests needed to double-check the substitutions. |
dnfield
left a comment
dnfield
left a comment
There was a problem hiding this comment.
LGTM with a nitpick about GetRelative and a typo
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. |
|
The goldens have identified a bug somewhere. Looking into it... |
There were a couple of places where I called a Rect method without storing its return value. I've fixed those and added Before my first commit on this PR a unit test had already caught one other case where I was ignoring a return value so this scenario is likely to happen again if not for the no-discard tags just added... |
…140359) flutter/engine@632103f...bbfee6f 2023-12-19 [email protected] Roll Skia from fff421ff633e to e5b0cdc11992 (4 revisions) (flutter/engine#49210) 2023-12-19 [email protected] Roll Skia from 63bed826008e to fff421ff633e (1 revision) (flutter/engine#49208) 2023-12-19 [email protected] Double timeout on mac builds. (flutter/engine#49205) 2023-12-18 [email protected] [Impeller] Make Rect origin and size fields private (flutter/engine#49168) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…lutter#140359) flutter/engine@632103f...bbfee6f 2023-12-19 [email protected] Roll Skia from fff421ff633e to e5b0cdc11992 (4 revisions) (flutter/engine#49210) 2023-12-19 [email protected] Roll Skia from 63bed826008e to fff421ff633e (1 revision) (flutter/engine#49208) 2023-12-19 [email protected] Double timeout on mac builds. (flutter/engine#49205) 2023-12-18 [email protected] [Impeller] Make Rect origin and size fields private (flutter/engine#49168) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
2 participants
Make the implementation fields of Rect private in preparation for switching its internal operations from XYWH to LTRB fields.
Additionally
[[nodiscard]]tags are added to all of the Rect methods that return a value to avoid problems with confusing mutator methods and return value methods.