Change Rect internal representation from Float32List to Float64List #8524
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 10, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 10, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 10, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 10, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 10, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 10, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 11, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 11, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 11, 2019
This was referenced Apr 11, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 11, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 11, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 11, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 11, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 11, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 11, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 11, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 11, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 12, 2019
9 tasks
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 12, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 12, 2019
engine-flutter-autoroll
added a commit
to flutter/flutter
that referenced
this pull request
Apr 12, 2019
flutter/engine@72986c3...f804c29 git log 72986c3..f804c29 --no-merges --oneline f804c29 Roll src/third_party/skia 3b60397fd35d..f74fff660084 (3 commits) (flutter/engine#8556) 1b448e5 Roll Dart 15b11b018364ce032eae50d78fc8a52b541e2bce...a8f3a5dae6203d1064726a5953cf06a7d484249c (flutter/engine#8555) 99259ae Roll src/third_party/skia 36477b49c2ef..3b60397fd35d (6 commits) (flutter/engine#8554) f37220d Roll src/third_party/skia c33e6dcc700b..36477b49c2ef (8 commits) (flutter/engine#8552) d84d204 Android Embedding PR30: Make FlutterView focusable so that the keyboard can interact with it. (flutter/engine#8551) a344015 [fuchsia] Add flutter:: to scene_host.cc (flutter/engine#8549) 711d843 Roll src/third_party/skia 76e626d9bb55..c33e6dcc700b (4 commits) (flutter/engine#8547) 82e6d68 Eliminate unused write to local (flutter/engine#8541) f53e477 Correct nullability for FlutterStandardReader (flutter/engine#8537) c00364a [font_collection] Add missing semicolon (flutter/engine#8546) ae4df6f Revert "Change Rect internal representation from Float32List to Float64List (#8524)" (flutter/engine#8545) ffdddb0 Roll src/third_party/skia 6d60534e95d8..76e626d9bb55 (1 commits) (flutter/engine#8544) 5ac728e Roll src/third_party/skia 4d657d5e894a..6d60534e95d8 (3 commits) (flutter/engine#8543) ee462ff Roll src/third_party/skia 99d792276740..4d657d5e894a (1 commits) (flutter/engine#8542) 18816c5 Roll src/third_party/skia 42280f8961fa..99d792276740 (8 commits) (flutter/engine#8539) 892591d Android Embedding PR28: Report app is active to Flutter in FlutterFragment.onResume() instead of onPostResume() forwarded from Activity. (flutter/engine#8536) 330c6c1 Use code cache dir for engine cache on API >= 21 (#14704). (flutter/engine#8534) c10ae2b Add an option to build the GLFW shell on macOS (flutter/engine#8531) 29aa5fc Roll src/third_party/skia b5c685991faa..42280f8961fa (32 commits) (flutter/engine#8535) 0a2869e Added support for authentication codes for the VM service (flutter/engine#8527) 36d2135 Redo a fix for cull rect calculation on TransformLayers with a perspective transform (flutter/engine#8528) 0b36d3e Change Rect internal representation from Float32List to Float64List (flutter/engine#8524) b1ae0cc Roll src/third_party/skia 10bf7020aa15..b5c685991faa (3 commits) (flutter/engine#8526) 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 (jsimmons@google.com), and stop the roller if necessary.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 13, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes flutter/flutter#27320.
The ui.Rect API is defined in terms of double. The internal representation of ui.Rect and ui.RRect had been Float32List to make it convenient to pass the Rect along to native SKIA methods.
The loss of precision has occasionally led to confusion and in the case of flutter/flutter#27320 it causes a crash. The crash occurs in trivial app with a Hero whose height is based on a Column with MainAxisSize.min. The source of the crash is that the overall height of the layout when the hero is "in flight" is 0.000017438 smaller than the initial layout. This causes the app's Column to "overflow".
The expected height is stored in a Rect in the hero implementation and that's what introduces the error. Not to put too fine a point on it but with the current implementation: (Offset.zero & size).height doesn't necessarily equal size.height.
This PR: when a Rect's Float32List is needed it's created with Float32List.fromList(). This adds a small performance cost and generates an additional four element Float32List of ephemeral garbage for each native SKIA call with a Rect parameter. The change does not appear to have an impact on the complex_layout/scroll_perf benchmark or macrobenchmarks/cull_opacity_perf.