[chromium] updateRect is incorrect when contentBounds != bounds

https://bugs.webkit.org/show_bug.cgi?id=72919

Reviewed by James Robinson.

Source/WebCore:

Unit test added to TiledLayerChromiumTest.cpp

The m_updateRect member in LayerChromium types is used to track
what was painted for that layer. For tiled layers (especially
image layers), the updateRect was being given with respect to the
size of the content, rather than the size of the layer. This patch
adds a conversion so that updateRect is always with respect to the
layer size, so that damage tracking will work correctly in those
cases.

* platform/graphics/chromium/LayerChromium.h:
* platform/graphics/chromium/TiledLayerChromium.cpp:
(WebCore::TiledLayerChromium::updateCompositorResources):

Source/WebKit/chromium:

* tests/TiledLayerChromiumTest.cpp:
(WTF::FakeTiledLayerWithScaledBounds::FakeTiledLayerWithScaledBounds):
(WTF::FakeTiledLayerWithScaledBounds::setContentBounds):
(WTF::FakeTiledLayerWithScaledBounds::contentBounds):
(WTF::FakeTiledLayerWithScaledBounds::updateRect):
(WTF::TEST):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 525b816..568caea 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2012-01-23  Shawn Singh  <shawnsingh@chromium.org>
+
+        [chromium] updateRect is incorrect when contentBounds != bounds
+        https://bugs.webkit.org/show_bug.cgi?id=72919
+
+        Reviewed by James Robinson.
+
+        Unit test added to TiledLayerChromiumTest.cpp
+
+        The m_updateRect member in LayerChromium types is used to track
+        what was painted for that layer. For tiled layers (especially
+        image layers), the updateRect was being given with respect to the
+        size of the content, rather than the size of the layer. This patch
+        adds a conversion so that updateRect is always with respect to the
+        layer size, so that damage tracking will work correctly in those
+        cases.
+
+        * platform/graphics/chromium/LayerChromium.h:
+        * platform/graphics/chromium/TiledLayerChromium.cpp:
+        (WebCore::TiledLayerChromium::updateCompositorResources):
+
 2012-01-23  Konrad Piascik  <kpiascik@rim.com>
 
         Web Inspector: Make "Copy as HTML" use the same copy functions as other copy methods.

diff --git a/Source/WebCore/platform/graphics/chromium/LayerChromium.h b/Source/WebCore/platform/graphics/chromium/LayerChromium.h
index 04dd1e6..c5f88f1 100644 (file)
--- a/Source/WebCore/platform/graphics/chromium/LayerChromium.h
+++ b/Source/WebCore/platform/graphics/chromium/LayerChromium.h
@@ -214,6 +214,7 @@ protected:
     // 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.
+    // Note this rect is in layer space (not content space).
     FloatRect m_updateRect;
 
     RefPtr<LayerChromium> m_maskLayer;

diff --git a/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp b/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp
index 92d102e..30acf72 100644 (file)
--- a/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp
+++ b/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp
@@ -240,7 +240,11 @@ void TiledLayerChromium::updateCompositorResources(GraphicsContext3D*, CCTexture
         }
     }
 
+    // The updateRect should be in layer space. So we have to convert the paintRect from content space to layer space.
     m_updateRect = FloatRect(m_paintRect);
+    float widthScale = bounds().width() / static_cast<float>(contentBounds().width());
+    float heightScale = bounds().height() / static_cast<float>(contentBounds().height());
+    m_updateRect.scale(widthScale, heightScale);
 }
 
 void TiledLayerChromium::setTilingOption(TilingOption tilingOption)

diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
index ac58f99..46cf3fa 100644 (file)
--- a/Source/WebKit/chromium/ChangeLog
+++ b/Source/WebKit/chromium/ChangeLog
@@ -1,3 +1,17 @@
+2012-01-23  Shawn Singh  <shawnsingh@chromium.org>
+
+        [chromium] updateRect is incorrect when contentBounds != bounds
+        https://bugs.webkit.org/show_bug.cgi?id=72919
+
+        Reviewed by James Robinson.
+
+        * tests/TiledLayerChromiumTest.cpp:
+        (WTF::FakeTiledLayerWithScaledBounds::FakeTiledLayerWithScaledBounds):
+        (WTF::FakeTiledLayerWithScaledBounds::setContentBounds):
+        (WTF::FakeTiledLayerWithScaledBounds::contentBounds):
+        (WTF::FakeTiledLayerWithScaledBounds::updateRect):
+        (WTF::TEST):
+
 2012-01-23  Takashi Toyoshima  <toyoshim@chromium.org>
 
         [Chromium][WebSocket] Remove binary communication using WebData in WebKit API

diff --git a/Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp b/Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp
index 0dac3aa..409d927 100644 (file)
--- a/Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp
+++ b/Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp
@@ -26,6 +26,7 @@
 
 #include "TiledLayerChromium.h"
 
+#include "CCLayerTreeTestCommon.h"
 #include "LayerTextureUpdater.h"
 #include "TextureManager.h"
 #include "cc/CCSingleThreadProxy.h" // For DebugScopedSetImplThread
@@ -143,6 +144,22 @@ private:
     TextureManager* m_textureManager;
 };
 
+class FakeTiledLayerWithScaledBounds : public FakeTiledLayerChromium {
+public:
+    explicit FakeTiledLayerWithScaledBounds(TextureManager* textureManager)
+        : FakeTiledLayerChromium(textureManager)
+    {
+    }
+
+    void setContentBounds(const IntSize& contentBounds) { m_forcedContentBounds = contentBounds; }
+    virtual IntSize contentBounds() const { return m_forcedContentBounds; }
+
+    FloatRect updateRect() { return m_updateRect; }
+
+protected:
+    IntSize m_forcedContentBounds;
+};
+
 void FakeLayerTextureUpdater::setRectToInvalidate(const IntRect& rect, FakeTiledLayerChromium* layer)
 {
     m_rectToInvalidate = rect;
@@ -339,4 +356,43 @@ TEST(TiledLayerChromiumTest, invalidateFromPrepare)
     EXPECT_EQ(1, layer->fakeLayerTextureUpdater()->prepareCount());
 }
 
+TEST(TiledLayerChromiumTest, verifyUpdateRectWhenContentBoundsAreScaled)
+{
+    // The updateRect (that indicates what was actually painted) should be in
+    // layer space, not the content space.
+
+    OwnPtr<TextureManager> textureManager = TextureManager::create(4*1024*1024, 2*1024*1024, 1024);
+    RefPtr<FakeTiledLayerWithScaledBounds> layer = adoptRef(new FakeTiledLayerWithScaledBounds(textureManager.get()));
+
+    FakeTextureAllocator textureAllocator;
+    CCTextureUpdater updater(&textureAllocator);
+
+    IntRect layerBounds(0, 0, 300, 200);
+    IntRect contentBounds(0, 0, 200, 250);
+
+    layer->setBounds(layerBounds.size());
+    layer->setContentBounds(contentBounds.size());
+    layer->setVisibleLayerRect(contentBounds);
+
+    // On first update, the updateRect includes all tiles, even beyond the boundaries of the layer.
+    // However, it should still be in layer space, not content space.
+    layer->invalidateRect(contentBounds);
+    layer->prepareToUpdate(contentBounds);
+    layer->updateCompositorResources(0, updater);
+    EXPECT_FLOAT_RECT_EQ(FloatRect(0, 0, 300, 300 * 0.8), layer->updateRect());
+
+    // After the tiles are updated once, another invalidate only needs to update the bounds of the layer.
+    layer->invalidateRect(contentBounds);
+    layer->prepareToUpdate(contentBounds);
+    layer->updateCompositorResources(0, updater);
+    EXPECT_FLOAT_RECT_EQ(FloatRect(layerBounds), layer->updateRect());
+
+    // Partial re-paint should also be represented by the updateRect in layer space, not content space.
+    IntRect partialDamage(30, 100, 10, 10);
+    layer->invalidateRect(partialDamage);
+    layer->prepareToUpdate(contentBounds);
+    layer->updateCompositorResources(0, updater);
+    EXPECT_FLOAT_RECT_EQ(FloatRect(45, 80, 15, 8), layer->updateRect());
+}
+
 } // namespace