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

Prevent panning to NaN territory #1446

Merged
merged 3 commits into from
May 8, 2015
Merged

Prevent panning to NaN territory #1446

merged 3 commits into from
May 8, 2015

Conversation

kkaefer
Copy link
Member

@kkaefer kkaefer commented May 8, 2015

Rapid panning and rotation sometimes lets the map view end up in a state where both x and y coordinates are NaN. Typically, the only thing rendering will be the background color. Once you're in this state, you can set a breakpoint at Painter::render() and inspect state's x and y variables.

@kkaefer
Copy link
Member Author

kkaefer commented May 6, 2015

painter cpp 2015-05-06 18-51-26

@mb12
Copy link

mb12 commented May 6, 2015

This should be consistently reproducible using the steps described in issue #1294. (Panning with animation and zoom).

#1294

@jfirebaugh jfirebaugh added this to the iOS Beta 1 milestone May 6, 2015
@incanus incanus added the bug label May 6, 2015
@1ec5
Copy link
Contributor

1ec5 commented May 8, 2015

Once centerCoordinate becomes (NaN, NaN), even Map::resetPosition() (Reset Position in the gear menu) fails to bring the user back to a valid location.

@kkaefer
Copy link
Member Author

kkaefer commented May 8, 2015

Looks like UIPinchGestureRecognizer sometimes returns NaN for velocity:

image

@1ec5
Copy link
Contributor

1ec5 commented May 8, 2015

Maybe if the pinch starts out and winds up in the same place? In any case, this looks like a pretty good fix to put in. Want to do the honors and convert this screenshot into a PR, @kkaefer?

@jfirebaugh
Copy link
Contributor

Any idea where the NaNs come from (other than UIPinchGestureRecognizer)? I'm happy taking the Transform guard clauses for b1 but finding and eliminating the upstream causes would be better long term.

jfirebaugh added a commit that referenced this pull request May 8, 2015
@jfirebaugh jfirebaugh merged commit c2afa54 into master May 8, 2015
@jfirebaugh jfirebaugh deleted the 1446-transform-to-nan branch May 8, 2015 18:52
@mb12
Copy link

mb12 commented May 8, 2015

@jfirebaugh isnan check also needs to be done in to be done handlePanGesture: selector as well.

@kkaefer
Copy link
Member Author

kkaefer commented May 8, 2015

I believe they're only coming from the gesture recognizer. I have never observed NaN issues in anything but the iOS app.

@mb12
Copy link

mb12 commented May 8, 2015

handlePanGesture also calls [UIPanGestureRecognize velocity].

I see this consistently with the following steps on iOS:

  1. Trigger pan animation using fast flick.
  2. Zoom in using a stepper control.

@1ec5
Copy link
Contributor

1ec5 commented May 8, 2015

@mb12, are you seeing -[UIPanGestureRecognizer velocity] itself return NaN for a standard flick? #1294 could’ve been caused by the lower-level transform issues that @kkaefer fixed in this PR.

@mb12
Copy link

mb12 commented May 8, 2015

Sorry for the confusion. Everything looks good.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants