Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] - allow zooming/scaling to use AnimationOptions #8181

Merged
merged 1 commit into from
Feb 28, 2017

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Feb 23, 2017

This PR enables to use AnimationOptions when performing a zoom/scale transformation (equivalent to #8092 for moveBy). This allows bindings to use an interpolator while doing a zoom/scale transformation.

To showcase the possibilities with this API, here is an example of the double tap zoom gesture using the easeInOutBack interpolator with an increased duration:

ezgif com-video-to-gif 22

Note that atm I'm not changing the default behaviour for Android.

@tobrun tobrun added the Core The cross-platform C++ core, aka mbgl label Feb 23, 2017
@tobrun tobrun added this to the android-v5.0.0 milestone Feb 23, 2017
@tobrun tobrun self-assigned this Feb 23, 2017
@mention-bot
Copy link

@tobrun, thanks for your PR! By analyzing this pull request, we identified @1ec5, @jfirebaugh and @zugaldia to be potential reviewers.

Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was surprised the macOS SDK built without errors following this change, but apparently it works because mbgl::AnimationOptions has a constructor that takes an mbgl::Duration. So this is a pretty elegant change.

void setZoom(double zoom, optional<EdgeInsets>, const Duration& = Duration::zero());
void setZoom(double zoom, const AnimationOptions& = {});
void setZoom(double zoom, optional<ScreenCoordinate>, const AnimationOptions& = {});
void setZoom(double zoom, optional<EdgeInsets>, const AnimationOptions& = {});
double getZoom() const;
void setLatLngZoom(const LatLng&, double zoom, const Duration& = Duration::zero());
void setLatLngZoom(const LatLng&, double zoom, optional<EdgeInsets>, const Duration& = Duration::zero());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about changing these other zoom-related methods too, for consistency?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great catch, let me add those as well

@jfirebaugh
Copy link
Contributor

How about making this change universally in map.hpp? s/const Duration& = Duration::zero()/const AnimationOptions& = {}/g

@tobrun tobrun force-pushed the tvn-animationoptions-zoom branch 2 times, most recently from 13478b6 to b6aa887 Compare February 28, 2017 14:13
@tobrun tobrun added Android Mapbox Maps SDK for Android GLFW labels Feb 28, 2017
@tobrun
Copy link
Member Author

tobrun commented Feb 28, 2017

Requested changes have been implemented (resulted in changing some GLFW code) and were rebased on @ivovandongen JNI refactor PR for #6533. Can I get another round of 👀?

Copy link
Contributor

@jfirebaugh jfirebaugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Good to merge after fixing a tiny nit.

@@ -1,4 +1,4 @@
#include <mbgl/map/transform_state.hpp>
#include <mbgl/map/transform_state.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidental added whitespace.

[glfw] - allow glfw binding to use AnimationOptions instead of using direction directly
@tobrun
Copy link
Member Author

tobrun commented Feb 28, 2017

nit was fixed, merging.

@tobrun tobrun merged commit 34fef22 into master Feb 28, 2017
@tobrun tobrun deleted the tvn-animationoptions-zoom branch February 28, 2017 18:22
@zugaldia zugaldia mentioned this pull request Mar 10, 2017
8 tasks
@tobrun tobrun mentioned this pull request Mar 17, 2017
10 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants