- Revision
- 102170
- Author
- commit-qu...@webkit.org
- Date
- 2011-12-06 13:00:29 -0800 (Tue, 06 Dec 2011)
Log Message
[chromium] Apply sent deltas on finishCommit
https://bugs.webkit.org/show_bug.cgi?id=73884
Patch by Alexandre Elias <ael...@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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (102169 => 102170)
--- trunk/Source/WebCore/ChangeLog 2011-12-06 20:48:27 UTC (rev 102169)
+++ trunk/Source/WebCore/ChangeLog 2011-12-06 21:00:29 UTC (rev 102170)
@@ -1,3 +1,40 @@
+2011-12-06 Alexandre Elias <ael...@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 <barraclo...@apple.com>
https://bugs.webkit.org/show_bug.cgi?id=68328
Modified: trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.cpp (102169 => 102170)
--- trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.cpp 2011-12-06 20:48:27 UTC (rev 102169)
+++ trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.cpp 2011-12-06 21:00:29 UTC (rev 102170)
@@ -293,6 +293,8 @@
layer->setSublayerTransform(m_sublayerTransform);
layer->setTransform(m_transform);
layer->setUpdateRect(m_updateRect);
+
+ layer->setScrollDelta(layer->scrollDelta() - layer->sentScrollDelta());
layer->setSentScrollDelta(IntSize());
if (maskLayer())
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp (102169 => 102170)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp 2011-12-06 20:48:27 UTC (rev 102169)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp 2011-12-06 21:00:29 UTC (rev 102170)
@@ -140,11 +140,6 @@
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 @@
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++;
}
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp (102169 => 102170)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp 2011-12-06 20:48:27 UTC (rev 102169)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp 2011-12-06 21:00:29 UTC (rev 102170)
@@ -212,13 +212,13 @@
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::applyPageScaleDeltaToScrollLayer()
{
if (m_scrollLayerImpl)
- m_scrollLayerImpl->setPageScaleDelta(m_pageScaleDelta * m_sentPageScaleDelta);
+ m_scrollLayerImpl->setPageScaleDelta(m_pageScaleDelta);
}
void CCLayerTreeHostImpl::updateMaxScrollPosition()
@@ -370,8 +370,6 @@
}
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,13 +377,8 @@
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());
+ m_scrollLayerImpl->setSentScrollDelta(scroll.scrollDelta);
- adjustScrollsForPageScaleChange(m_sentPageScaleDelta);
-
return scrollInfo.release();
}
Modified: trunk/Source/WebKit/chromium/ChangeLog (102169 => 102170)
--- trunk/Source/WebKit/chromium/ChangeLog 2011-12-06 20:48:27 UTC (rev 102169)
+++ trunk/Source/WebKit/chromium/ChangeLog 2011-12-06 21:00:29 UTC (rev 102170)
@@ -1,3 +1,32 @@
+2011-12-06 Alexandre Elias <ael...@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 <aba...@webkit.org>
Remove forwarding header now that downstream has been fixed to refer to
Modified: trunk/Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp (102169 => 102170)
--- trunk/Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp 2011-12-06 20:48:27 UTC (rev 102169)
+++ trunk/Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp 2011-12-06 21:00:29 UTC (rev 102170)
@@ -125,25 +125,20 @@
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)
