From abfd989dd30ab045376e0fbb35f61d7c9bba450c Mon Sep 17 00:00:00 2001 From: Julien Heanley Date: Thu, 12 Jun 2014 11:43:42 +0100 Subject: [PATCH] ScrollView - Store properties in local values for GetCurrentScrollPosition [problem] If application performs more than one ScrollTo operation with the first having duration of 0 seconds, the second ScrollTo could have wrapping issues [cause] Internally scroll view uses current scroll position from property to check wrapping and perform correct animation, in case where this is done twice before an update the property may be incorrect for the second ScrollTo [solution] Store result of first ScrollTo if it is set immediately due to 0 second duration and use that value for next ScrollTo calculations Change-Id: I5230e00140c3cb75df8b14fc5e9227c16928406f Signed-off-by: Julien Heanley Signed-off-by: Adeel Kazmi --- .../scrollable/scroll-view/scroll-view-impl.cpp | 241 +++++++++------------ .../scrollable/scroll-view/scroll-view-impl.h | 28 +-- .../scrollable/scroll-view/scroll-view.cpp | 4 + 3 files changed, 125 insertions(+), 148 deletions(-) diff --git a/base/dali-toolkit/internal/controls/scrollable/scroll-view/scroll-view-impl.cpp b/base/dali-toolkit/internal/controls/scrollable/scroll-view/scroll-view-impl.cpp index 0dabb5a..97f4145 100644 --- a/base/dali-toolkit/internal/controls/scrollable/scroll-view/scroll-view-impl.cpp +++ b/base/dali-toolkit/internal/controls/scrollable/scroll-view/scroll-view-impl.cpp @@ -611,6 +611,7 @@ ScrollView::ScrollView() mFrictionCoefficient(Toolkit::ScrollView::DEFAULT_FRICTION_COEFFICIENT), mFlickSpeedCoefficient(Toolkit::ScrollView::DEFAULT_FLICK_SPEED_COEFFICIENT), mMaxFlickSpeed(Toolkit::ScrollView::DEFAULT_MAX_FLICK_SPEED), + mPropertiesUpdated(false), mInAccessibilityPan(false), mInitialized(false), mScrolling(false), @@ -1222,7 +1223,7 @@ Vector2 ScrollView::GetMouseWheelScrollDistanceStep() const unsigned int ScrollView::GetCurrentPage() const { // in case animation is currently taking place. - Vector3 position = GetPropertyPosition(); + Vector3 position = GetCurrentScrollPosition(); Actor self = Self(); unsigned int page = 0; @@ -1230,8 +1231,8 @@ unsigned int ScrollView::GetCurrentPage() const unsigned int volume = 0; // if rulerX is enabled, then get page count (columns) - page = mRulerX->GetPageFromPosition(-position.x, mWrapMode); - volume = mRulerY->GetPageFromPosition(-position.y, mWrapMode); + page = mRulerX->GetPageFromPosition(position.x, mWrapMode); + volume = mRulerY->GetPageFromPosition(position.y, mWrapMode); pagesPerVolume = mRulerX->GetTotalPages(); return volume * pagesPerVolume + page; @@ -1239,6 +1240,10 @@ unsigned int ScrollView::GetCurrentPage() const Vector3 ScrollView::GetCurrentScrollPosition() const { + if( mPropertiesUpdated ) + { + return -mScrollPostPosition; + } return -GetPropertyPosition(); } @@ -1280,19 +1285,10 @@ void ScrollView::TransformTo(const Vector3& position, const Vector3& scale, floa Vector3 currentScrollPosition = GetCurrentScrollPosition(); Self().SetProperty( mPropertyScrollStartPagePosition, currentScrollPosition ); - if( mScrolling ) // are we interrupting a current scroll? - { - // set mScrolling to false, in case user has code that interrogates mScrolling Getter() in complete. - mScrolling = false; - DALI_LOG_SCROLL_STATE("[0x%X] mScrollCompletedSignalV2 1 [%.2f, %.2f]", this, currentScrollPosition.x, currentScrollPosition.y); - mScrollCompletedSignalV2.Emit( currentScrollPosition ); - } + PreAnimatedScrollSetup(); - Self().SetProperty(mPropertyScrolling, true); - mScrolling = true; + Vector3 scrollStartPosition = GetCurrentScrollPosition(); - DALI_LOG_SCROLL_STATE("[0x%X] mScrollStartedSignalV2 1 [%.2f, %.2f]", this, currentScrollPosition.x, currentScrollPosition.y); - mScrollStartedSignalV2.Emit( currentScrollPosition ); bool animating = AnimateTo(-position, Vector3::ONE * duration, scale, @@ -1305,14 +1301,16 @@ void ScrollView::TransformTo(const Vector3& position, const Vector3& scale, floa verticalBias, Snap); + if( !mScrolling ) + { + DALI_LOG_SCROLL_STATE("[0x%X] mScrollStartedSignalV2 1 [%.2f, %.2f]", this, scrollStartPosition.x, scrollStartPosition.y); + ScrollingStarted(scrollStartPosition); + } + if(!animating) { - // if not animating, then this pan has completed right now. - Self().SetProperty(mPropertyScrolling, false); - mScrolling = false; - DALI_LOG_SCROLL_STATE("[0x%X] mScrollCompletedSignalV2 2 [%.2f, %.2f]", this, currentScrollPosition.x, currentScrollPosition.y); - SetScrollUpdateNotification(false); - mScrollCompletedSignalV2.Emit( currentScrollPosition ); + DALI_LOG_SCROLL_STATE("[0x%X] mScrollCompletedSignalV2 1 [%.2f, %.2f]", this, -mScrollPostPosition.x, -mScrollPostPosition.y); + ScrollingStopped(-mScrollPostPosition); } } @@ -1720,7 +1718,10 @@ void ScrollView::StopAnimation(void) StopAnimation(mSnapAnimation); StopAnimation(mInternalXAnimation); StopAnimation(mInternalYAnimation); + + // clear scrolling flags mScrollStateFlags = 0; + // remove scroll animation flags HandleStoppedAnimation(); } @@ -1756,12 +1757,6 @@ bool ScrollView::AnimateTo(const Vector3& position, const Vector3& positionDurat totalDuration = std::max(totalDuration, positionDuration.x); totalDuration = std::max(totalDuration, positionDuration.y); } - else - { - // try to animate for a frame, on some occasions update will be changing scroll value while event side thinks it hasnt changed - totalDuration = 0.01f; - positionChanged = true; - } if(scaleChanged) { @@ -1801,14 +1796,14 @@ bool ScrollView::AnimateTo(const Vector3& position, const Vector3& positionDurat // a horizonal/vertical wall.delay AnimateInternalXTo(mScrollTargetPosition.x, positionDuration.x, alpha); AnimateInternalYTo(mScrollTargetPosition.y, positionDuration.y, alpha); + } - if( !(mScrollStateFlags & SCROLL_ANIMATION_FLAGS) ) - { - self.SetProperty(mPropertyPrePosition, mScrollTargetPosition); - mScrollPrePosition = mScrollTargetPosition; - mScrollPostPosition = mScrollTargetPosition; - WrapPosition(mScrollPostPosition); - } + if( !(mScrollStateFlags & SCROLL_ANIMATION_FLAGS) ) + { + mScrollTargetPosition = position; + WrapPosition(mScrollTargetPosition); + self.SetProperty(mPropertyPrePosition, mScrollTargetPosition); + mScrollPrePosition = mScrollPostPosition = mScrollTargetPosition; } // Scale Delta /////////////////////////////////////////////////////// @@ -1904,17 +1899,12 @@ void ScrollView::FindAndUnbindActor(Actor child) Vector3 ScrollView::GetPropertyPrePosition() const { - Vector3 position = Self().GetProperty(mPropertyPrePosition); - WrapPosition(position); - return position; + return Self().GetProperty(mPropertyPrePosition); } Vector3 ScrollView::GetPropertyPosition() const { - Vector3 position = Self().GetProperty(mPropertyPosition); - WrapPosition(position); - - return position; + return Self().GetProperty(mPropertyPosition); } Vector3 ScrollView::GetPropertyScale() const @@ -1930,23 +1920,20 @@ void ScrollView::HandleStoppedAnimation() void ScrollView::HandleSnapAnimationFinished() { // Emit Signal that scrolling has completed. - mScrolling = false; Actor self = Self(); self.SetProperty(mPropertyScrolling, false); Vector3 deltaPosition(mScrollPrePosition); - UpdateLocalScrollProperties(); WrapPosition(mScrollPrePosition); self.SetProperty(mPropertyPrePosition, mScrollPrePosition); - Vector3 currentScrollPosition = GetCurrentScrollPosition(); - DALI_LOG_SCROLL_STATE("[0x%X] mScrollCompletedSignalV2 3 [%.2f, %.2f]", this, currentScrollPosition.x, currentScrollPosition.y); - mScrollCompletedSignalV2.Emit( currentScrollPosition ); - mDomainOffset += deltaPosition - mScrollPostPosition; self.SetProperty(mPropertyDomainOffset, mDomainOffset); HandleStoppedAnimation(); + + DALI_LOG_SCROLL_STATE("[0x%X] mScrollCompletedSignalV2 2 [%.2f, %.2f]", this, GetCurrentScrollPosition().x, GetCurrentScrollPosition().y); + ScrollingStopped(GetCurrentScrollPosition()); } void ScrollView::SetScrollUpdateNotification( bool enabled ) @@ -1983,8 +1970,12 @@ void ScrollView::OnScrollUpdateNotification(Dali::PropertyNotification& source) // Guard against destruction during signal emission Toolkit::ScrollView handle( GetOwner() ); - Vector3 currentScrollPosition = GetCurrentScrollPosition(); - mScrollUpdatedSignalV2.Emit( currentScrollPosition ); + // sometimes notification isnt removed quickly enough, dont emit signal if not scrolling + if( mScrolling ) + { + Vector3 currentScrollPosition = GetCurrentScrollPosition(); + mScrollUpdatedSignalV2.Emit( currentScrollPosition ); + } } bool ScrollView::DoConnectSignal( BaseObject* object, ConnectionTrackerInterface* tracker, const std::string& signalName, FunctorDelegate* functor ) @@ -2103,9 +2094,12 @@ bool ScrollView::OnTouchDownTimeout() Self().SetProperty(mPropertyDomainOffset, Vector3::ZERO); UpdateLocalScrollProperties(); - Vector3 currentScrollPosition = GetCurrentScrollPosition(); - DALI_LOG_SCROLL_STATE("[0x%X] mScrollCompletedSignalV2 4 [%.2f, %.2f]", this, currentScrollPosition.x, currentScrollPosition.y); - mScrollCompletedSignalV2.Emit( currentScrollPosition ); + mPropertiesUpdated = true; + DALI_LOG_SCROLL_STATE("[0x%X] mScrollCompletedSignalV2 3 [%.2f, %.2f]", this, GetCurrentScrollPosition().x, GetCurrentScrollPosition().y); + ScrollingStopped(GetCurrentScrollPosition()); + + // make sure scroll view returns actual property value and not locally stored value once we return from this function + mPropertiesUpdated = false; } } @@ -2126,6 +2120,9 @@ bool ScrollView::OnTouchEvent(const TouchEvent& event) return false; } + UpdateLocalScrollProperties(); + mPropertiesUpdated = true; + if( event.GetPoint(0).state == TouchPoint::Down ) { if(mGestureStackDepth==0) @@ -2153,7 +2150,6 @@ bool ScrollView::OnTouchEvent(const TouchEvent& event) mLastVelocity = Vector2( 0.0f, 0.0f ); } - UpdateLocalScrollProperties(); // Only finish the transform if scrolling was interrupted on down or if we are scrolling if ( mScrollInterrupted || mScrolling ) { @@ -2164,6 +2160,9 @@ bool ScrollView::OnTouchEvent(const TouchEvent& event) mScrollInterrupted = false; } + // no longer guarantee local properties are up to date + mPropertiesUpdated = false; + return true; } @@ -2213,19 +2212,15 @@ bool ScrollView::OnMouseWheelEvent(const MouseWheelEvent& event) return true; } -void ScrollView::ResetScrolling() -{ - Actor self = Self(); - self.GetProperty(mPropertyPosition).Get(mScrollPostPosition); - mScrollPrePosition = mScrollPostPosition; - self.SetProperty(mPropertyPrePosition, mScrollPostPosition); -} - void ScrollView::UpdateLocalScrollProperties() { - Actor self = Self(); - self.GetProperty(mPropertyPrePosition).Get(mScrollPrePosition); - self.GetProperty(mPropertyPosition).Get(mScrollPostPosition); + if( !mPropertiesUpdated ) + { + // only update local properties if we are not currently handler an event originating from this scroll view + Actor self = Self(); + self.GetProperty(mPropertyPrePosition).Get(mScrollPrePosition); + self.GetProperty(mPropertyPosition).Get(mScrollPostPosition); + } } // private functions @@ -2237,24 +2232,10 @@ void ScrollView::PreAnimatedScrollSetup() Actor self = Self(); - Vector3 deltaPosition(mScrollPostPosition); - WrapPosition(mScrollPostPosition); + Vector3 deltaPosition(mScrollPrePosition); mDomainOffset += deltaPosition - mScrollPostPosition; Self().SetProperty(mPropertyDomainOffset, mDomainOffset); - - if( mScrollStateFlags & SCROLL_X_STATE_MASK ) - { - // already performing animation on internal x position - StopAnimation(mInternalXAnimation); - } - - if( mScrollStateFlags & SCROLL_Y_STATE_MASK ) - { - // already performing animation on internal y position - StopAnimation(mInternalYAnimation); - } - - mScrollStateFlags = 0; + StopAnimation(); mScrollPostScale = GetPropertyScale(); @@ -2263,6 +2244,9 @@ void ScrollView::PreAnimatedScrollSetup() mScrollPreScale = mScrollPostScale; mScrollPreRotation = mScrollPostRotation; + + // animated scroll needs update notification + SetScrollUpdateNotification(true); } void ScrollView::FinaliseAnimatedScroll() @@ -2318,6 +2302,7 @@ void ScrollView::OnScrollAnimationFinished( Animation& source ) // update our local scroll positions UpdateLocalScrollProperties(); + mPropertiesUpdated = true; if( source == mSnapAnimation ) { @@ -2363,6 +2348,7 @@ void ScrollView::OnScrollAnimationFinished( Animation& source ) { HandleSnapAnimationFinished(); } + mPropertiesUpdated = false; } void ScrollView::OnSnapInternalPositionFinished( Animation& source ) @@ -2444,30 +2430,15 @@ void ScrollView::GestureStarted() mScaleDelta = Vector3::ONE; mRotationDelta = 0.0f; mLastVelocity = Vector2(0.0f, 0.0f); - if( !mScrolling ) - { - mLockAxis = LockPossible; - } - if( mScrollStateFlags & SCROLL_X_STATE_MASK ) + // inform application that animated scroll has been interrupted + if( mScrolling ) { - StopAnimation(mInternalXAnimation); - } - if( mScrollStateFlags & SCROLL_Y_STATE_MASK ) - { - StopAnimation(mInternalYAnimation); - } - mScrollStateFlags = 0; - - if(mScrolling) // are we interrupting a current scroll? - { - // set mScrolling to false, in case user has code that interrogates mScrolling Getter() in complete. - mScrolling = false; - // send negative scroll position since scroll internal scroll position works as an offset for actors, - // give applications the position within the domain from the scroll view's anchor position - DALI_LOG_SCROLL_STATE("[0x%X] mScrollCompletedSignalV2 5 [%.2f, %.2f]", this, -mScrollPostPosition.x, -mScrollPostPosition.y); - mScrollCompletedSignalV2.Emit( -mScrollPostPosition ); + DALI_LOG_SCROLL_STATE("[0x%X] mScrollCompletedSignalV2 4 [%.2f, %.2f]", this, GetCurrentScrollPosition().x, GetCurrentScrollPosition().y); + ScrollingStopped(GetCurrentScrollPosition()); } + DALI_LOG_SCROLL_STATE("[0x%X] mScrollStartedSignalV2 2 [%.2f, %.2f]", this, GetCurrentScrollPosition().x, GetCurrentScrollPosition().y); + ScrollingStarted(GetCurrentScrollPosition()); } } @@ -2492,6 +2463,24 @@ void ScrollView::GestureContinuing(const Vector2& panDelta, const Vector2& scale } // end if mAxisAutoLock } +void ScrollView::ScrollingStarted( const Vector3& position ) +{ + Self().SetProperty(mPropertyScrolling, true); + mScrolling = true; + mLockAxis = LockPossible; + + mScrollStartedSignalV2.Emit( position ); +} + +void ScrollView::ScrollingStopped( const Vector3& position ) +{ + Self().SetProperty(mPropertyScrolling, false); + mScrolling = false; + SetScrollUpdateNotification(false); + + mScrollCompletedSignalV2.Emit( position ); +} + // TODO: Upgrade to use a more powerful gesture detector (one that supports multiple touches on pan - so works as pan and flick gesture) // TODO: Reimplement Scaling (pinching 2+ points) // TODO: Reimplment Rotation (pinching 2+ points) @@ -2509,13 +2498,14 @@ void ScrollView::OnPan(PanGesture gesture) // this callback will still be called, so we must suppress it. return; } + UpdateLocalScrollProperties(); + mPropertiesUpdated = true; // translate Gesture input to get useful data... switch(gesture.state) { case Gesture::Started: { - UpdateLocalScrollProperties(); GestureStarted(); mPanning = true; self.SetProperty( mPropertyPanning, true ); @@ -2534,7 +2524,6 @@ void ScrollView::OnPan(PanGesture gesture) case Gesture::Finished: case Gesture::Cancelled: { - UpdateLocalScrollProperties(); mLastVelocity = gesture.velocity; mPanning = false; self.SetProperty( mPropertyPanning, false ); @@ -2543,6 +2532,11 @@ void ScrollView::OnPan(PanGesture gesture) { self.RemoveConstraint(mScrollMainInternalPrePositionConstraint); } + mGestureStackDepth--; + if(mGestureStackDepth==0) + { + FinishTransform(); + } break; } @@ -2552,37 +2546,10 @@ void ScrollView::OnPan(PanGesture gesture) // Nothing to do, not needed. break; } - } // end switch(gesture.state) - OnGestureEx(gesture.state); -} - -void ScrollView::OnGestureEx(Gesture::State state) -{ - // call necessary signals for application developer - - if(state == Gesture::Started) - { - Vector3 currentScrollPosition = GetCurrentScrollPosition(); - Self().SetProperty(mPropertyScrolling, true); - mScrolling = true; - DALI_LOG_SCROLL_STATE("[0x%X] mScrollStartedSignalV2 2 [%.2f, %.2f]", this, currentScrollPosition.x, currentScrollPosition.y); - mScrollStartedSignalV2.Emit( currentScrollPosition ); - } - else if( (state == Gesture::Finished) || - (state == Gesture::Cancelled) ) // Finished/default - { - // when all the gestures have finished, we finish the transform. - // so if a user decides to pan (1 gesture), and then pan+zoom (2 gestures) - // then stop panning (back to 1 gesture), and then stop zooming (0 gestures). - // this is the point we end, and perform necessary snapping. - mGestureStackDepth--; - if(mGestureStackDepth==0) - { - FinishTransform(); - } - } + // can no longer guarantee local properties are latest + mPropertiesUpdated = false; } void ScrollView::UpdateTransform() @@ -2614,9 +2581,15 @@ void ScrollView::FinishTransform() { SnapInternalYTo(mScrollTargetPosition.y); } - Vector3 currentScrollPosition = GetCurrentScrollPosition(); - DALI_LOG_SCROLL_STATE("[0x%X] mScrollCompletedSignalV2 6 [%.2f, %.2f]", this, currentScrollPosition.x, currentScrollPosition.y); - mScrollCompletedSignalV2.Emit( currentScrollPosition ); + if( !(mScrollStateFlags & SNAP_ANIMATION_FLAGS) ) + { + // set final position + WrapPosition(mScrollTargetPosition); + mScrollPrePosition = mScrollTargetPosition; + self.SetProperty(mPropertyPrePosition, mScrollPrePosition); + } + DALI_LOG_SCROLL_STATE("[0x%X] mScrollCompletedSignalV2 5 [%.2f, %.2f]", this, GetCurrentScrollPosition().x, GetCurrentScrollPosition().y); + ScrollingStopped(GetCurrentScrollPosition()); } } diff --git a/base/dali-toolkit/internal/controls/scrollable/scroll-view/scroll-view-impl.h b/base/dali-toolkit/internal/controls/scrollable/scroll-view/scroll-view-impl.h index 3f8562f..f60d3de 100644 --- a/base/dali-toolkit/internal/controls/scrollable/scroll-view/scroll-view-impl.h +++ b/base/dali-toolkit/internal/controls/scrollable/scroll-view/scroll-view-impl.h @@ -614,13 +614,6 @@ private: bool OnTouchDownTimeout(); /** - * Called whenever a snap animation has completed - * @param[in] source the Animation instance that has completed. - * Resets all scrolling animations and states, leaving current scroll position at mPropertyPosition - */ - void ResetScrolling(); - - /** * Updates mScrollInternalPosition, mScrollPrePosition and mScrollPostPosition from their property counterparts */ void UpdateLocalScrollProperties(); @@ -705,18 +698,25 @@ private: void GestureContinuing(const Vector2& panDelta, const Vector2& scaleDelta, float rotationDelta); /** - * Called upon pan gesture event. + * Called when either pan starts or animated scroll starts * - * @param[in] gesture The gesture event. + * @param[in] scroll position where scrolling started in positive scroll coordinates (scroll properties are negative) */ - void OnPan(PanGesture pan); + void ScrollingStarted( const Vector3& position ); /** - * Extension of the above gestures. + * Called when scrolling stops, either due to interruption from pan or when scroll animation finishes + * + * @param[in] scroll position where scrolling stopped in positive scroll coordinates (scroll properties are negative) + */ + void ScrollingStopped( const Vector3& position ); + + /** + * Called upon pan gesture event. * * @param[in] gesture The gesture event. */ - void OnGestureEx(Gesture::State state); + void OnPan(PanGesture pan); /** * Performs snapping while taking into account Velocity of gesture @@ -929,7 +929,6 @@ private: Animation mInternalXAnimation; ///< Animates mPropertyX to a snap position or application requested scroll position Animation mInternalYAnimation; ///< Animates mPropertyY to a snap position or application requested scroll position - Vector2 mLastVelocity; ///< Record the last velocity from PanGesture (Finish event doesn't have correct velocity) LockAxis mLockAxis; @@ -977,7 +976,8 @@ private: Toolkit::ScrollView::SnapStartedSignalV2 mSnapStartedSignalV2; - bool mInAccessibilityPan : 1; ///< With AccessibilityPan its easier to move between snap positions + bool mPropertiesUpdated:1; ///< Flag telling us when we can return local values to application or if we have to return the property + bool mInAccessibilityPan:1; ///< With AccessibilityPan its easier to move between snap positions bool mInitialized:1; bool mScrolling:1; ///< Flag indicating whether the scroll view is being scrolled (by user or animation) bool mScrollInterrupted:1; ///< Flag set for when a down event interrupts a scroll diff --git a/base/dali-toolkit/public-api/controls/scrollable/scroll-view/scroll-view.cpp b/base/dali-toolkit/public-api/controls/scrollable/scroll-view/scroll-view.cpp index ff8fd79..84a7df9 100644 --- a/base/dali-toolkit/public-api/controls/scrollable/scroll-view/scroll-view.cpp +++ b/base/dali-toolkit/public-api/controls/scrollable/scroll-view/scroll-view.cpp @@ -233,6 +233,10 @@ unsigned int FixedRuler::GetPageFromPosition(float position, bool wrap) const // spacing must be present. if(mEnabled && fabsf(mSpacing) > Math::MACHINE_EPSILON_1) { + if( wrap ) + { + position = WrapInDomain(position, mDomain.min, mDomain.max); + } page = floor((position - mDomain.min) / mSpacing + 0.5f); if(wrap) -- 2.7.4