diff options
author | Julian Rex <julian.rex@mapbox.com> | 2018-04-17 00:14:09 -0400 |
---|---|---|
committer | Julian Rex <julian.rex@mapbox.com> | 2018-05-21 13:05:04 -0400 |
commit | dd967497903be57aa5adb7a9f0a0cd418d3a3776 (patch) | |
tree | 508cb55509b2c13a97228e81cf3fdbd080ffff31 | |
parent | a73d2ad6c3f06eed638e0eb97e8d19b6cf15218e (diff) | |
download | qtlocation-mapboxgl-dd967497903be57aa5adb7a9f0a0cd418d3a3776.tar.gz |
Rejig handling of transition finish lambda. Potentially addresses #5833
-rw-r--r-- | src/mbgl/map/transform.cpp | 65 | ||||
-rw-r--r-- | src/mbgl/map/transform.hpp | 2 |
2 files changed, 28 insertions, 39 deletions
diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp index 331c1dd958..95b820a282 100644 --- a/src/mbgl/map/transform.cpp +++ b/src/mbgl/map/transform.cpp @@ -594,36 +594,10 @@ void Transform::startTransition(const CameraOptions& camera, animation.transitionFrameFn(t); } observer.onCameraIsChanging(); + return false; } else { - - // Use a temporary function to ensure that the transitionFinishFn - // lambda is only called once at the end of a transition. - // - // This addresses the symptons of https://github.com/mapbox/mapbox-gl-native/issues/11180 - // where setting a shape source to nil (or similar) in the - // `onCameraDidChange` observer function causes `Map::Impl::onUpdate()` - // to be called which in turn calls this lambda (before the current - // iteration has completed), leading to an infinite loop. - // - // By using a temporary, and clearing transitionFinishFn we stop this - // recursion. (However it does not address the underlying problem of - // `onSourceChanged()` called from observer methods triggering - // `Map::Impl::onUpdate()`) - // - // This is now addressed at an earlier stage by a similar change - // below in `Transform::updateTransitions()`. - - auto finish = transitionFinishFn; - - transitionFinishFn = nullptr; - transitionFrameFn = nullptr; - - if (finish) { - finish(); - } - - // This callback gets destroyed here, - // we can only return after this point. + // Indicate that we need to terminate this transition + return true; } }; @@ -638,7 +612,16 @@ void Transform::startTransition(const CameraOptions& camera, }; if (!isAnimated) { - transitionFrameFn(Clock::now()); + auto update = transitionFrameFn; + auto finish = transitionFinishFn; + + transitionFrameFn = nullptr; + transitionFinishFn = nullptr; + + auto shouldFinish = update(Clock::now()); + assert(shouldFinish); + + finish(); } } @@ -657,19 +640,25 @@ void Transform::updateTransitions(const TimePoint& now) { // leading to an infinite loop. // // By temporarily nulling the `transitionFrameFn` (and then restoring it - // after the temporary has been called) we stop this recursion. (However it - // does not address the underlying problem of `onSourceChanged()` - // called from observer methods triggering `Map::Impl::onUpdate()`) + // after the temporary has been called) we stop this recursion. auto transition = transitionFrameFn; transitionFrameFn = nullptr; - if (transition) { - transition(now); + if (transition && transition(now)) { + // If the transition indicates that it is complete, then we should call + // the finish lambda (going via a temporary as above) + auto finish = transitionFinishFn; + + transitionFinishFn = nullptr; + transitionFrameFn = nullptr; - // Only reset the transition function if we haven't already finished. - if (transitionFinishFn) - transitionFrameFn = transition; + if (finish) { + finish(); + } + } + else { + transitionFrameFn = transition; } } diff --git a/src/mbgl/map/transform.hpp b/src/mbgl/map/transform.hpp index d429c57661..bff44a2dcd 100644 --- a/src/mbgl/map/transform.hpp +++ b/src/mbgl/map/transform.hpp @@ -165,7 +165,7 @@ private: TimePoint transitionStart; Duration transitionDuration; - std::function<void(const TimePoint)> transitionFrameFn; + std::function<bool(const TimePoint)> transitionFrameFn; std::function<void()> transitionFinishFn; }; |