From 484078e368c1718cc3b1508931eb6b9f5e561fd5 Mon Sep 17 00:00:00 2001 From: Steve Kobes Date: Thu, 10 Mar 2016 11:40:18 -0800 Subject: [PATCH] Account for scroll origin in scroll animators. Blink and cc have different notions of scroll offset - Blink's is relative to the scroll origin which is non-zero in RTL documents. The CompositorScrollOffsetAnimationCurve must work entirely in cc scroll offsets since it is handed over to cc which doesn't know about the scroll origin. This patch teaches ScrollAnimator and ProgrammaticScrollAnimator to convert in both directions when creating and using the curve object. BUG=593186 Review URL: https://codereview.chromium.org/1776503002 Cr-Commit-Position: refs/heads/master@{#379974} (cherry picked from commit 0bb16600006793953292ee1e7fd8bf0a66f575dc) Review URL: https://codereview.chromium.org/1780993003 . Cr-Commit-Position: refs/branch-heads/2661@{#183} Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081} --- .../WebKit/LayoutTests/TestExpectations | 1 + ...rizontal-smooth-scroll-in-rtl-expected.txt | 11 +++++ .../horizontal-smooth-scroll-in-rtl.html | 40 +++++++++++++++++++ .../scroll/ProgrammaticScrollAnimator.cpp | 10 +++-- .../Source/platform/scroll/ScrollAnimator.cpp | 14 ++++--- .../ScrollAnimatorCompositorCoordinator.cpp | 12 ++++++ .../ScrollAnimatorCompositorCoordinator.h | 4 ++ 7 files changed, 82 insertions(+), 10 deletions(-) create mode 100644 third_party/WebKit/LayoutTests/virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl-expected.txt create mode 100644 third_party/WebKit/LayoutTests/virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl.html diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 78731925dd723..10551b300efdb 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations @@ -919,6 +919,7 @@ crbug.com/364614 [ Mac ] virtual/threaded/fast/scroll-behavior/overflow-scroll-r crbug.com/364614 [ Mac ] virtual/scroll_customization/fast/scroll-behavior/overflow-scroll-root-frame-animates.html [ Skip ] crbug.com/574283 [ Mac ] virtual/threaded/fast/scroll-behavior/smooth-scroll/fixed-background-in-iframe.html [ Skip ] +crbug.com/574283 [ Mac ] virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl.html [ Skip ] crbug.com/574283 [ Mac ] virtual/threaded/fast/scroll-behavior/smooth-scroll/main-thread-scrolling-reason-added.html [ Skip ] crbug.com/574283 [ Mac ] virtual/threaded/fast/scroll-behavior/smooth-scroll/main-thread-scrolling-reason-correctness.html [ Skip ] diff --git a/third_party/WebKit/LayoutTests/virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl-expected.txt b/third_party/WebKit/LayoutTests/virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl-expected.txt new file mode 100644 index 0000000000000..955b971667452 --- /dev/null +++ b/third_party/WebKit/LayoutTests/virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl-expected.txt @@ -0,0 +1,11 @@ +This test verifies that both input-driven and programmatic smooth scrolls serviced by the compositor thread go to the correct scroll position on RTL pages with horizontal overflow. + +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". + + +PASS scrollX became -80 +PASS scrollX became -40 +PASS successfullyParsed is true + +TEST COMPLETE + diff --git a/third_party/WebKit/LayoutTests/virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl.html b/third_party/WebKit/LayoutTests/virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl.html new file mode 100644 index 0000000000000..e011d22e294cc --- /dev/null +++ b/third_party/WebKit/LayoutTests/virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl.html @@ -0,0 +1,40 @@ + + + + diff --git a/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp b/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp index 99f58fd7b3390..437bee38681c0 100644 --- a/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp +++ b/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp @@ -56,7 +56,7 @@ void ProgrammaticScrollAnimator::animateToOffset(FloatPoint offset) m_startTime = 0.0; m_targetOffset = offset; m_animationCurve = adoptPtr(CompositorFactory::current().createScrollOffsetAnimationCurve( - m_targetOffset, + compositorOffsetFromBlinkOffset(m_targetOffset), CompositorAnimationCurve::TimingFunctionTypeEaseInOut, CompositorScrollOffsetAnimationCurve::ScrollDurationDeltaBased)); @@ -83,7 +83,7 @@ void ProgrammaticScrollAnimator::tickAnimation(double monotonicTime) m_startTime = monotonicTime; double elapsedTime = monotonicTime - m_startTime; bool isFinished = (elapsedTime > m_animationCurve->duration()); - FloatPoint offset = m_animationCurve->getValue(elapsedTime); + FloatPoint offset = blinkOffsetFromCompositorOffset(m_animationCurve->getValue(elapsedTime)); notifyPositionChanged(IntPoint(offset.x(), offset.y())); if (isFinished) { @@ -139,7 +139,8 @@ void ProgrammaticScrollAnimator::updateCompositorAnimations() if (!sentToCompositor) { m_runState = RunState::RunningOnMainThread; - m_animationCurve->setInitialValue(FloatPoint(m_scrollableArea->scrollPosition())); + m_animationCurve->setInitialValue(compositorOffsetFromBlinkOffset( + FloatPoint(m_scrollableArea->scrollPosition()))); if (!m_scrollableArea->scheduleAnimation()) { notifyPositionChanged(IntPoint(m_targetOffset.x(), m_targetOffset.y())); resetAnimationState(); @@ -158,7 +159,8 @@ void ProgrammaticScrollAnimator::layerForCompositedScrollingDidChange(Compositor m_runState = RunState::RunningOnMainThread; m_compositorAnimationId = 0; m_compositorAnimationGroupId = 0; - m_animationCurve->setInitialValue(FloatPoint(m_scrollableArea->scrollPosition())); + m_animationCurve->setInitialValue(compositorOffsetFromBlinkOffset( + FloatPoint(m_scrollableArea->scrollPosition()))); m_scrollableArea->registerForAnimation(); if (!m_scrollableArea->scheduleAnimation()) { resetAnimationState(); diff --git a/third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp b/third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp index abb8afc4a0ad8..8286233099689 100644 --- a/third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp +++ b/third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp @@ -151,7 +151,8 @@ bool ScrollAnimator::willAnimateToOffset(const FloatPoint& targetPos) // Running on the main thread, simply update the target offset instead // of sending to the compositor. if (m_runState == RunState::RunningOnMainThread) { - m_animationCurve->updateTarget(m_timeFunction() - m_startTime, targetPos); + m_animationCurve->updateTarget(m_timeFunction() - m_startTime, + compositorOffsetFromBlinkOffset(targetPos)); return true; } @@ -190,8 +191,9 @@ void ScrollAnimator::tickAnimation(double monotonicTime) double elapsedTime = monotonicTime - m_startTime; bool isFinished = (elapsedTime > m_animationCurve->duration()); - FloatPoint offset = isFinished ? m_animationCurve->targetValue() - : m_animationCurve->getValue(elapsedTime); + FloatPoint offset = blinkOffsetFromCompositorOffset(isFinished + ? m_animationCurve->targetValue() + : m_animationCurve->getValue(elapsedTime)); offset = FloatPoint(m_scrollableArea->clampScrollPosition(offset)); @@ -290,18 +292,18 @@ void ScrollAnimator::updateCompositorAnimations() m_compositorAnimationGroupId = 0; m_animationCurve->updateTarget(m_timeFunction() - m_startTime, - m_targetOffset); + compositorOffsetFromBlinkOffset(m_targetOffset)); m_runState = RunState::WaitingToSendToCompositor; } if (!m_animationCurve) { m_animationCurve = adoptPtr(CompositorFactory::current().createScrollOffsetAnimationCurve( - m_targetOffset, + compositorOffsetFromBlinkOffset(m_targetOffset), CompositorAnimationCurve::TimingFunctionTypeEaseInOut, m_lastGranularity == ScrollByPixel ? CompositorScrollOffsetAnimationCurve::ScrollDurationInverseDelta : CompositorScrollOffsetAnimationCurve::ScrollDurationConstant)); - m_animationCurve->setInitialValue(currentPosition()); + m_animationCurve->setInitialValue(compositorOffsetFromBlinkOffset(currentPosition())); } bool runningOnMainThread = false; diff --git a/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.cpp b/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.cpp index 27dfece11f6ad..690cc059858f8 100644 --- a/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.cpp +++ b/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.cpp @@ -246,4 +246,16 @@ CompositorAnimationPlayer* ScrollAnimatorCompositorCoordinator::compositorPlayer return m_compositorPlayer.get(); } +FloatPoint ScrollAnimatorCompositorCoordinator::compositorOffsetFromBlinkOffset(FloatPoint offset) +{ + offset.moveBy(scrollableArea()->scrollOrigin()); + return offset; +} + +FloatPoint ScrollAnimatorCompositorCoordinator::blinkOffsetFromCompositorOffset(FloatPoint offset) +{ + offset.moveBy(-scrollableArea()->scrollOrigin()); + return offset; +} + } // namespace blink diff --git a/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.h b/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.h index 4ebf061b30028..26abcd0dc53ce 100644 --- a/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.h +++ b/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.h @@ -9,6 +9,7 @@ #include "cc/animation/animation_curve.h" #include "platform/PlatformExport.h" #include "platform/animation/CompositorAnimationPlayerClient.h" +#include "platform/geometry/FloatPoint.h" #include "platform/heap/Handle.h" #include "public/platform/WebCompositorAnimationDelegate.h" #include "wtf/Allocator.h" @@ -57,6 +58,9 @@ class PLATFORM_EXPORT ScrollAnimatorCompositorCoordinator : public NoBaseWillBeG void removeAnimation(); virtual void abortAnimation(); + FloatPoint compositorOffsetFromBlinkOffset(FloatPoint); + FloatPoint blinkOffsetFromCompositorOffset(FloatPoint); + void compositorAnimationFinished(int groupId); // Returns true if the compositor player was attached to a new layer. bool reattachCompositorPlayerIfNeeded(CompositorAnimationTimeline*);