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

MGLMapView viewport methods should cancel animations #2313

Closed
1ec5 opened this issue Sep 11, 2015 · 5 comments · Fixed by #3086
Closed

MGLMapView viewport methods should cancel animations #2313

1ec5 opened this issue Sep 11, 2015 · 5 comments · Fixed by #3086
Assignees
Labels
bug iOS Mapbox Maps SDK for iOS

Comments

@1ec5
Copy link
Contributor

1ec5 commented Sep 11, 2015

Per #2296, methods like -[MGLMapView setCenterCoordinate:zoomLevel:] should call mbgl::Map::cancelTransitions(), just like the gesture recognizers do.

/cc @incanus

@1ec5 1ec5 added bug iOS Mapbox Maps SDK for iOS starter-task labels Sep 11, 2015
@incanus
Copy link
Contributor

incanus commented Sep 11, 2015

This would probably help #1975 (comment).

@incanus
Copy link
Contributor

incanus commented Sep 14, 2015

To be clear: the goal here is to avoid any sort of stale viewport changes to occur after applying another viewport change.

@tomtaylor
Copy link

Just to confirm, while testing #2204, that this is an issue with setCamera too.

@tomtaylor
Copy link

@incanus do you have an idea of how much work this is? The last good version for me was pre2, which seems to also contain some crashing bugs under iOS 9. I'm blocked on releasing an iOS 9 compatible version of my app at the moment.

@1ec5
Copy link
Contributor Author

1ec5 commented Sep 23, 2015

@adam-mapbox, #2321 has been merged, but it doesn’t seem to be testing this scenario: -testSetCenterCancelsTransitions seems to:

  1. Go to DC instantaneously.
  2. Go to the west with animation.
  3. Halfway through the animation:
    1. Go back to DC instantaneously.
    2. Immediately assert that step (i) happened.

However, to adequately test whether setting a coordinate cancels other animations, we need to perform (ii) after the animation in (2) runs to completion. This test doesn’t account for the possibility that the animation in (2) continues running after (3) is done, clobbering (i).

@incanus incanus added the P1 label Nov 4, 2015
1ec5 added a commit that referenced this issue Nov 20, 2015
Coalescing willChange/didChange notifications means keeping track of how many gestures are currently in progress; a boolean isn’t enough to track this state. This change refactors the gesture recognizers, making them more consistent with each other and more consistent in the case where more than one of them has fired. It also explicitly cancels transitions before all programmatic viewport-modifying methods, since mbgl only does so when animating.

Fixes #2313, fixes #2379, fixes #3062.
@1ec5 1ec5 self-assigned this Nov 20, 2015
1ec5 added a commit that referenced this issue Nov 25, 2015
Coalescing willChange/didChange notifications means keeping track of how many gestures are currently in progress; a boolean isn’t enough to track this state. This change refactors the gesture recognizers, making them more consistent with each other and more consistent in the case where more than one of them has fired. It also explicitly cancels transitions before all programmatic viewport-modifying methods, since mbgl only does so when animating.

Fixes #2313, fixes #2379, fixes #3062.
@1ec5 1ec5 removed the in progress label Nov 25, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants