[webkit-changes] [88496] trunk

commit-queue Thu, 09 Jun 2011 15:54:27 -0700

Title: [88496] trunk

Revision: 88496
Author: [email protected]
Date: 2011-06-09 15:53:53 -0700 (Thu, 09 Jun 2011)

Log Message

2011-06-09  James Robinson  <[email protected]>

        Reviewed by Kenneth Russell.

[chromium] Scissor rect not set for clipping layers set offscreen
https://bugs.webkit.org/show_bug.cgi?id=62339

Tests that a layer that should clip its children actually does clip even when scrolled offscreen.

* platform/chromium/compositing/scissor-out-of-viewport-expected.png: Added.
* platform/chromium/compositing/scissor-out-of-viewport-expected.txt: Added.
* platform/chromium/compositing/scissor-out-of-viewport.html: Added.
2011-06-09 James Robinson <[email protected]>

Reviewed by Kenneth Russell.

[chromium] Scissor rect not set for clipping layers set offscreen
https://bugs.webkit.org/show_bug.cgi?id=62339

We set a scissorRect on each layer, but only layers with masksToBounds and their descendants should actually set
a scissor. Layers that didn't need to scissor had empty scissorRects. Unfortunately layers with masksToBounds
and their descendants that are scrolled offscreen also end up with an empty clipped scissor rect.

This patch sets an explicit bit on each layer that should scissor and then checks that bit instead of checking
for an empty scissor rect at draw time. RenderSurfaceChromiums have different requirements for
setScissorToRect, so the old behavior is still available with a flag. This can probably be cleaned up more.

Test: platform/chromium/compositing/scissor-out-of-viewport.html

* platform/graphics/chromium/LayerRendererChromium.cpp:
(WebCore::LayerRendererChromium::updatePropertiesAndRenderSurfaces):
(WebCore::LayerRendererChromium::drawLayer):
(WebCore::LayerRendererChromium::setScissorToRect):
* platform/graphics/chromium/LayerRendererChromium.h:
* platform/graphics/chromium/RenderSurfaceChromium.cpp:
(WebCore::RenderSurfaceChromium::draw):
* platform/graphics/chromium/cc/CCLayerImpl.cpp:
(WebCore::CCLayerImpl::CCLayerImpl):
* platform/graphics/chromium/cc/CCLayerImpl.h:
(WebCore::CCLayerImpl::setUsesLayerScissor):
(WebCore::CCLayerImpl::usesLayerScissor):

Modified Paths

trunk/LayoutTests/ChangeLog
trunk/Source/WebCore/ChangeLog
trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
trunk/Source/WebCore/platform/graphics/chromium/RenderSurfaceChromium.cpp
trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp
trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h

Added Paths

trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport-expected.png
trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport-expected.txt
trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport.html

Diff

Modified: trunk/LayoutTests/ChangeLog (88495 => 88496)


--- trunk/LayoutTests/ChangeLog	2011-06-09 22:53:32 UTC (rev 88495)
+++ trunk/LayoutTests/ChangeLog	2011-06-09 22:53:53 UTC (rev 88496)
@@ -1,3 +1,16 @@
+2011-06-09  James Robinson  <[email protected]>
+
+        Reviewed by Kenneth Russell.
+
+        [chromium] Scissor rect not set for clipping layers set offscreen
+        https://bugs.webkit.org/show_bug.cgi?id=62339
+
+        Tests that a layer that should clip its children actually does clip even when scrolled offscreen.
+
+        * platform/chromium/compositing/scissor-out-of-viewport-expected.png: Added.
+        * platform/chromium/compositing/scissor-out-of-viewport-expected.txt: Added.
+        * platform/chromium/compositing/scissor-out-of-viewport.html: Added.
+
 2011-06-09  Sheriff Bot  <[email protected]>
 
         Unreviewed, rolling out r88468.

Added: trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport-expected.png (0 => 88496)


--- trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport-expected.png	                        (rev 0)
+++ trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport-expected.png	2011-06-09 22:53:53 UTC (rev 88496)
@@ -0,0 +1,6 @@
+\x89PNG
+
+
+IHDR X')tEXtchecksum853de00567d121bea0b7bece66a5d61c`7\xFF\xFB
+\xAAIDATx\x9C\xED\xD6\xC1	 \xC00u\xFF\x9D\xCF%
+\x82$\xF4\xD9=3\x80\xCEy\xF0\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X1\x833X\xB1d\xAD4\xD1ӅIEND\xAEB`\x82
\ No newline at end of file

Added: trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport-expected.txt (0 => 88496)


--- trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport-expected.txt	2011-06-09 22:53:53 UTC (rev 88496)
@@ -0,0 +1,2 @@
+
+

Added: trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport.html (0 => 88496)


--- trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport.html	                        (rev 0)
+++ trunk/LayoutTests/platform/chromium/compositing/scissor-out-of-viewport.html	2011-06-09 22:53:53 UTC (rev 88496)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<!-- This test has a 16px tall div with overflow:hidden set and a child image.  When the test loads the div is scrolled out of view.
+     The test is a pass if the image does not appear. -->
+<body style="overflow:hidden">
+<canvas style="position: absolute;"></canvas>
+<script>
+  document.querySelector("canvas").getContext("2d");
+</script>
+<div style="top:0px; height: 16px; overflow: hidden; position: relative;">
+  <img style="position: absolute; left: -16px;" id="i"></img>
+</div>
+<br>
+<div style="height:2000px"></div>
+<script>
+var can = document.createElement("canvas");
+can.width = can.height = 500;
+var ctx = can.getContext("2d");
+ctx.fillStyle = "red";
+ctx.fillRect(0, 0, 500, 500);
+document.getElementById("i").src = ""
+document.body.scrollTop = 50;
+if (window.layoutTestController)
+    layoutTestController.dumpAsText(true);
+</script>

Modified: trunk/Source/WebCore/ChangeLog (88495 => 88496)


--- trunk/Source/WebCore/ChangeLog	2011-06-09 22:53:32 UTC (rev 88495)
+++ trunk/Source/WebCore/ChangeLog	2011-06-09 22:53:53 UTC (rev 88496)
@@ -1,3 +1,33 @@
+2011-06-09  James Robinson  <[email protected]>
+
+        Reviewed by Kenneth Russell.
+
+        [chromium] Scissor rect not set for clipping layers set offscreen
+        https://bugs.webkit.org/show_bug.cgi?id=62339
+
+        We set a scissorRect on each layer, but only layers with masksToBounds and their descendants should actually set
+        a scissor.  Layers that didn't need to scissor had empty scissorRects.  Unfortunately layers with masksToBounds
+        and their descendants that are scrolled offscreen also end up with an empty clipped scissor rect.
+
+        This patch sets an explicit bit on each layer that should scissor and then checks that bit instead of checking
+        for an empty scissor rect at draw time.  RenderSurfaceChromiums have different requirements for
+        setScissorToRect, so the old behavior is still available with a flag.  This can probably be cleaned up more.
+
+        Test: platform/chromium/compositing/scissor-out-of-viewport.html
+
+        * platform/graphics/chromium/LayerRendererChromium.cpp:
+        (WebCore::LayerRendererChromium::updatePropertiesAndRenderSurfaces):
+        (WebCore::LayerRendererChromium::drawLayer):
+        (WebCore::LayerRendererChromium::setScissorToRect):
+        * platform/graphics/chromium/LayerRendererChromium.h:
+        * platform/graphics/chromium/RenderSurfaceChromium.cpp:
+        (WebCore::RenderSurfaceChromium::draw):
+        * platform/graphics/chromium/cc/CCLayerImpl.cpp:
+        (WebCore::CCLayerImpl::CCLayerImpl):
+        * platform/graphics/chromium/cc/CCLayerImpl.h:
+        (WebCore::CCLayerImpl::setUsesLayerScissor):
+        (WebCore::CCLayerImpl::usesLayerScissor):
+
 2011-06-09  Sheriff Bot  <[email protected]>
 
         Unreviewed, rolling out r88468.

Modified: trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp (88495 => 88496)


--- trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp	2011-06-09 22:53:32 UTC (rev 88495)
+++ trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp	2011-06-09 22:53:53 UTC (rev 88496)
@@ -658,6 +658,9 @@
     FloatRect layerRect(-0.5 * layer->bounds().width(), -0.5 * layer->bounds().height(), layer->bounds().width(), layer->bounds().height());
     IntRect transformedLayerRect;
 
+    // FIXME: This seems like the wrong place to set this
+    layer->setUsesLayerScissor(false);
+
     // The layer and its descendants render on a new RenderSurface if any of
     // these conditions hold:
     // 1. The layer clips its descendants and its transform is not a simple translation.
@@ -694,7 +697,6 @@
         TransformationMatrix layerOriginTransform = combinedTransform;
         layerOriginTransform.translate3d(-0.5 * bounds.width(), -0.5 * bounds.height(), 0);
         renderSurface->m_originTransform = layerOriginTransform;
-        layer->setScissorRect(IntRect());
 
         // The render surface scissor rect is the scissor rect that needs to
         // be applied before drawing the render surface onto its containing
@@ -726,6 +728,8 @@
 
             // Layers inherit the scissor rect from their parent.
             layer->setScissorRect(layer->parent()->scissorRect());
+            if (layer->parent()->usesLayerScissor())
+                layer->setUsesLayerScissor(true);
 
             layer->setTargetRenderSurface(layer->parent()->targetRenderSurface());
         }
@@ -738,6 +742,7 @@
             if (!layer->scissorRect().isEmpty())
                 scissor.intersect(layer->scissorRect());
             layer->setScissorRect(scissor);
+            layer->setUsesLayerScissor(true);
         }
     }
 
@@ -973,12 +978,11 @@
         return;
     }
 
-    setScissorToRect(layer->scissorRect());
-
+    if (layer->usesLayerScissor())
+        setScissorToRect(layer->scissorRect());
+    else
+        GLC(m_context.get(), m_context->disable(GraphicsContext3D::SCISSOR_TEST));
     IntRect targetSurfaceRect = m_currentRenderSurface ? m_currentRenderSurface->contentRect() : m_defaultRenderSurface->contentRect();
-    IntRect scissorRect = layer->scissorRect();
-    if (!scissorRect.isEmpty())
-        targetSurfaceRect.intersect(scissorRect);
 
     // Check if the layer falls within the visible bounds of the page.
     IntRect layerRect = layer->getDrawRect();
@@ -1014,11 +1018,11 @@
 
 // Sets the scissor region to the given rectangle. The coordinate system for the
 // scissorRect has its origin at the top left corner of the current visible rect.
-void LayerRendererChromium::setScissorToRect(const IntRect& requestedScissor)
+void LayerRendererChromium::setScissorToRect(const IntRect& scissorRect)
 {
     IntRect contentRect = (m_currentRenderSurface ? m_currentRenderSurface->m_contentRect : m_defaultRenderSurface->m_contentRect);
 
-    const IntRect scissorRect = requestedScissor.isEmpty() ? contentRect : requestedScissor;
+    GLC(m_context.get(), m_context->enable(GraphicsContext3D::SCISSOR_TEST));
 
     // The scissor coordinates must be supplied in viewport space so we need to offset
     // by the relative position of the top left corner of the current render surface.

Modified: trunk/Source/WebCore/platform/graphics/chromium/RenderSurfaceChromium.cpp (88495 => 88496)


--- trunk/Source/WebCore/platform/graphics/chromium/RenderSurfaceChromium.cpp	2011-06-09 22:53:32 UTC (rev 88495)
+++ trunk/Source/WebCore/platform/graphics/chromium/RenderSurfaceChromium.cpp	2011-06-09 22:53:53 UTC (rev 88496)
@@ -152,8 +152,12 @@
     if (!m_maskLayer && m_owningLayer->replicaLayer())
         replicaMaskLayer = m_owningLayer->replicaLayer()->maskLayer();
 
-    layerRenderer()->setScissorToRect(m_scissorRect);
+    if (m_owningLayer->parent() && m_owningLayer->parent()->usesLayerScissor())
+        layerRenderer()->setScissorToRect(m_scissorRect);
+    else
+        GLC(layerRenderer()->context(), layerRenderer()->context()->disable(GraphicsContext3D::SCISSOR_TEST));
 
+
     // Reflection draws before the layer.
     if (m_owningLayer->replicaLayer()) 
         drawSurface(replicaMaskLayer, m_replicaDrawTransform);

Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp (88495 => 88496)


--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp	2011-06-09 22:53:32 UTC (rev 88495)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp	2011-06-09 22:53:53 UTC (rev 88496)
@@ -70,6 +70,7 @@
     , m_masksToBounds(false)
     , m_opacity(1.0)
     , m_preserves3D(false)
+    , m_usesLayerScissor(false)
     , m_targetRenderSurface(0)
     , m_drawDepth(0)
     , m_drawOpacity(0)

Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h (88495 => 88496)


--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h	2011-06-09 22:53:32 UTC (rev 88495)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h	2011-06-09 22:53:53 UTC (rev 88496)
@@ -100,6 +100,9 @@
     void setPreserves3D(bool preserves3D) { m_preserves3D = preserves3D; }
     bool preserves3D() const { return m_preserves3D; }
 
+    void setUsesLayerScissor(bool usesLayerScissor) { m_usesLayerScissor = usesLayerScissor; }
+    bool usesLayerScissor() const { return m_usesLayerScissor; }
+
     void setSublayerTransform(const TransformationMatrix& sublayerTransform) { m_sublayerTransform = sublayerTransform; }
     const TransformationMatrix& sublayerTransform() const { return m_sublayerTransform; }
 
@@ -183,6 +186,7 @@
     bool m_preserves3D;
     TransformationMatrix m_sublayerTransform;
     TransformationMatrix m_transform;
+    bool m_usesLayerScissor;
 
     // Properties owned exclusively by this CCLayerImpl.
     // Debugging.

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes