-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve panning behavior on pitched maps #888
Conversation
I find the fling inertia on iPad (Pro 11) on tilted map very nice: Untitled.mov |
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.
Mostly rubber stamp on tests code, reviewed gesture implementation. Code change looks good and it performs nice on iOS.
guard case .mercator = try? mapProjection() else { | ||
return false | ||
} | ||
let topMargin = 0.04 * size.height |
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.
This is explained in Android PR comment: 4% of height is arbitrary, experimental, a dead gesture zone under horizon line that IMHO works OK on iPad 11 (as it requires that finger is fully under horizon line). It helps in avoiding sharp map movements but gl-native fixes on tracking ellipsoid also help there, so you might decide to remove it or reduce it.
location: touchLocation, | ||
velocity: gestureRecognizer.velocity(in: view), | ||
location: initialDecelerationLocation, | ||
velocity: clampTouchLocation(velocity, previousTouchLocation: .zero), |
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.
Just would like to understand, why is .zero here not causing issues when pan direction is clamped to be vertical?
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.
Previously, to achieve horizontal or vertical clamping during fling, we'd clamp the simulated touchLocation. While working on this, I realized that we could achieve the same effect more elegantly by setting the velocity to 0 for the dimension where we don't want any movement.
I'm just reusing the same method that was originally named with the idea that it'd be used to clamp positions. Now that it's used to clamp positions during drag and velocities during fling, I can update the name to be more generic.
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.
Updated in 86f19cb to hopefully be a bit more clear
* Fix StyleColor tests for iOS 12 and lower (#847) * Make persistent layer APIs non-experimental (#849) * Generate private module interface (#854) * Fix changelog (#857) * Fix to only localize the primary localization and not the fall-throug… (#856) * Exclude swiftlint config from packaged artifact (#859) * View Annotations follow-up (#846) * Reduce geometry wrapping using GeometryConvertible (#861) * Update baselines to new format (#863) * Delay adding puck images until adding layer (#862) * Add missing StyleColor support for exponentials (#873) * Fix attribution dialog initialization (#865) * Add v10.0.2 to CHANGELOG.md (#883) * Improve panning behavior on pitched maps (#888) * Add pinch gesture tradeoff configuration option (#890) * Update MapboxCoreMaps and MapboxCommon (#891) * Update changelog and bump version for v10.2.0-rc.1 (#892)
Pull request checklist:
Summary of changes
Improves panning behavior on pitched maps: