This repository has been archived by the owner on Aug 8, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
refs #6779: mobile SDK style transition options #7711
Merged
Merged
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
aeb20cb
refs #6779: first cut of iOS transition options
incanus b2b0198
use properties instead
incanus 705cb6b
use auto
incanus fb9c416
[core] Fix calculation of delayed transitions
jfirebaugh 59f5e7a
Merge remote-tracking branch 'origin/fix-7720' into 6779-style-transi…
incanus 2c68ae7
re-enable transition delay due to #7756 fix
incanus 2f4ee9c
Merge remote-tracking branch 'origin/master' into 6779-style-transiti…
incanus 16d6a05
pipe Android support
incanus 530e32b
docs comments
incanus 6ddc216
clearer function naming
incanus 1fa75dd
update macos for new function
incanus 71947b6
exercise macos transition options
incanus 264005b
upping checkstyle max file length to 2042 per chat with @zugaldia
incanus d2d4af9
minor comment clarity tweaks
incanus 37f3c54
Merge remote-tracking branch 'origin/master' into 6779-style-transiti…
incanus 6745984
update Android, iOS, and macOS changelogs
incanus File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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.
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.
MGLTimeIntervalFromDuration()
would make the return type clearer, presuming that anNSTimeInterval
is always measured in seconds (which is the case).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.
Then should we not also rename
MGLDurationInSeconds()
toMGLDurationFromTimeInterval()
?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.
MGLDurationInSecondsFromTimeInterval()
, actually.mbgl::Duration
can be expressed in many units; seconds is the one we’ve chosen to use for consistency withNSTimeInterval
, but other parts of mbgl use milliseconds instead.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.
Right, took me a while to figure that out, but true. Do you think it's worth adding an assertion or other check here that it remains in seconds in this part of the code?
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.
No, I don’t think that’s an issue with
std::chrono::duration
ormbgl::Duration
(which is actually in nanoseconds, my bad).MGLDurationInSeconds()
performs aduration_cast
to ensure that the output has the right quantity (if not the same unit as the input).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.
mbgl::Duration
is similar toNSTimeInterval
in the sense that the unit is fixed and built into the type. You can't have onembgl::Duration
representing a duration "in seconds" and another representing a duration "in milliseconds". The actual unit is defined bystd::chrono::steady_clock
and I'm not sure that it's invariant across platforms or standard library implementations. But it is fixed at compile time.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.
6ddc216