Modified: trunk/Source/WebCore/ChangeLog (104001 => 104002)
--- trunk/Source/WebCore/ChangeLog 2012-01-04 03:34:18 UTC (rev 104001)
+++ trunk/Source/WebCore/ChangeLog 2012-01-04 03:36:46 UTC (rev 104002)
@@ -1,3 +1,26 @@
+2012-01-03 Shawn Singh <[email protected]>
+
+ [chromium] CCLayerSorter accidentally reverses order of some layers.
+ https://bugs.webkit.org/show_bug.cgi?id=75046
+
+ Reviewed by James Robinson.
+
+ Unit test added to CCLayerSorterTest.cpp
+
+ Fixes two related bugs that were causing z-ordering problems in
+ layers when preserves3D triggers the need for layer sorting.
+
+ The first problem was that CCLayerSorter accidentally reversed the
+ order of layers when there was no sorting dependency between them.
+
+ The second problem was that zDiff had numerical precision problems
+ that forced sorting dependencies that were sometimes incorrect,
+ when the dependencies should not have existed.
+
+ * platform/graphics/chromium/cc/CCLayerSorter.cpp:
+ (WebCore::CCLayerSorter::checkOverlap):
+ (WebCore::CCLayerSorter::sort):
+
2012-01-03 Leo Yang <[email protected]>
[BlackBerry] Add support of blob form data to the BlackBerry port
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerSorter.cpp (104001 => 104002)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerSorter.cpp 2012-01-04 03:34:18 UTC (rev 104001)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerSorter.cpp 2012-01-04 03:36:46 UTC (rev 104002)
@@ -29,6 +29,7 @@
#include "TransformationMatrix.h"
#include "cc/CCRenderSurface.h"
#include <limits.h>
+#include <wtf/Deque.h>
using namespace std;
@@ -259,13 +260,17 @@
if (!a->shape.boundingBox.intersects(b->shape.boundingBox))
return None;
- // Make the early exit threshold proportional to the total Z range.
+ // These thresholds are defined relative to the total Z range. If further Z-fighting
+ // bugs come up due to precision errors, it may be worth considering doing an ulp
+ // error measurement instead.
float exitThreshold = m_zRange * 0.01;
+ float nonZeroThreshold = max(exitThreshold, 0.001f);
+
float zDiff = calculateZDiff(a->shape, b->shape, exitThreshold);
- if (zDiff > 0)
+ if (zDiff > nonZeroThreshold)
return BBeforeA;
- if (zDiff < 0)
+ if (zDiff < -nonZeroThreshold)
return ABeforeB;
return None;
@@ -388,13 +393,20 @@
// cyclical order dependencies. Cycles and intersections are broken aribtrarily.
// Sorting of layers is done via a topological sort of a directed graph whose nodes are
// the layers themselves. An edge from node A to node B signifies that layer A needs to
-// be drawn before layer B.
+// be drawn before layer B. If A and B have no dependency between each other, then we
+// preserve the ordering of those layers as they were in the original list.
+//
// The draw order between two layers is determined by projecting the two triangles making
// up each layer quad to the Z = 0 plane, finding points of intersection between the triangles
// and backprojecting those points to the plane of the layer to determine the corresponding Z
// coordinate. The layer with the lower Z coordinate (farther from the eye) needs to be rendered
-// first. If the layer projections don't intersect then the layers can drawn in any order and no
-// edges are created between them in the graph.
+// first.
+//
+// If the layer projections don't intersect, then no edges (dependencies) are created
+// between them in the graph. HOWEVER, in this case we still need to preserve the ordering
+// of the original list of layers, since that list should already have proper z-index
+// ordering of layers.
+//
void CCLayerSorter::sort(LayerList::iterator first, LayerList::iterator last)
{
#if !defined( NDEBUG )
@@ -405,7 +417,7 @@
createGraphEdges();
Vector<GraphNode*> sortedList;
- Vector<GraphNode*> noIncomingEdgeNodeList;
+ Deque<GraphNode*> noIncomingEdgeNodeList;
// Find all the nodes that don't have incoming edges.
for (NodeList::iterator la = m_nodes.begin(); la < m_nodes.end(); la++) {
@@ -418,10 +430,13 @@
#endif
while (m_activeEdges.size() || noIncomingEdgeNodeList.size()) {
while (noIncomingEdgeNodeList.size()) {
- // Pop the last entry from the list.
- GraphNode* fromNode = noIncomingEdgeNodeList[noIncomingEdgeNodeList.size() - 1];
- noIncomingEdgeNodeList.removeLast();
+ // It is necessary to preserve the existing ordering of layers, when there are
+ // no explicit dependencies (because this existing ordering has correct
+ // z-index/layout ordering). To preserve this ordering, we process Nodes in
+ // the same order that they were added to the list.
+ GraphNode* fromNode = noIncomingEdgeNodeList.takeFirst();
+
// Add it to the final list.
sortedList.append(fromNode);
Modified: trunk/Source/WebKit/chromium/tests/CCLayerSorterTest.cpp (104001 => 104002)
--- trunk/Source/WebKit/chromium/tests/CCLayerSorterTest.cpp 2012-01-04 03:34:18 UTC (rev 104001)
+++ trunk/Source/WebKit/chromium/tests/CCLayerSorterTest.cpp 2012-01-04 03:36:46 UTC (rev 104002)
@@ -26,6 +26,8 @@
#include "cc/CCLayerSorter.h"
+#include "cc/CCLayerImpl.h"
+#include "cc/CCSingleThreadProxy.h"
#include <gtest/gtest.h>
using namespace WebCore;
@@ -156,4 +158,76 @@
EXPECT_EQ(CCLayerSorter::calculateZDiff(layerC, layerC, threshold), 0.0);
}
+TEST(CCLayerSorterTest, verifyExistingOrderingPreservedWhenNoZDiff)
+{
+ DebugScopedSetImplThread thisScopeIsOnImplThread;
+
+ // If there is no reason to re-sort the layers (i.e. no 3d z difference), then the
+ // existing ordering provided on input should be retained. This test covers the fix in
+ // https://bugs.webkit.org/show_bug.cgi?id=75046. Before this fix, ordering was
+ // accidentally reversed, causing bugs in z-index ordering on websites when
+ // preserves3D triggered the CCLayerSorter.
+
+ // Input list of layers: [1, 2, 3, 4, 5].
+ // Expected output: [3, 4, 1, 2, 5].
+ // - 1, 2, and 5 do not have a 3d z difference, and therefore their relative ordering should be retained.
+ // - 3 and 4 do not have a 3d z difference, and therefore their relative ordering should be retained.
+ // - 3 and 4 should be re-sorted so they are in front of 1, 2, and 5.
+
+ RefPtr<CCLayerImpl> layer1 = CCLayerImpl::create(1);
+ RefPtr<CCLayerImpl> layer2 = CCLayerImpl::create(2);
+ RefPtr<CCLayerImpl> layer3 = CCLayerImpl::create(3);
+ RefPtr<CCLayerImpl> layer4 = CCLayerImpl::create(4);
+ RefPtr<CCLayerImpl> layer5 = CCLayerImpl::create(5);
+
+ TransformationMatrix BehindMatrix;
+ BehindMatrix.translate3d(0, 0, 2);
+ TransformationMatrix FrontMatrix;
+ FrontMatrix.translate3d(0, 0, 1);
+
+ layer1->setBounds(IntSize(10, 10));
+ layer1->setDrawTransform(BehindMatrix);
+ layer1->setDrawsContent(true);
+
+ layer2->setBounds(IntSize(20, 20));
+ layer2->setDrawTransform(BehindMatrix);
+ layer2->setDrawsContent(true);
+
+ layer3->setBounds(IntSize(30, 30));
+ layer3->setDrawTransform(FrontMatrix);
+ layer3->setDrawsContent(true);
+
+ layer4->setBounds(IntSize(40, 40));
+ layer4->setDrawTransform(FrontMatrix);
+ layer4->setDrawsContent(true);
+
+ layer5->setBounds(IntSize(50, 50));
+ layer5->setDrawTransform(BehindMatrix);
+ layer5->setDrawsContent(true);
+
+ Vector<RefPtr<CCLayerImpl> > layerList;
+ layerList.append(layer1);
+ layerList.append(layer2);
+ layerList.append(layer3);
+ layerList.append(layer4);
+ layerList.append(layer5);
+
+ ASSERT_EQ(static_cast<size_t>(5), layerList.size());
+ EXPECT_EQ(1, layerList[0]->id());
+ EXPECT_EQ(2, layerList[1]->id());
+ EXPECT_EQ(3, layerList[2]->id());
+ EXPECT_EQ(4, layerList[3]->id());
+ EXPECT_EQ(5, layerList[4]->id());
+
+ CCLayerSorter layerSorter;
+ layerSorter.sort(layerList.begin(), layerList.end());
+
+ ASSERT_EQ(static_cast<size_t>(5), layerList.size());
+ EXPECT_EQ(3, layerList[0]->id());
+ EXPECT_EQ(4, layerList[1]->id());
+ EXPECT_EQ(1, layerList[2]->id());
+ EXPECT_EQ(2, layerList[3]->id());
+ EXPECT_EQ(5, layerList[4]->id());
+}
+
} // namespace
