- Revision
- 116195
- Author
- [email protected]
- Date
- 2012-05-04 17:38:03 -0700 (Fri, 04 May 2012)
Log Message
[chromium] Changes to layer tree structure need to be tracked properly
https://bugs.webkit.org/show_bug.cgi?id=85421
Reviewed by Adrienne Walker.
Unit test added: TreeSynchronizerTest.syncSimpleTreeAndTrackStackingOrderChange
Source/WebCore:
Earlier, we were relying on WebCore behavior that always called
setNeedsDisplay whenever the layer tree structure changed.
However, in general it is more correct to consider layer tree
changes even when things don't need repainting; for example Aura
code is encountring this bug now. This patch corrects the
compositor so that layer tree structural changes are considered
property changes, without requiring that layers needed to be
repainted.
* platform/graphics/chromium/LayerChromium.cpp:
(WebCore::LayerChromium::LayerChromium):
(WebCore::LayerChromium::insertChild):
(WebCore::LayerChromium::pushPropertiesTo):
* platform/graphics/chromium/LayerChromium.h:
(LayerChromium):
* platform/graphics/chromium/cc/CCLayerImpl.cpp:
(WebCore::CCLayerImpl::setStackingOrderChanged):
(WebCore):
* platform/graphics/chromium/cc/CCLayerImpl.h:
(CCLayerImpl):
Source/WebKit/chromium:
* tests/TreeSynchronizerTest.cpp:
(WebKitTests):
(WebKitTests::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (116194 => 116195)
--- trunk/Source/WebCore/ChangeLog 2012-05-05 00:10:37 UTC (rev 116194)
+++ trunk/Source/WebCore/ChangeLog 2012-05-05 00:38:03 UTC (rev 116195)
@@ -1,3 +1,33 @@
+2012-05-04 Shawn Singh <[email protected]>
+
+ [chromium] Changes to layer tree structure need to be tracked properly
+ https://bugs.webkit.org/show_bug.cgi?id=85421
+
+ Reviewed by Adrienne Walker.
+
+ Unit test added: TreeSynchronizerTest.syncSimpleTreeAndTrackStackingOrderChange
+
+ Earlier, we were relying on WebCore behavior that always called
+ setNeedsDisplay whenever the layer tree structure changed.
+ However, in general it is more correct to consider layer tree
+ changes even when things don't need repainting; for example Aura
+ code is encountring this bug now. This patch corrects the
+ compositor so that layer tree structural changes are considered
+ property changes, without requiring that layers needed to be
+ repainted.
+
+ * platform/graphics/chromium/LayerChromium.cpp:
+ (WebCore::LayerChromium::LayerChromium):
+ (WebCore::LayerChromium::insertChild):
+ (WebCore::LayerChromium::pushPropertiesTo):
+ * platform/graphics/chromium/LayerChromium.h:
+ (LayerChromium):
+ * platform/graphics/chromium/cc/CCLayerImpl.cpp:
+ (WebCore::CCLayerImpl::setStackingOrderChanged):
+ (WebCore):
+ * platform/graphics/chromium/cc/CCLayerImpl.h:
+ (CCLayerImpl):
+
2012-05-04 Jeffrey Pfau <[email protected]>
Unreviewed; build fix after r116191.
Modified: trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.cpp (116194 => 116195)
--- trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.cpp 2012-05-05 00:10:37 UTC (rev 116194)
+++ trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.cpp 2012-05-05 00:38:03 UTC (rev 116195)
@@ -57,6 +57,7 @@
LayerChromium::LayerChromium()
: m_needsDisplay(false)
+ , m_stackingOrderChanged(false)
, m_layerId(s_nextLayerId++)
, m_parent(0)
, m_layerTreeHost(0)
@@ -153,6 +154,7 @@
index = min(index, m_children.size());
child->removeFromParent();
child->setParent(this);
+ child->m_stackingOrderChanged = true;
m_children.insert(index, child);
setNeedsCommit();
}
@@ -514,6 +516,8 @@
layer->setScrollDelta(layer->scrollDelta() - layer->sentScrollDelta());
layer->setSentScrollDelta(IntSize());
+ layer->setStackingOrderChanged(m_stackingOrderChanged);
+
if (maskLayer())
maskLayer()->pushPropertiesTo(layer->maskLayer());
if (replicaLayer())
@@ -522,6 +526,7 @@
m_layerAnimationController->pushAnimationUpdatesTo(layer->layerAnimationController());
// Reset any state that should be cleared for the next update.
+ m_stackingOrderChanged = false;
m_updateRect = FloatRect();
}
Modified: trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.h (116194 => 116195)
--- trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.h 2012-05-05 00:10:37 UTC (rev 116194)
+++ trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.h 2012-05-05 00:38:03 UTC (rev 116195)
@@ -264,6 +264,9 @@
// This flag is set when layer need repainting/updating.
bool m_needsDisplay;
+ // Tracks whether this layer may have changed stacking order with its siblings.
+ bool m_stackingOrderChanged;
+
// The update rect is the region of the compositor resource that was actually updated by the compositor.
// For layers that may do updating outside the compositor's control (i.e. plugin layers), this information
// is not available and the update rect will remain empty.
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp (116194 => 116195)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp 2012-05-05 00:10:37 UTC (rev 116194)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp 2012-05-05 00:38:03 UTC (rev 116195)
@@ -265,6 +265,13 @@
m_children[i]->dumpLayer(ts, indent+1);
}
+void CCLayerImpl::setStackingOrderChanged(bool stackingOrderChanged)
+{
+ // We don't need to store this flag; we only need to track that the change occurred.
+ if (stackingOrderChanged)
+ noteLayerPropertyChangedForSubtree();
+}
+
void CCLayerImpl::noteLayerPropertyChangedForSubtree()
{
m_layerPropertyChanged = true;
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h (116194 => 116195)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h 2012-05-05 00:10:37 UTC (rev 116194)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h 2012-05-05 00:38:03 UTC (rev 116195)
@@ -229,6 +229,8 @@
String layerTreeAsText() const;
+ void setStackingOrderChanged(bool);
+
bool layerPropertyChanged() const { return m_layerPropertyChanged; }
void resetAllChangeTrackingForSubtree();
Modified: trunk/Source/WebKit/chromium/ChangeLog (116194 => 116195)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-05-05 00:10:37 UTC (rev 116194)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-05-05 00:38:03 UTC (rev 116195)
@@ -1,3 +1,16 @@
+2012-05-04 Shawn Singh <[email protected]>
+
+ [chromium] Changes to layer tree structure need to be tracked properly
+ https://bugs.webkit.org/show_bug.cgi?id=85421
+
+ Reviewed by Adrienne Walker.
+
+ Unit test added: TreeSynchronizerTest.syncSimpleTreeAndTrackStackingOrderChange
+
+ * tests/TreeSynchronizerTest.cpp:
+ (WebKitTests):
+ (WebKitTests::TEST):
+
2012-05-04 Tony Chang <[email protected]>
[chromium] enable msvs_error_on_missing_sources at gyp time
Modified: trunk/Source/WebKit/chromium/tests/TreeSynchronizerTest.cpp (116194 => 116195)
--- trunk/Source/WebKit/chromium/tests/TreeSynchronizerTest.cpp 2012-05-05 00:10:37 UTC (rev 116194)
+++ trunk/Source/WebKit/chromium/tests/TreeSynchronizerTest.cpp 2012-05-05 00:38:03 UTC (rev 116195)
@@ -197,6 +197,34 @@
EXPECT_EQ(secondCCLayerId, ccLayerDestructionList[0]);
}
+// Constructs a very simple tree and checks that a stacking-order change is tracked properly.
+TEST(TreeSynchronizerTest, syncSimpleTreeAndTrackStackingOrderChange)
+{
+ DebugScopedSetImplThread impl;
+ Vector<int> ccLayerDestructionList;
+
+ // Set up the tree and sync once. child2 needs to be synced here, too, even though we
+ // remove it to set up the intended scenario.
+ RefPtr<LayerChromium> layerTreeRoot = MockLayerChromium::create(&ccLayerDestructionList);
+ RefPtr<LayerChromium> child2 = MockLayerChromium::create(&ccLayerDestructionList);
+ layerTreeRoot->addChild(MockLayerChromium::create(&ccLayerDestructionList));
+ layerTreeRoot->addChild(child2);
+ OwnPtr<CCLayerImpl> ccLayerTreeRoot = TreeSynchronizer::synchronizeTrees(layerTreeRoot.get(), nullptr);
+ expectTreesAreIdentical(layerTreeRoot.get(), ccLayerTreeRoot.get());
+ ccLayerTreeRoot->resetAllChangeTrackingForSubtree();
+
+ // re-insert the layer and sync again.
+ child2->removeFromParent();
+ layerTreeRoot->addChild(child2);
+ ccLayerTreeRoot = TreeSynchronizer::synchronizeTrees(layerTreeRoot.get(), ccLayerTreeRoot.release());
+ expectTreesAreIdentical(layerTreeRoot.get(), ccLayerTreeRoot.get());
+
+ // Check that the impl thread properly tracked the change.
+ EXPECT_FALSE(ccLayerTreeRoot->layerPropertyChanged());
+ EXPECT_FALSE(ccLayerTreeRoot->children()[0]->layerPropertyChanged());
+ EXPECT_TRUE(ccLayerTreeRoot->children()[1]->layerPropertyChanged());
+}
+
TEST(TreeSynchronizerTest, syncSimpleTreeAndProperties)
{
DebugScopedSetImplThread impl;
