[chromium] Apply sent deltas on finishCommit
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Dec 2011 21:00:29 +0000 (21:00 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Dec 2011 21:00:29 +0000 (21:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=73884

Patch by Alexandre Elias <aelias@google.com> on 2011-12-06
Reviewed by James Robinson.

This moves scroll and pageScale "sent" deltas to be applied to
the layer at the end of the commit, instead of the beginning.

This has several advantages, especially for page scale:
- When pageScale changes, no longer any need to change the scroll's
coordinate space at beginning of commit, which is complex and prone to
bugs (this fixes a problem where we were forgetting to modify the
scrollPosition before).
- No need for non-commit-related code to consider the "sent" values.
m_pageScale is now always the content scale factor, and
m_pageScaleDelta is the scale to be on the impl-side matrix.
- This will make it easy to send arbitrary fake or future delta
values for example while pinch zooming out.

The scroll logic is similarly altered for consistency's sake.  Note that
I also moved the tree synchronize to the beginning of finishCommit
in order to avoid having to change the pageScale coordinate space of
sentScrollDelta in adjustScrollsForPageScaleChange().

Source/WebCore:

No new tests. (Refactoring of existing code.)

* platform/graphics/chromium/LayerChromium.cpp:
(WebCore::LayerChromium::pushPropertiesTo):
* platform/graphics/chromium/cc/CCLayerTreeHost.cpp:
(WebCore::CCLayerTreeHost::finishCommitOnImplThread):
* platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:
(WebCore::CCLayerTreeHostImpl::setPageScaleFactorAndLimits):
(WebCore::CCLayerTreeHostImpl::applyPageScaleDeltaToScrollLayer):
(WebCore::CCLayerTreeHostImpl::processScrollDeltas):

Source/WebKit/chromium:

* tests/CCLayerTreeHostImplTest.cpp:
(WebKit::TEST_F):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@102170 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/chromium/LayerChromium.cpp
Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp
Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp

index 829f76d..e5fea13 100644 (file)
@@ -1,3 +1,40 @@
+2011-12-06  Alexandre Elias  <aelias@google.com>
+
+        [chromium] Apply sent deltas on finishCommit
+        https://bugs.webkit.org/show_bug.cgi?id=73884
+
+        Reviewed by James Robinson.
+
+        This moves scroll and pageScale "sent" deltas to be applied to
+        the layer at the end of the commit, instead of the beginning.
+
+        This has several advantages, especially for page scale:
+        - When pageScale changes, no longer any need to change the scroll's
+        coordinate space at beginning of commit, which is complex and prone to
+        bugs (this fixes a problem where we were forgetting to modify the
+        scrollPosition before).
+        - No need for non-commit-related code to consider the "sent" values.
+        m_pageScale is now always the content scale factor, and
+        m_pageScaleDelta is the scale to be on the impl-side matrix.
+        - This will make it easy to send arbitrary fake or future delta
+        values for example while pinch zooming out.
+
+        The scroll logic is similarly altered for consistency's sake.  Note that
+        I also moved the tree synchronize to the beginning of finishCommit
+        in order to avoid having to change the pageScale coordinate space of
+        sentScrollDelta in adjustScrollsForPageScaleChange().
+
+        No new tests. (Refactoring of existing code.)
+
+        * platform/graphics/chromium/LayerChromium.cpp:
+        (WebCore::LayerChromium::pushPropertiesTo):
+        * platform/graphics/chromium/cc/CCLayerTreeHost.cpp:
+        (WebCore::CCLayerTreeHost::finishCommitOnImplThread):
+        * platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:
+        (WebCore::CCLayerTreeHostImpl::setPageScaleFactorAndLimits):
+        (WebCore::CCLayerTreeHostImpl::applyPageScaleDeltaToScrollLayer):
+        (WebCore::CCLayerTreeHostImpl::processScrollDeltas):
+
 2011-12-06  Gavin Barraclough  <barraclough@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=68328
index 0d75fdb..e257322 100644 (file)
@@ -293,6 +293,8 @@ void LayerChromium::pushPropertiesTo(CCLayerImpl* layer)
     layer->setSublayerTransform(m_sublayerTransform);
     layer->setTransform(m_transform);
     layer->setUpdateRect(m_updateRect);
+
+    layer->setScrollDelta(layer->scrollDelta() - layer->sentScrollDelta());
     layer->setSentScrollDelta(IntSize());
 
     if (maskLayer())
index 27c1704..27d956a 100644 (file)
@@ -140,11 +140,6 @@ void CCLayerTreeHost::beginCommitOnImplThread(CCLayerTreeHostImpl* hostImpl)
 void CCLayerTreeHost::finishCommitOnImplThread(CCLayerTreeHostImpl* hostImpl)
 {
     ASSERT(CCProxy::isImplThread());
-    hostImpl->setSourceFrameNumber(frameNumber());
-    hostImpl->setHaveWheelEventHandlers(m_haveWheelEventHandlers);
-    hostImpl->setZoomAnimatorTransform(m_zoomAnimatorTransform);
-    hostImpl->setViewport(viewportSize());
-    hostImpl->setPageScaleFactorAndLimits(pageScale(), m_minPageScale, m_maxPageScale);
 
     // Synchronize trees, if one exists at all...
     if (rootLayer())
@@ -152,6 +147,12 @@ void CCLayerTreeHost::finishCommitOnImplThread(CCLayerTreeHostImpl* hostImpl)
     else
         hostImpl->setRootLayer(0);
 
+    hostImpl->setSourceFrameNumber(frameNumber());
+    hostImpl->setHaveWheelEventHandlers(m_haveWheelEventHandlers);
+    hostImpl->setZoomAnimatorTransform(m_zoomAnimatorTransform);
+    hostImpl->setViewport(viewportSize());
+    hostImpl->setPageScaleFactorAndLimits(pageScale(), m_minPageScale, m_maxPageScale);
+
     m_frameNumber++;
 }
 
index 150b93d..c393bb7 100644 (file)
@@ -212,13 +212,13 @@ void CCLayerTreeHostImpl::setPageScaleFactorAndLimits(float pageScale, float min
 
     float pageScaleChange = pageScale / m_pageScale;
     m_pageScale = pageScale;
-    m_sentPageScaleDelta = 1;
+
+    adjustScrollsForPageScaleChange(pageScaleChange);
 
     // Clamp delta to limits and refresh display matrix.
-    setPageScaleDelta(m_pageScaleDelta);
+    setPageScaleDelta(m_pageScaleDelta / m_sentPageScaleDelta);
+    m_sentPageScaleDelta = 1;
     applyPageScaleDeltaToScrollLayer();
-
-    adjustScrollsForPageScaleChange(pageScaleChange);
 }
 
 void CCLayerTreeHostImpl::adjustScrollsForPageScaleChange(float pageScaleChange)
@@ -255,7 +255,7 @@ void CCLayerTreeHostImpl::setPageScaleDelta(float delta)
 void CCLayerTreeHostImpl::applyPageScaleDeltaToScrollLayer()
 {
     if (m_scrollLayerImpl)
-        m_scrollLayerImpl->setPageScaleDelta(m_pageScaleDelta * m_sentPageScaleDelta);
+        m_scrollLayerImpl->setPageScaleDelta(m_pageScaleDelta);
 }
 
 void CCLayerTreeHostImpl::updateMaxScrollPosition()
@@ -370,8 +370,6 @@ PassOwnPtr<CCScrollAndScaleSet> CCLayerTreeHostImpl::processScrollDeltas()
     }
 
     m_sentPageScaleDelta = scrollInfo->pageScaleDelta = m_pageScaleDelta;
-    m_pageScale = m_pageScale * m_sentPageScaleDelta;
-    setPageScaleDelta(1);
 
     // FIXME: track scrolls from layers other than the root
     CCLayerTreeHostCommon::ScrollUpdateInfo scroll;
@@ -379,12 +377,7 @@ PassOwnPtr<CCScrollAndScaleSet> CCLayerTreeHostImpl::processScrollDeltas()
     scroll.scrollDelta = m_scrollLayerImpl->scrollDelta();
     scrollInfo->scrolls.append(scroll);
 
-    m_scrollLayerImpl->setScrollPosition(m_scrollLayerImpl->scrollPosition() + m_scrollLayerImpl->scrollDelta());
-    m_scrollLayerImpl->setPosition(m_scrollLayerImpl->position() - m_scrollLayerImpl->scrollDelta());
-    m_scrollLayerImpl->setSentScrollDelta(m_scrollLayerImpl->scrollDelta());
-    m_scrollLayerImpl->setScrollDelta(IntSize());
-
-    adjustScrollsForPageScaleChange(m_sentPageScaleDelta);
+    m_scrollLayerImpl->setSentScrollDelta(scroll.scrollDelta);
 
     return scrollInfo.release();
 }
index 0ca5cfb..fe0082e 100644 (file)
@@ -1,3 +1,32 @@
+2011-12-06  Alexandre Elias  <aelias@google.com>
+
+        [chromium] Apply sent deltas on finishCommit
+        https://bugs.webkit.org/show_bug.cgi?id=73884
+
+        Reviewed by James Robinson.
+
+        This moves scroll and pageScale "sent" deltas to be applied to
+        the layer at the end of the commit, instead of the beginning.
+
+        This has several advantages, especially for page scale:
+        - When pageScale changes, no longer any need to change the scroll's
+        coordinate space at beginning of commit, which is complex and prone to
+        bugs (this fixes a problem where we were forgetting to modify the
+        scrollPosition before).
+        - No need for non-commit-related code to consider the "sent" values.
+        m_pageScale is now always the content scale factor, and
+        m_pageScaleDelta is the scale to be on the impl-side matrix.
+        - This will make it easy to send arbitrary fake or future delta
+        values for example while pinch zooming out.
+
+        The scroll logic is similarly altered for consistency's sake.  Note that
+        I also moved the tree synchronize to the beginning of finishCommit
+        in order to avoid having to change the pageScale coordinate space of
+        sentScrollDelta in adjustScrollsForPageScaleChange().
+
+        * tests/CCLayerTreeHostImplTest.cpp:
+        (WebKit::TEST_F):
+
 2011-12-06  Adam Barth  <abarth@webkit.org>
 
         Remove forwarding header now that downstream has been fixed to refer to
index 5acb925..e835ab7 100644 (file)
@@ -125,25 +125,20 @@ TEST_F(CCLayerTreeHostImplTest, scrollDeltaRepeatedScrolls)
     OwnPtr<CCScrollAndScaleSet> scrollInfo;
 
     scrollInfo = m_hostImpl->processScrollDeltas();
-    ASSERT_EQ(root->scrollPosition(), scrollPosition + scrollDelta);
     ASSERT_EQ(scrollInfo->scrolls.size(), 1u);
     EXPECT_EQ(root->sentScrollDelta(), scrollDelta);
     expectContains(*scrollInfo.get(), root->id(), scrollDelta);
-    expectClearedScrollDeltasRecursive(root.get());
 
     IntSize scrollDelta2(-5, 27);
     root->scrollBy(scrollDelta2);
     scrollInfo = m_hostImpl->processScrollDeltas();
-    ASSERT_EQ(root->scrollPosition(), scrollPosition + scrollDelta + scrollDelta2);
     ASSERT_EQ(scrollInfo->scrolls.size(), 1u);
-    expectContains(*scrollInfo.get(), root->id(), scrollDelta2);
-    expectClearedScrollDeltasRecursive(root.get());
+    EXPECT_EQ(root->sentScrollDelta(), scrollDelta + scrollDelta2);
+    expectContains(*scrollInfo.get(), root->id(), scrollDelta + scrollDelta2);
 
     root->scrollBy(IntSize());
     scrollInfo = m_hostImpl->processScrollDeltas();
-    ASSERT_EQ(root->scrollPosition(), scrollPosition + scrollDelta + scrollDelta2);
-    ASSERT_EQ(scrollInfo->scrolls.size(), 0u);
-    expectClearedScrollDeltasRecursive(root.get());
+    EXPECT_EQ(root->sentScrollDelta(), scrollDelta + scrollDelta2);
 }
 
 TEST_F(CCLayerTreeHostImplTest, scrollRootCallsCommitAndRedraw)