This repository was archived by the owner on Feb 25, 2025. It is now read-only.
[OR] Offset.fromDirection and Size.aspectRatio#6805
Merged
Hixie merged 1 commit intoflutter:masterfrom Dec 15, 2018
Merged
Conversation
Contributor
Author
|
Completely untested code right now. :-) |
jason-simmons
approved these changes
Nov 8, 2018
jamesderlin
reviewed
Nov 8, 2018
testing/dart/geometry_test.dart
Outdated
| expect(new Offset.fromDirection(pi / 2.0), const Offset(0.0, 1.0)); | ||
| expect(new Offset.fromDirection(-pi / 2.0), const Offset(0.0, -1.0)); | ||
| expect(new Offset.fromDirection(0.0), const Offset(1.0, 0.0)); | ||
| expect(new Offset.fromDirection(pi / 4.0), const Offset(1.0, 1.0)); |
Contributor
There was a problem hiding this comment.
Does this test succeed? Either the distance should be sqrt(2) or the expected value should be Offset(sqrt(2) / 2, sqrt(2) / 2), no? I also would worry that the floating point comparison would fail.
Same for some of the other tests.
Contributor
Author
There was a problem hiding this comment.
the code was completely untested at the point where i pushed it, i wanted to get something on github so i could show a community member who was asking for it.
lib/ui/geometry.dart
Outdated
| /// | ||
| /// The distance can be omitted, to create a unit vector (distance = 1.0). | ||
| factory Offset.fromDirection(double direction, [ double distance = 1.0 ]) { | ||
| return new Offset(distance * math.cos(direction), - distance * math.sin(direction)); |
Contributor
There was a problem hiding this comment.
Our style recommendation is to not use new anymore, right?
Contributor
Author
There was a problem hiding this comment.
Yeah but this file hasn't been converted yet.
lib/ui/geometry.dart
Outdated
| /// ratio. | ||
| /// * [FittedBox], a widget that (in most modes) attempts to maintain a | ||
| /// child widget's aspect ratio while changing its size. | ||
| double get aspectRatio => dx / dy; |
Member
There was a problem hiding this comment.
Change this to _dx / _dy or width / height
Hixie
changed the title
Hixie
changed the title
Contributor
Author
|
Ok, this time it's actually been tested. |
Member
|
@Hixie thanks 👍 |
Hixie
changed the title
Contributor
Author
|
Rebased and kicked bots, will land on green. |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 15, 2018
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 15, 2018
This was referenced Dec 15, 2018
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 16, 2018
engine-flutter-autoroll
added a commit
to flutter/flutter
that referenced
this pull request
Dec 16, 2018
flutter/engine@101b27d...4941125 git log 101b27d..4941125 --no-merges --oneline 4941125 Revert "Compile embedder unit test Dart to kernel (#7227)" (flutter/engine#7230) f7d91d6 Roll src/third_party/skia 282ec5dc6ca7..2e6db18c4309 (1 commits) (flutter/engine#7228) ac9e521 Compile embedder unit test Dart to kernel (flutter/engine#7227) 828acc7 Roll src/third_party/skia d4962e7e07c0..282ec5dc6ca7 (1 commits) (flutter/engine#7226) 8a7ae95 Undeprecated BigInteger support, but document what it actually does. (flutter/engine#6903) 1778924 Offset.fromDirection and Size.aspectRatio (flutter/engine#6805) 9ae8217 Roll src/third_party/skia 8b78d70b9a10..d4962e7e07c0 (1 commits) (flutter/engine#7225) a987b17 Roll src/third_party/skia a1bded9a4f28..8b78d70b9a10 (1 commits) (flutter/engine#7224) ac05993 Roll src/third_party/skia 16d00eeef7d1..a1bded9a4f28 (2 commits) (flutter/engine#7223) 91bb1f8 Roll src/third_party/skia f391d0f771c6..16d00eeef7d1 (4 commits) (flutter/engine#7222) 6c40f84 [vulkan] Fix Fuchsia build 4f148ed Roll src/third_party/skia f6206f91b3c1..f391d0f771c6 (4 commits) (flutter/engine#7220) 1bc7ccf Support real fonts in 'flutter test' (flutter/engine#6913) 5244cf4 Roll src/third_party/skia 59c9f1595ecd..f6206f91b3c1 (1 commits) (flutter/engine#7219) 0f10d3f Roll src/third_party/skia 42e7a7ed6511..59c9f1595ecd (3 commits) (flutter/engine#7218) 1676421 Roll src/third_party/skia 21a7be0741ac..42e7a7ed6511 (1 commits) (flutter/engine#7217) 1a2714b Adds force cursor support (flutter/engine#6945) b50e349 Roll src/third_party/skia cb6b4bde0c2e..21a7be0741ac (3 commits) (flutter/engine#7216) 826f342 [Fuchsia] Depend on libtrace when that is what's really meant (flutter/engine#7214) 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 ([email protected]), and stop the roller if necessary.
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
No description provided.