[chromium] updateRect is incorrect when contentBounds != bounds
authorshawnsingh@chromium.org <shawnsingh@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jan 2012 02:45:03 +0000 (02:45 +0000)
committershawnsingh@chromium.org <shawnsingh@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jan 2012 02:45:03 +0000 (02:45 +0000)
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: https://svn.webkit.org/repository/webkit/trunk@105680 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/chromium/LayerChromium.h
Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp

index 525b816f3b100ffede3695c1e151e6f7d1f8351c..568caeafad7d9eba9b044d590d956de87d0feb9b 100644 (file)
@@ -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.
index 04dd1e662af7400e99c26faeb5d4e833d82b2fee..c5f88f1326e8d227a5117007ad0ee2c493f62285 100644 (file)
@@ -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;
index 92d102e322a218355e15681e5005ad32a2e2b6e5..30acf723d66e33b7bb0dc94045cbb0a348a8bc0f 100644 (file)
@@ -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)
index ac58f990eced326644989fab21e0c1c13b65b47f..46cf3fa76fc58c0dd3898472ff23518e2b348864 100644 (file)
@@ -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
index 0dac3aaf8c67bfdaa10fe1a1b6bdd2d775942b24..409d92719506b95ffb873a160beea3a89426f54d 100644 (file)
@@ -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