Round tap-zoom gestures to nearest integer #8027

friedbunny · 2017-02-11T08:12:35Z

iOS changes

Rounds double-tap and two-finger tap zoom gestures to the nearest integer zoom level. This has benefits for raster tiles, as well as styles with zoom-based functions that transition on integers.

This results in a wider possible zoom range per gesticulation — 1.0 → ~0.5-1.5 levels. Here’s a comparison of double-tapping to zoom in with the changes in this PR:

Starting Zoom	Old Result	New Result
4.0	5.0 (+1.0)	5.0 (+1.0)
4.4	5.4 (+1.0)	5.0 (+0.6)
4.6	5.6 (+1.0)	6.0 (+1.4)

Fixes #2276.

Core changes

Exposes an already-existing setZoom variation that takes an anchor parameter — I did this to avoid calculating scale levels in the iOS zoom code. This makes Map::setZoom(zoom, {}, duration) ambiguous, so it’s now necessary to specify a type for the middle parameter.

mention-bot · 2017-02-11T08:12:39Z

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

1ec5 · 2017-02-11T18:44:34Z

platform/ios/src/MGLMapView.mm

@@ -1556,18 +1556,20 @@ - (void)handleDoubleTapGesture:(UITapGestureRecognizer *)doubleTap
    if (doubleTap.state == UIGestureRecognizerStateEnded)
    {
        MGLMapCamera *oldCamera = self.camera;
-
+
+        double newZoom = round(self.zoomLevel) + 1.0;


Can you make the corresponding changes to the Zoom In and Zoom Out buttons in the Android and macOS SDKs?

mapbox-gl-native/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Transform.java

Lines 180 to 184 in e6c15d0

if (zoomIn) {

mapView.scaleBy(2.0, x, y, MapboxConstants.ANIMATION_DURATION);

} else {

mapView.scaleBy(0.5, x, y, MapboxConstants.ANIMATION_DURATION);

}

mapbox-gl-native/platform/macos/src/MGLMapView.mm

Line 1045 in e6c15d0

[self setZoomLevel:self.zoomLevel + zoomDelta animated:animated];

/cc @tobrun

Went a bit further on macOS in 4ed80f9 and rounded all of the non-freeform zoom ~~methods~~ gestures and commands.

I’ll leave Android to @tobrun, in light of #7630.

friedbunny · 2017-02-11T20:03:32Z

As an extreme example, here’s Mapbox Streets at z10.99 vs z11.00:

1ec5

Looks good, and thanks also for the changes on the macOS side. I’ll leave it to you to decide whether the Android SDK should get this fix here or in a separate PR.

1ec5 · 2017-02-11T22:09:06Z

platform/macos/CHANGELOG.md

@@ -39,6 +39,7 @@
 * Fixed flickering that occurred when panning past the antimeridian. ([#7574](https://github.com/mapbox/mapbox-gl-native/pull/7574))
 * Added a method to MGLMapViewDelegate, `-mapView:shouldChangeFromCamera:toCamera:`, that you can implement to restrict which parts the user can navigate to using gestures. ([#5584](https://github.com/mapbox/mapbox-gl-native/pull/5584))
 * Added a `MGLDistanceFormatter` class for formatting geographic distances. ([#7888](https://github.com/mapbox/mapbox-gl-native/pull/7888))
+* Double-tap, two-finger tap, zoom buttons, and demo app menu items and shortcut keys now zoom to the nearest integer zoom level. ([#8027](https://github.com/mapbox/mapbox-gl-native/pull/8027))


FYI, the ⌥↑ and ⌥↓ shortcuts are built into MGLMapView. The menu items (which have their own shortcuts) are defined in MapDocument but could conceivably be moved into MGLMapView as well.

Ah, did not know that — I’ll update the text here.

1ec5 · 2017-02-11T22:13:36Z

It looks like #7630 would’ve implemented a similar fix on the Android side. Not sure why @tobrun closed it.

Round double-tap and two-finger tap zoom gestures to the nearest integer zoom level. This has the benefits for raster tiles, as well as styles with zoom-based functions. This results in a wider possible zoom range — ~0.5-1.5: Old: z4.6 → z5.6 (+1.0), z4.4 → z5.4 (+1.0) New: z4.6 → z6.0 (+1.4), z4.4 → z5.0 (+0.6)

Affects: - Double-tap gestures - Two-finger tap gestures - +/- button pushes - Shortcut keys - Menu items and shortcut keys (in macapp)

tobrun · 2017-02-13T06:41:21Z

@1ec5 @friedbunny I initially created that PR to showcase the possibility of such a feature but this didn't make it to master at that time as I didn't want to change the "default" behaviour. We could definitely look into reopening it for consistency between the bindings.

cc @mapbox/android

zugaldia · 2017-02-13T14:01:03Z

We could definitely look into reopening it for consistency between the bindings.

Yeah, let's do that for 5.0. It doesn't look like a big lift.

1ec5 · 2017-02-13T23:01:59Z

The Android side is happening once again in #7630. 🚢

friedbunny added Core The cross-platform C++ core, aka mbgl Google Maps parity For feature parity with the Google Maps SDK for Android or iOS iOS Mapbox Maps SDK for iOS MapKit parity For feature parity with MapKit on iOS or macOS labels Feb 11, 2017

friedbunny self-assigned this Feb 11, 2017

friedbunny requested review from jfirebaugh and 1ec5 February 11, 2017 08:12

friedbunny force-pushed the fb-integer-zooming branch from b64d9cb to de6d862 Compare February 11, 2017 08:32

friedbunny added this to the ios-v3.5.0 milestone Feb 11, 2017

1ec5 reviewed Feb 11, 2017

View reviewed changes

friedbunny force-pushed the fb-integer-zooming branch from bb07128 to 4ed80f9 Compare February 11, 2017 22:02

1ec5 approved these changes Feb 11, 2017

View reviewed changes

1ec5 reviewed Feb 11, 2017

View reviewed changes

1ec5 mentioned this pull request Feb 11, 2017

[android] - rounded zoom when zooming with zoom button controller #7630

Closed

friedbunny added 2 commits February 11, 2017 17:17

[core] Expose Map::setZoom with anchor parameter

7530e0a

friedbunny force-pushed the fb-integer-zooming branch from 4ed80f9 to b27ace8 Compare February 11, 2017 22:19

friedbunny added 2 commits February 11, 2017 17:50

[ios] Add iosapp Show Zoom Level debug option

74b1901

[macos] Round non-freeform zoom gestures/commands to nearest integer

4b227ed

Affects: - Double-tap gestures - Two-finger tap gestures - +/- button pushes - Shortcut keys - Menu items and shortcut keys (in macapp)

friedbunny force-pushed the fb-integer-zooming branch from b27ace8 to 4b227ed Compare February 11, 2017 22:50

friedbunny added the macOS Mapbox Maps SDK for macOS label Feb 11, 2017

jfirebaugh approved these changes Feb 13, 2017

View reviewed changes

friedbunny merged commit d12ac71 into master Feb 13, 2017

friedbunny deleted the fb-integer-zooming branch February 13, 2017 23:35

1ec5 mentioned this pull request Feb 15, 2017

[macos] Make +, - and = keys zoom in/out #8046

Merged

tobrun mentioned this pull request Mar 7, 2017

[android] - zoom to rounded levels #8299

Merged

zugaldia mentioned this pull request Mar 10, 2017

Release Android v5.0.0-beta.3 #8350

Closed

8 tasks

tobrun mentioned this pull request Mar 17, 2017

Release Android v5.0.0 #8452

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Round tap-zoom gestures to nearest integer #8027

Round tap-zoom gestures to nearest integer #8027

friedbunny commented Feb 11, 2017 •

edited

Loading

mention-bot commented Feb 11, 2017

1ec5 Feb 11, 2017

friedbunny Feb 11, 2017 •

edited

Loading

friedbunny Feb 11, 2017

friedbunny commented Feb 11, 2017 •

edited

Loading

1ec5 left a comment

1ec5 Feb 11, 2017

friedbunny Feb 11, 2017

1ec5 commented Feb 11, 2017

tobrun commented Feb 13, 2017

zugaldia commented Feb 13, 2017

1ec5 commented Feb 13, 2017

	if (zoomIn) {
	mapView.scaleBy(2.0, x, y, MapboxConstants.ANIMATION_DURATION);
	} else {
	mapView.scaleBy(0.5, x, y, MapboxConstants.ANIMATION_DURATION);
	}

Round tap-zoom gestures to nearest integer #8027

Round tap-zoom gestures to nearest integer #8027

Conversation

friedbunny commented Feb 11, 2017 • edited Loading

iOS changes

Core changes

mention-bot commented Feb 11, 2017

1ec5 Feb 11, 2017

Choose a reason for hiding this comment

friedbunny Feb 11, 2017 • edited Loading

Choose a reason for hiding this comment

friedbunny Feb 11, 2017

Choose a reason for hiding this comment

friedbunny commented Feb 11, 2017 • edited Loading

1ec5 left a comment

Choose a reason for hiding this comment

1ec5 Feb 11, 2017

Choose a reason for hiding this comment

friedbunny Feb 11, 2017

Choose a reason for hiding this comment

1ec5 commented Feb 11, 2017

tobrun commented Feb 13, 2017

zugaldia commented Feb 13, 2017

1ec5 commented Feb 13, 2017

friedbunny commented Feb 11, 2017 •

edited

Loading

friedbunny Feb 11, 2017 •

edited

Loading

friedbunny commented Feb 11, 2017 •

edited

Loading