[chromium] Replace destRect with destOffset in texture upload
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Aug 2012 23:43:29 +0000 (23:43 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Aug 2012 23:43:29 +0000 (23:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=94154

Patch by Alexandre Elias <aelias@google.com> on 2012-08-16
Reviewed by James Robinson.

Previously, texture upload code implicitly assumed that sourceRect and
destRect have the same size. The behavior is undefined if they are
different, since they are used interchangeably and there's no support
for scaling from one rect to the other. This patch enforces that
constraint at the interface level by replacing all instances of
"IntRect destRect" by "IntSize destOffset".

Source/WebCore:

No new tests (no-op refactoring).

* platform/graphics/chromium/BitmapCanvasLayerTextureUpdater.cpp:
(WebCore::BitmapCanvasLayerTextureUpdater::Texture::updateRect):
(WebCore::BitmapCanvasLayerTextureUpdater::updateTextureRect):
* platform/graphics/chromium/BitmapCanvasLayerTextureUpdater.h:
(Texture):
(BitmapCanvasLayerTextureUpdater):
* platform/graphics/chromium/BitmapSkPictureCanvasLayerTextureUpdater.cpp:
(WebCore::BitmapSkPictureCanvasLayerTextureUpdater::Texture::updateRect):
* platform/graphics/chromium/BitmapSkPictureCanvasLayerTextureUpdater.h:
(Texture):
* platform/graphics/chromium/FrameBufferSkPictureCanvasLayerTextureUpdater.cpp:
(WebCore::FrameBufferSkPictureCanvasLayerTextureUpdater::Texture::updateRect):
(WebCore::FrameBufferSkPictureCanvasLayerTextureUpdater::updateTextureRect):
* platform/graphics/chromium/FrameBufferSkPictureCanvasLayerTextureUpdater.h:
(Texture):
(FrameBufferSkPictureCanvasLayerTextureUpdater):
* platform/graphics/chromium/ImageLayerChromium.cpp:
(WebCore::ImageLayerTextureUpdater::updateTextureRect):
* platform/graphics/chromium/LayerRendererChromium.cpp:
* platform/graphics/chromium/LayerTextureSubImage.cpp:
(WebCore::LayerTextureSubImage::upload):
(WebCore::LayerTextureSubImage::uploadWithTexSubImage):
(WebCore::LayerTextureSubImage::uploadWithMapTexSubImage):
* platform/graphics/chromium/LayerTextureSubImage.h:
(LayerTextureSubImage):
* platform/graphics/chromium/LayerTextureUpdater.h:
(Texture):
* platform/graphics/chromium/ScrollbarLayerChromium.cpp:
(WebCore::ScrollbarLayerChromium::updatePart):
* platform/graphics/chromium/TextureUploader.h:
(Parameters):
* platform/graphics/chromium/ThrottledTextureUploader.cpp:
(WebCore::ThrottledTextureUploader::uploadTexture):
* platform/graphics/chromium/TiledLayerChromium.cpp:
(WebCore::TiledLayerChromium::updateTileTextures):
* platform/graphics/chromium/cc/CCHeadsUpDisplayLayerImpl.cpp:
(WebCore::CCHeadsUpDisplayLayerImpl::willDraw):
* platform/graphics/chromium/cc/CCPrioritizedTexture.cpp:
(WebCore::CCPrioritizedTexture::upload):
* platform/graphics/chromium/cc/CCPrioritizedTexture.h:
(CCPrioritizedTexture):
* platform/graphics/chromium/cc/CCResourceProvider.cpp:
(WebCore::CCResourceProvider::upload):
* platform/graphics/chromium/cc/CCResourceProvider.h:
(CCResourceProvider):
* platform/graphics/chromium/cc/CCVideoLayerImpl.cpp:
(WebCore::CCVideoLayerImpl::copyPlaneData):

Source/WebKit/chromium:

* tests/CCResourceProviderTest.cpp:
(WebKit::TEST_F):
* tests/CCTextureUpdateControllerTest.cpp:
* tests/CCTiledLayerTestCommon.cpp:
(WebKitTests::FakeLayerTextureUpdater::Texture::updateRect):
* tests/CCTiledLayerTestCommon.h:
(Texture):
(WebKitTests::FakeTextureUploader::uploadTexture):

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

28 files changed:
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/chromium/BitmapCanvasLayerTextureUpdater.cpp
Source/WebCore/platform/graphics/chromium/BitmapCanvasLayerTextureUpdater.h
Source/WebCore/platform/graphics/chromium/BitmapSkPictureCanvasLayerTextureUpdater.cpp
Source/WebCore/platform/graphics/chromium/BitmapSkPictureCanvasLayerTextureUpdater.h
Source/WebCore/platform/graphics/chromium/FrameBufferSkPictureCanvasLayerTextureUpdater.cpp
Source/WebCore/platform/graphics/chromium/FrameBufferSkPictureCanvasLayerTextureUpdater.h
Source/WebCore/platform/graphics/chromium/ImageLayerChromium.cpp
Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
Source/WebCore/platform/graphics/chromium/LayerTextureSubImage.cpp
Source/WebCore/platform/graphics/chromium/LayerTextureSubImage.h
Source/WebCore/platform/graphics/chromium/LayerTextureUpdater.h
Source/WebCore/platform/graphics/chromium/ScrollbarLayerChromium.cpp
Source/WebCore/platform/graphics/chromium/TextureUploader.h
Source/WebCore/platform/graphics/chromium/ThrottledTextureUploader.cpp
Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp
Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplayLayerImpl.cpp
Source/WebCore/platform/graphics/chromium/cc/CCPrioritizedTexture.cpp
Source/WebCore/platform/graphics/chromium/cc/CCPrioritizedTexture.h
Source/WebCore/platform/graphics/chromium/cc/CCResourceProvider.cpp
Source/WebCore/platform/graphics/chromium/cc/CCResourceProvider.h
Source/WebCore/platform/graphics/chromium/cc/CCVideoLayerImpl.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp
Source/WebKit/chromium/tests/CCResourceProviderTest.cpp
Source/WebKit/chromium/tests/CCTextureUpdateControllerTest.cpp
Source/WebKit/chromium/tests/CCTiledLayerTestCommon.cpp
Source/WebKit/chromium/tests/CCTiledLayerTestCommon.h

index 7a42623..f7cf00e 100644 (file)
@@ -1,3 +1,67 @@
+2012-08-16  Alexandre Elias  <aelias@google.com>
+
+        [chromium] Replace destRect with destOffset in texture upload
+        https://bugs.webkit.org/show_bug.cgi?id=94154
+
+        Reviewed by James Robinson.
+
+        Previously, texture upload code implicitly assumed that sourceRect and
+        destRect have the same size. The behavior is undefined if they are
+        different, since they are used interchangeably and there's no support
+        for scaling from one rect to the other. This patch enforces that
+        constraint at the interface level by replacing all instances of
+        "IntRect destRect" by "IntSize destOffset".
+
+        No new tests (no-op refactoring).
+
+        * platform/graphics/chromium/BitmapCanvasLayerTextureUpdater.cpp:
+        (WebCore::BitmapCanvasLayerTextureUpdater::Texture::updateRect):
+        (WebCore::BitmapCanvasLayerTextureUpdater::updateTextureRect):
+        * platform/graphics/chromium/BitmapCanvasLayerTextureUpdater.h:
+        (Texture):
+        (BitmapCanvasLayerTextureUpdater):
+        * platform/graphics/chromium/BitmapSkPictureCanvasLayerTextureUpdater.cpp:
+        (WebCore::BitmapSkPictureCanvasLayerTextureUpdater::Texture::updateRect):
+        * platform/graphics/chromium/BitmapSkPictureCanvasLayerTextureUpdater.h:
+        (Texture):
+        * platform/graphics/chromium/FrameBufferSkPictureCanvasLayerTextureUpdater.cpp:
+        (WebCore::FrameBufferSkPictureCanvasLayerTextureUpdater::Texture::updateRect):
+        (WebCore::FrameBufferSkPictureCanvasLayerTextureUpdater::updateTextureRect):
+        * platform/graphics/chromium/FrameBufferSkPictureCanvasLayerTextureUpdater.h:
+        (Texture):
+        (FrameBufferSkPictureCanvasLayerTextureUpdater):
+        * platform/graphics/chromium/ImageLayerChromium.cpp:
+        (WebCore::ImageLayerTextureUpdater::updateTextureRect):
+        * platform/graphics/chromium/LayerRendererChromium.cpp:
+        * platform/graphics/chromium/LayerTextureSubImage.cpp:
+        (WebCore::LayerTextureSubImage::upload):
+        (WebCore::LayerTextureSubImage::uploadWithTexSubImage):
+        (WebCore::LayerTextureSubImage::uploadWithMapTexSubImage):
+        * platform/graphics/chromium/LayerTextureSubImage.h:
+        (LayerTextureSubImage):
+        * platform/graphics/chromium/LayerTextureUpdater.h:
+        (Texture):
+        * platform/graphics/chromium/ScrollbarLayerChromium.cpp:
+        (WebCore::ScrollbarLayerChromium::updatePart):
+        * platform/graphics/chromium/TextureUploader.h:
+        (Parameters):
+        * platform/graphics/chromium/ThrottledTextureUploader.cpp:
+        (WebCore::ThrottledTextureUploader::uploadTexture):
+        * platform/graphics/chromium/TiledLayerChromium.cpp:
+        (WebCore::TiledLayerChromium::updateTileTextures):
+        * platform/graphics/chromium/cc/CCHeadsUpDisplayLayerImpl.cpp:
+        (WebCore::CCHeadsUpDisplayLayerImpl::willDraw):
+        * platform/graphics/chromium/cc/CCPrioritizedTexture.cpp:
+        (WebCore::CCPrioritizedTexture::upload):
+        * platform/graphics/chromium/cc/CCPrioritizedTexture.h:
+        (CCPrioritizedTexture):
+        * platform/graphics/chromium/cc/CCResourceProvider.cpp:
+        (WebCore::CCResourceProvider::upload):
+        * platform/graphics/chromium/cc/CCResourceProvider.h:
+        (CCResourceProvider):
+        * platform/graphics/chromium/cc/CCVideoLayerImpl.cpp:
+        (WebCore::CCVideoLayerImpl::copyPlaneData):
+
 2012-08-16  James Robinson  <jamesr@chromium.org>
 
         [chromium] Remove unnecessary tree hierarchy inspection APIs from WebLayer
index c92ce06..c1210e5 100644 (file)
@@ -46,9 +46,9 @@ BitmapCanvasLayerTextureUpdater::Texture::~Texture()
 {
 }
 
-void BitmapCanvasLayerTextureUpdater::Texture::updateRect(CCResourceProvider* resourceProvider, const IntRect& sourceRect, const IntRect& destRect)
+void BitmapCanvasLayerTextureUpdater::Texture::updateRect(CCResourceProvider* resourceProvider, const IntRect& sourceRect, const IntSize& destOffset)
 {
-    textureUpdater()->updateTextureRect(resourceProvider, texture(), sourceRect, destRect);
+    textureUpdater()->updateTextureRect(resourceProvider, texture(), sourceRect, destOffset);
 }
 
 PassRefPtr<BitmapCanvasLayerTextureUpdater> BitmapCanvasLayerTextureUpdater::create(PassOwnPtr<LayerPainterChromium> painter)
@@ -88,12 +88,12 @@ void BitmapCanvasLayerTextureUpdater::prepareToUpdate(const IntRect& contentRect
     paintContents(m_canvas.get(), contentRect, contentsWidthScale, contentsHeightScale, resultingOpaqueRect, stats);
 }
 
-void BitmapCanvasLayerTextureUpdater::updateTextureRect(CCResourceProvider* resourceProvider, CCPrioritizedTexture* texture, const IntRect& sourceRect, const IntRect& destRect)
+void BitmapCanvasLayerTextureUpdater::updateTextureRect(CCResourceProvider* resourceProvider, CCPrioritizedTexture* texture, const IntRect& sourceRect, const IntSize& destOffset)
 {
     const SkBitmap& bitmap = m_canvas->getDevice()->accessBitmap(false);
     bitmap.lockPixels();
 
-    texture->upload(resourceProvider, static_cast<const uint8_t*>(bitmap.getPixels()), contentRect(), sourceRect, destRect);
+    texture->upload(resourceProvider, static_cast<const uint8_t*>(bitmap.getPixels()), contentRect(), sourceRect, destOffset);
     bitmap.unlockPixels();
 }
 
index 8450e1f..d54521f 100644 (file)
@@ -47,7 +47,7 @@ public:
         Texture(BitmapCanvasLayerTextureUpdater*, PassOwnPtr<CCPrioritizedTexture>);
         virtual ~Texture();
 
-        virtual void updateRect(CCResourceProvider*, const IntRect& sourceRect, const IntRect& destRect) OVERRIDE;
+        virtual void updateRect(CCResourceProvider*, const IntRect& sourceRect, const IntSize& destOffset) OVERRIDE;
 
     private:
         BitmapCanvasLayerTextureUpdater* textureUpdater() { return m_textureUpdater; }
@@ -61,7 +61,7 @@ public:
     virtual PassOwnPtr<LayerTextureUpdater::Texture> createTexture(CCPrioritizedTextureManager*) OVERRIDE;
     virtual SampledTexelFormat sampledTexelFormat(GC3Denum textureFormat) OVERRIDE;
     virtual void prepareToUpdate(const IntRect& contentRect, const IntSize& tileSize, float contentsWidthScale, float contentsHeightScale, IntRect& resultingOpaqueRect, CCRenderingStats&) OVERRIDE;
-    void updateTextureRect(CCResourceProvider*, CCPrioritizedTexture*, const IntRect& sourceRect, const IntRect& destRect);
+    void updateTextureRect(CCResourceProvider*, CCPrioritizedTexture*, const IntRect& sourceRect, const IntSize& destOffset);
 
     virtual void setOpaque(bool) OVERRIDE;
 
index de01086..55f97dd 100644 (file)
@@ -57,10 +57,10 @@ void BitmapSkPictureCanvasLayerTextureUpdater::Texture::prepareRect(const IntRec
     stats.totalPaintTimeInSeconds += monotonicallyIncreasingTime() - paintBeginTime;
 }
 
-void BitmapSkPictureCanvasLayerTextureUpdater::Texture::updateRect(CCResourceProvider* resourceProvider, const IntRect& sourceRect, const IntRect& destRect)
+void BitmapSkPictureCanvasLayerTextureUpdater::Texture::updateRect(CCResourceProvider* resourceProvider, const IntRect& sourceRect, const IntSize& destOffset)
 {
     m_bitmap.lockPixels();
-    texture()->upload(resourceProvider, static_cast<uint8_t*>(m_bitmap.getPixels()), destRect, destRect, destRect);
+    texture()->upload(resourceProvider, static_cast<uint8_t*>(m_bitmap.getPixels()), sourceRect, sourceRect, destOffset);
     m_bitmap.unlockPixels();
     m_bitmap.reset();
 }
index 70bdd5f..38da53c 100644 (file)
@@ -42,7 +42,7 @@ public:
         Texture(BitmapSkPictureCanvasLayerTextureUpdater*, PassOwnPtr<CCPrioritizedTexture>);
 
         virtual void prepareRect(const IntRect& sourceRect, CCRenderingStats&) OVERRIDE;
-        virtual void updateRect(CCResourceProvider*, const IntRect& sourceRect, const IntRect& destRect) OVERRIDE;
+        virtual void updateRect(CCResourceProvider*, const IntRect& sourceRect, const IntSize& destOffset) OVERRIDE;
 
     private:
         BitmapSkPictureCanvasLayerTextureUpdater* textureUpdater() { return m_textureUpdater; }
index a0e857e..00044c7 100644 (file)
@@ -67,13 +67,13 @@ FrameBufferSkPictureCanvasLayerTextureUpdater::Texture::~Texture()
 {
 }
 
-void FrameBufferSkPictureCanvasLayerTextureUpdater::Texture::updateRect(CCResourceProvider* resourceProvider, const IntRect& sourceRect, const IntRect& destRect)
+void FrameBufferSkPictureCanvasLayerTextureUpdater::Texture::updateRect(CCResourceProvider* resourceProvider, const IntRect& sourceRect, const IntSize& destOffset)
 {
     WebGraphicsContext3D* sharedContext = CCProxy::hasImplThread() ? WebSharedGraphicsContext3D::compositorThreadContext() : WebSharedGraphicsContext3D::mainThreadContext();
     GrContext* sharedGrContext = CCProxy::hasImplThread() ? WebSharedGraphicsContext3D::compositorThreadGrContext() : WebSharedGraphicsContext3D::mainThreadGrContext();
     if (!sharedContext || !sharedGrContext)
         return;
-    textureUpdater()->updateTextureRect(sharedContext, sharedGrContext, resourceProvider, texture(), sourceRect, destRect);
+    textureUpdater()->updateTextureRect(sharedContext, sharedGrContext, resourceProvider, texture(), sourceRect, destOffset);
 }
 
 PassRefPtr<FrameBufferSkPictureCanvasLayerTextureUpdater> FrameBufferSkPictureCanvasLayerTextureUpdater::create(PassOwnPtr<LayerPainterChromium> painter)
@@ -101,7 +101,7 @@ LayerTextureUpdater::SampledTexelFormat FrameBufferSkPictureCanvasLayerTextureUp
     return LayerTextureUpdater::SampledTexelFormatRGBA;
 }
 
-void FrameBufferSkPictureCanvasLayerTextureUpdater::updateTextureRect(WebGraphicsContext3D* context, GrContext* grContext, CCResourceProvider* resourceProvider, CCPrioritizedTexture* texture, const IntRect& sourceRect, const IntRect& destRect)
+void FrameBufferSkPictureCanvasLayerTextureUpdater::updateTextureRect(WebGraphicsContext3D* context, GrContext* grContext, CCResourceProvider* resourceProvider, CCPrioritizedTexture* texture, const IntRect& sourceRect, const IntSize& destOffset)
 {
     // Make sure ganesh uses the correct GL context.
     context->makeContextCurrent();
@@ -116,12 +116,12 @@ void FrameBufferSkPictureCanvasLayerTextureUpdater::updateTextureRect(WebGraphic
     // need to do a y-flip.
     canvas->translate(0.0, texture->size().height());
     canvas->scale(1.0, -1.0);
-    // Only the region corresponding to destRect on the texture must be updated.
-    canvas->clipRect(SkRect::MakeXYWH(destRect.x(), destRect.y(), destRect.width(), destRect.height()));
+    // Clip to the destination on the texture that must be updated.
+    canvas->clipRect(SkRect::MakeXYWH(destOffset.width(), destOffset.height(), sourceRect.width(), sourceRect.height()));
     // Translate the origin of contentRect to that of destRect.
     // Note that destRect is defined relative to sourceRect.
-    canvas->translate(contentRect().x() - sourceRect.x() + destRect.x(),
-                      contentRect().y() - sourceRect.y() + destRect.y());
+    canvas->translate(contentRect().x() - sourceRect.x() + destOffset.width(),
+                      contentRect().y() - sourceRect.y() + destOffset.height());
     drawPicture(canvas.get());
 
     // Flush ganesh context so that all the rendered stuff appears on the texture.
index 64d5eec..05a9dfe 100644 (file)
@@ -50,7 +50,7 @@ public:
         Texture(FrameBufferSkPictureCanvasLayerTextureUpdater*, PassOwnPtr<CCPrioritizedTexture>);
         virtual ~Texture();
 
-        virtual void updateRect(CCResourceProvider*, const IntRect& sourceRect, const IntRect& destRect) OVERRIDE;
+        virtual void updateRect(CCResourceProvider*, const IntRect& sourceRect, const IntSize& destOffset) OVERRIDE;
 
     private:
         FrameBufferSkPictureCanvasLayerTextureUpdater* textureUpdater() { return m_textureUpdater; }
@@ -63,7 +63,7 @@ public:
 
     virtual PassOwnPtr<LayerTextureUpdater::Texture> createTexture(CCPrioritizedTextureManager*) OVERRIDE;
     virtual SampledTexelFormat sampledTexelFormat(GC3Denum textureFormat) OVERRIDE;
-    void updateTextureRect(WebKit::WebGraphicsContext3D*, GrContext*, CCResourceProvider*, CCPrioritizedTexture*, const IntRect& sourceRect, const IntRect& destRect);
+    void updateTextureRect(WebKit::WebGraphicsContext3D*, GrContext*, CCResourceProvider*, CCPrioritizedTexture*, const IntRect& sourceRect, const IntSize& destOffset);
 
 private:
     explicit FrameBufferSkPictureCanvasLayerTextureUpdater(PassOwnPtr<LayerPainterChromium>);
index 5073d1c..54911fb 100644 (file)
@@ -50,9 +50,9 @@ public:
         {
         }
 
-        virtual void updateRect(CCResourceProvider* resourceProvider, const IntRect& sourceRect, const IntRect& destRect) OVERRIDE
+        virtual void updateRect(CCResourceProvider* resourceProvider, const IntRect& sourceRect, const IntSize& destOffset) OVERRIDE
         {
-            textureUpdater()->updateTextureRect(resourceProvider, texture(), sourceRect, destRect);
+            textureUpdater()->updateTextureRect(resourceProvider, texture(), sourceRect, destOffset);
         }
 
     private:
@@ -79,7 +79,7 @@ public:
                 LayerTextureUpdater::SampledTexelFormatRGBA : LayerTextureUpdater::SampledTexelFormatBGRA;
     }
 
-    void updateTextureRect(CCResourceProvider* resourceProvider, CCPrioritizedTexture* texture, const IntRect& sourceRect, const IntRect& destRect)
+    void updateTextureRect(CCResourceProvider* resourceProvider, CCPrioritizedTexture* texture, const IntRect& sourceRect, const IntSize& destOffset)
     {
         // Source rect should never go outside the image pixels, even if this
         // is requested because the texture extends outside the image.
@@ -87,12 +87,10 @@ public:
         IntRect imageRect = IntRect(0, 0, m_bitmap.width(), m_bitmap.height());
         clippedSourceRect.intersect(imageRect);
 
-        IntRect clippedDestRect = destRect;
-        clippedDestRect.move(clippedSourceRect.location() - sourceRect.location());
-        clippedDestRect.setSize(clippedSourceRect.size());
+        IntSize clippedDestOffset = destOffset + IntSize(clippedSourceRect.location() - sourceRect.location());
 
         SkAutoLockPixels lock(m_bitmap);
-        texture->upload(resourceProvider, static_cast<const uint8_t*>(m_bitmap.getPixels()), imageRect, clippedSourceRect, clippedDestRect);
+        texture->upload(resourceProvider, static_cast<const uint8_t*>(m_bitmap.getPixels()), imageRect, clippedSourceRect, clippedDestOffset);
     }
 
     void setBitmap(const SkBitmap& bitmap)
index 217d5a5..085f4bb 100644 (file)
@@ -126,7 +126,7 @@ public:
     virtual bool isBusy() OVERRIDE { return false; }
     virtual void beginUploads() OVERRIDE { }
     virtual void endUploads() OVERRIDE { }
-    virtual void uploadTexture(CCResourceProvider* resourceProvider, Parameters upload) OVERRIDE { upload.texture->updateRect(resourceProvider, upload.sourceRect, upload.destRect); }
+    virtual void uploadTexture(CCResourceProvider* resourceProvider, Parameters upload) OVERRIDE { upload.texture->updateRect(resourceProvider, upload.sourceRect, upload.destOffset); }
 
 protected:
     UnthrottledTextureUploader() { }
index b66d79e..83c7114 100644 (file)
@@ -49,17 +49,17 @@ LayerTextureSubImage::~LayerTextureSubImage()
 }
 
 void LayerTextureSubImage::upload(const uint8_t* image, const IntRect& imageRect,
-                                  const IntRect& sourceRect, const IntRect& destRect,
+                                  const IntRect& sourceRect, const IntSize& destOffset,
                                   GC3Denum format, WebGraphicsContext3D* context)
 {
     if (m_useMapTexSubImage)
-        uploadWithMapTexSubImage(image, imageRect, sourceRect, destRect, format, context);
+        uploadWithMapTexSubImage(image, imageRect, sourceRect, destOffset, format, context);
     else
-        uploadWithTexSubImage(image, imageRect, sourceRect, destRect, format, context);
+        uploadWithTexSubImage(image, imageRect, sourceRect, destOffset, format, context);
 }
 
 void LayerTextureSubImage::uploadWithTexSubImage(const uint8_t* image, const IntRect& imageRect,
-                                                 const IntRect& sourceRect, const IntRect& destRect,
+                                                 const IntRect& sourceRect, const IntSize& destOffset,
                                                  GC3Denum format, WebGraphicsContext3D* context)
 {
     TRACE_EVENT0("cc", "LayerTextureSubImage::uploadWithTexSubImage");
@@ -71,26 +71,26 @@ void LayerTextureSubImage::uploadWithTexSubImage(const uint8_t* image, const Int
     if (imageRect.width() == sourceRect.width() && !offset.x())
         pixelSource = &image[4 * offset.y() * imageRect.width()];
     else {
-        size_t neededSize = 4 * destRect.width() * destRect.height();
+        size_t neededSize = 4 * sourceRect.width() * sourceRect.height();
         if (m_subImageSize < neededSize) {
           m_subImage = adoptArrayPtr(new uint8_t[neededSize]);
           m_subImageSize = neededSize;
         }
         // Strides not equal, so do a row-by-row memcpy from the
         // paint results into a temp buffer for uploading.
-        for (int row = 0; row < destRect.height(); ++row)
-            memcpy(&m_subImage[destRect.width() * 4 * row],
+        for (int row = 0; row < sourceRect.height(); ++row)
+            memcpy(&m_subImage[sourceRect.width() * 4 * row],
                    &image[4 * (offset.x() + (offset.y() + row) * imageRect.width())],
-                   destRect.width() * 4);
+                   sourceRect.width() * 4);
 
         pixelSource = &m_subImage[0];
     }
 
-    GLC(context, context->texSubImage2D(GraphicsContext3D::TEXTURE_2D, 0, destRect.x(), destRect.y(), destRect.width(), destRect.height(), format, GraphicsContext3D::UNSIGNED_BYTE, pixelSource));
+    GLC(context, context->texSubImage2D(GraphicsContext3D::TEXTURE_2D, 0, destOffset.width(), destOffset.height(), sourceRect.width(), sourceRect.height(), format, GraphicsContext3D::UNSIGNED_BYTE, pixelSource));
 }
 
 void LayerTextureSubImage::uploadWithMapTexSubImage(const uint8_t* image, const IntRect& imageRect,
-                                                    const IntRect& sourceRect, const IntRect& destRect,
+                                                    const IntRect& sourceRect, const IntSize& destOffset,
                                                     GC3Denum format, WebGraphicsContext3D* context)
 {
     TRACE_EVENT0("cc", "LayerTextureSubImage::uploadWithMapTexSubImage");
@@ -98,10 +98,10 @@ void LayerTextureSubImage::uploadWithMapTexSubImage(const uint8_t* image, const
     IntPoint offset(sourceRect.x() - imageRect.x(), sourceRect.y() - imageRect.y());
 
     // Upload tile data via a mapped transfer buffer
-    uint8_t* pixelDest = static_cast<uint8_t*>(context->mapTexSubImage2DCHROMIUM(GraphicsContext3D::TEXTURE_2D, 0, destRect.x(), destRect.y(), destRect.width(), destRect.height(), format, GraphicsContext3D::UNSIGNED_BYTE, Extensions3DChromium::WRITE_ONLY));
+    uint8_t* pixelDest = static_cast<uint8_t*>(context->mapTexSubImage2DCHROMIUM(GraphicsContext3D::TEXTURE_2D, 0, destOffset.width(), destOffset.height(), sourceRect.width(), sourceRect.height(), format, GraphicsContext3D::UNSIGNED_BYTE, Extensions3DChromium::WRITE_ONLY));
 
     if (!pixelDest) {
-        uploadWithTexSubImage(image, imageRect, sourceRect, destRect, format, context);
+        uploadWithTexSubImage(image, imageRect, sourceRect, destOffset, format, context);
         return;
     }
 
@@ -113,14 +113,14 @@ void LayerTextureSubImage::uploadWithMapTexSubImage(const uint8_t* image, const
     }
 
     if (imageRect.width() == sourceRect.width() && !offset.x())
-        memcpy(pixelDest, &image[offset.y() * imageRect.width() * componentsPerPixel * bytesPerComponent], imageRect.width() * destRect.height() * componentsPerPixel * bytesPerComponent);
+        memcpy(pixelDest, &image[offset.y() * imageRect.width() * componentsPerPixel * bytesPerComponent], imageRect.width() * sourceRect.height() * componentsPerPixel * bytesPerComponent);
     else {
         // Strides not equal, so do a row-by-row memcpy from the
         // paint results into the pixelDest
-        for (int row = 0; row < destRect.height(); ++row)
-            memcpy(&pixelDest[destRect.width() * row * componentsPerPixel * bytesPerComponent],
+        for (int row = 0; row < sourceRect.height(); ++row)
+            memcpy(&pixelDest[sourceRect.width() * row * componentsPerPixel * bytesPerComponent],
                    &image[4 * (offset.x() + (offset.y() + row) * imageRect.width())],
-                   destRect.width() * componentsPerPixel * bytesPerComponent);
+                   sourceRect.width() * componentsPerPixel * bytesPerComponent);
     }
     GLC(context, context->unmapTexSubImage2DCHROMIUM(pixelDest));
 }
index 357b641..e2a739a 100644 (file)
@@ -46,15 +46,15 @@ public:
     ~LayerTextureSubImage();
 
     void upload(const uint8_t* image, const IntRect& imageRect,
-                const IntRect& sourceRect, const IntRect& destRect,
+                const IntRect& sourceRect, const IntSize& destOffset,
                 GC3Denum format, WebKit::WebGraphicsContext3D*);
 
 private:
     void uploadWithTexSubImage(const uint8_t* image, const IntRect& imageRect,
-                               const IntRect& sourceRect, const IntRect& destRect,
+                               const IntRect& sourceRect, const IntSize& destOffset,
                                GC3Denum format, WebKit::WebGraphicsContext3D*);
     void uploadWithMapTexSubImage(const uint8_t* image, const IntRect& imageRect,
-                                  const IntRect& sourceRect, const IntRect& destRect,
+                                  const IntRect& sourceRect, const IntSize& destOffset,
                                   GC3Denum format, WebKit::WebGraphicsContext3D*);
 
     bool m_useMapTexSubImage;
index e3ac499..cc7536b 100644 (file)
@@ -50,7 +50,7 @@ public:
         CCPrioritizedTexture* texture() { return m_texture.get(); }
         void swapTextureWith(OwnPtr<CCPrioritizedTexture>& texture) { m_texture.swap(texture); }
         virtual void prepareRect(const IntRect& /* sourceRect */, CCRenderingStats&) { }
-        virtual void updateRect(CCResourceProvider*, const IntRect& sourceRect, const IntRect& destRect) = 0;
+        virtual void updateRect(CCResourceProvider*, const IntRect& sourceRect, const IntSize& destOffset) = 0;
     protected:
         explicit Texture(PassOwnPtr<CCPrioritizedTexture> texture) : m_texture(texture) { }
 
index a0fbf5c..b417e33 100644 (file)
@@ -228,8 +228,8 @@ void ScrollbarLayerChromium::updatePart(LayerTextureUpdater* painter, LayerTextu
     painter->prepareToUpdate(rect, rect.size(), 1, 1, paintedOpaqueRect, stats);
     texture->prepareRect(rect, stats);
 
-    IntRect destRect(IntPoint(), rect.size());
-    TextureUploader::Parameters upload = { texture, rect, destRect };
+    IntSize destOffset(0, 0);
+    TextureUploader::Parameters upload = { texture, rect, destOffset };
     queue.appendFullUpload(upload);
 }
 
index 7a51653..0a86161 100644 (file)
@@ -34,7 +34,7 @@ public:
     struct Parameters {
         LayerTextureUpdater::Texture* texture;
         IntRect sourceRect;
-        IntRect destRect;
+        IntSize destOffset;
     };
 
     virtual ~TextureUploader() { }
index 93da189..b82ed2b 100644 (file)
@@ -122,7 +122,7 @@ void ThrottledTextureUploader::endUploads()
 
 void ThrottledTextureUploader::uploadTexture(CCResourceProvider* resourceProvider, Parameters upload)
 {
-    upload.texture->updateRect(resourceProvider, upload.sourceRect, upload.destRect);
+    upload.texture->updateRect(resourceProvider, upload.sourceRect, upload.destOffset);
 }
 
 void ThrottledTextureUploader::processQueries()
index afbe8f2..2ae1720 100644 (file)
@@ -529,10 +529,10 @@ void TiledLayerChromium::updateTileTextures(const IntRect& paintRect, int left,
             const IntPoint anchor = m_tiler->tileRect(tile).location();
 
             // Calculate tile-space rectangle to upload into.
-            IntRect destRect(IntPoint(sourceRect.x() - anchor.x(), sourceRect.y() - anchor.y()), sourceRect.size());
-            if (destRect.x() < 0)
+            IntSize destOffset(sourceRect.x() - anchor.x(), sourceRect.y() - anchor.y());
+            if (destOffset.width() < 0)
                 CRASH();
-            if (destRect.y() < 0)
+            if (destOffset.height() < 0)
                 CRASH();
 
             // Offset from paint rectangle to this tile's dirty rectangle.
@@ -541,12 +541,12 @@ void TiledLayerChromium::updateTileTextures(const IntRect& paintRect, int left,
                 CRASH();
             if (paintOffset.y() < 0)
                 CRASH();
-            if (paintOffset.x() + destRect.width() > paintRect.width())
+            if (paintOffset.x() + sourceRect.width() > paintRect.width())
                 CRASH();
-            if (paintOffset.y() + destRect.height() > paintRect.height())
+            if (paintOffset.y() + sourceRect.height() > paintRect.height())
                 CRASH();
 
-            TextureUploader::Parameters upload = { tile->texture(), sourceRect, destRect };
+            TextureUploader::Parameters upload = { tile->texture(), sourceRect, destOffset };
             if (tile->partialUpdate)
                 queue.appendPartialUpload(upload);
             else
index 97d1ef5..edc5645 100644 (file)
@@ -107,7 +107,7 @@ void CCHeadsUpDisplayLayerImpl::willDraw(CCResourceProvider* resourceProvider)
 
     IntRect layerRect(IntPoint(), bounds());
     ASSERT(bitmap->config() == SkBitmap::kARGB_8888_Config);
-    resourceProvider->upload(m_hudTexture->id(), static_cast<const uint8_t*>(bitmap->getPixels()), layerRect, layerRect, layerRect);
+    resourceProvider->upload(m_hudTexture->id(), static_cast<const uint8_t*>(bitmap->getPixels()), layerRect, layerRect, IntSize());
 }
 
 void CCHeadsUpDisplayLayerImpl::appendQuads(CCQuadSink& quadList, const CCSharedQuadState* sharedQuadState, bool&)
index 60cf1a1..b172276 100644 (file)
@@ -105,13 +105,13 @@ CCResourceProvider::ResourceId CCPrioritizedTexture::resourceId() const
 
 void CCPrioritizedTexture::upload(CCResourceProvider* resourceProvider,
                                   const uint8_t* image, const IntRect& imageRect,
-                                  const IntRect& sourceRect, const IntRect& destRect)
+                                  const IntRect& sourceRect, const IntSize& destOffset)
 {
     ASSERT(m_isAbovePriorityCutoff);
     if (m_isAbovePriorityCutoff)
         acquireBackingTexture(resourceProvider);
     ASSERT(m_backing);
-    resourceProvider->upload(resourceId(), image, imageRect, sourceRect, destRect);
+    resourceProvider->upload(resourceId(), image, imageRect, sourceRect, destOffset);
 }
 
 void CCPrioritizedTexture::link(Backing* backing)
index 764cbec..4d47b63 100644 (file)
@@ -86,7 +86,7 @@ public:
     bool requestLate();
 
     // Uploads pixels into the backing resource. This functions will aquire the backing if needed.
-    void upload(CCResourceProvider*, const uint8_t* image, const IntRect& imageRect, const IntRect& sourceRect, const IntRect& destRect);
+    void upload(CCResourceProvider*, const uint8_t* image, const IntRect& imageRect, const IntRect& sourceRect, const IntSize& destOffset);
 
     CCResourceProvider::ResourceId resourceId() const;
 
index a0bd37c..d21c4b5 100644 (file)
@@ -150,7 +150,7 @@ void CCResourceProvider::deleteOwnedResources(int pool)
         deleteResource(*it);
 }
 
-void CCResourceProvider::upload(ResourceId id, const uint8_t* image, const IntRect& imageRect, const IntRect& sourceRect, const IntRect& destRect)
+void CCResourceProvider::upload(ResourceId id, const uint8_t* image, const IntRect& imageRect, const IntRect& sourceRect, const IntSize& destOffset)
 {
     ASSERT(CCProxy::isImplThread());
     ASSERT(m_texSubImage.get());
@@ -163,7 +163,7 @@ void CCResourceProvider::upload(ResourceId id, const uint8_t* image, const IntRe
     ASSERT(it != m_resources.end() && !it->second.lockedForWrite && !it->second.lockForReadCount && !it->second.external);
 
     context3d->bindTexture(GraphicsContext3D::TEXTURE_2D, it->second.glId);
-    m_texSubImage->upload(image, imageRect, sourceRect, destRect, it->second.format, context3d);
+    m_texSubImage->upload(image, imageRect, sourceRect, destOffset, it->second.format, context3d);
 }
 
 unsigned CCResourceProvider::lockForWrite(ResourceId id)
index add909e..2524121 100644 (file)
@@ -80,7 +80,7 @@ public:
     void deleteOwnedResources(int pool);
 
     // Upload data from image, copying sourceRect (in image) into destRect (in the resource).
-    void upload(ResourceId, const uint8_t* image, const IntRect& imageRect, const IntRect& sourceRect, const IntRect& destRect);
+    void upload(ResourceId, const uint8_t* image, const IntRect& imageRect, const IntRect& sourceRect, const IntSize& destOffset);
 
     // Flush all context operations, kicking uploads and ensuring ordering with
     // respect to other contexts.
index 4208d52..47b3c6f 100644 (file)
@@ -351,7 +351,7 @@ bool CCVideoLayerImpl::copyPlaneData(CCResourceProvider* resourceProvider)
         CCVideoLayerImpl::FramePlane& plane = m_framePlanes[softwarePlaneIndex];
         const uint8_t* softwarePlanePixels = static_cast<const uint8_t*>(m_frame->data(softwarePlaneIndex));
         IntRect planeRect(IntPoint(), plane.size);
-        resourceProvider->upload(plane.resourceId, softwarePlanePixels, planeRect, planeRect, planeRect);
+        resourceProvider->upload(plane.resourceId, softwarePlanePixels, planeRect, planeRect, IntSize());
     }
     return true;
 }
index f6260e8..792f104 100644 (file)
@@ -1,3 +1,26 @@
+2012-08-16  Alexandre Elias  <aelias@google.com>
+
+        [chromium] Replace destRect with destOffset in texture upload
+        https://bugs.webkit.org/show_bug.cgi?id=94154
+
+        Reviewed by James Robinson.
+
+        Previously, texture upload code implicitly assumed that sourceRect and
+        destRect have the same size. The behavior is undefined if they are
+        different, since they are used interchangeably and there's no support
+        for scaling from one rect to the other. This patch enforces that
+        constraint at the interface level by replacing all instances of
+        "IntRect destRect" by "IntSize destOffset".
+
+        * tests/CCResourceProviderTest.cpp:
+        (WebKit::TEST_F):
+        * tests/CCTextureUpdateControllerTest.cpp:
+        * tests/CCTiledLayerTestCommon.cpp:
+        (WebKitTests::FakeLayerTextureUpdater::Texture::updateRect):
+        * tests/CCTiledLayerTestCommon.h:
+        (Texture):
+        (WebKitTests::FakeTextureUploader::uploadTexture):
+
 2012-08-16  James Robinson  <jamesr@chromium.org>
 
         [chromium] Remove unnecessary tree hierarchy inspection APIs from WebLayer
index 4aa1afd..59baeac 100644 (file)
@@ -2353,7 +2353,7 @@ public:
     static PassOwnPtr<EvictionTrackingTexture> create(PassOwnPtr<CCPrioritizedTexture> texture) { return adoptPtr(new EvictionTrackingTexture(texture)); }
     virtual ~EvictionTrackingTexture() { }
 
-    virtual void updateRect(CCResourceProvider* resourceProvider, const IntRect&, const IntRect&) OVERRIDE
+    virtual void updateRect(CCResourceProvider* resourceProvider, const IntRect&, const IntSize&) OVERRIDE
     {
         ASSERT_TRUE(!texture()->haveBackingTexture() || resourceProvider->numResources() > 0);
         texture()->acquireBackingTexture(resourceProvider);
@@ -2438,7 +2438,7 @@ void EvictionTestLayer::update(CCTextureUpdateQueue& queue, const CCOcclusionTra
     if (!m_texture.get())
         return;
     IntRect fullRect(0, 0, 10, 10);
-    TextureUploader::Parameters parameters = { m_texture.get(), fullRect, fullRect };
+    TextureUploader::Parameters parameters = { m_texture.get(), fullRect, IntSize() };
     queue.appendFullUpload(parameters);
 }
 
index 4227745..228a200 100644 (file)
@@ -223,7 +223,7 @@ TEST_F(CCResourceProviderTest, Basic)
 
     uint8_t data[4] = {1, 2, 3, 4};
     IntRect rect(IntPoint(), size);
-    m_resourceProvider->upload(id, data, rect, rect, rect);
+    m_resourceProvider->upload(id, data, rect, rect, IntSize());
 
     uint8_t result[4] = {0};
     getResourcePixels(id, size, format, result);
@@ -263,7 +263,7 @@ TEST_F(CCResourceProviderTest, Upload)
 
     uint8_t image[16] = {0};
     IntRect imageRect(IntPoint(), size);
-    m_resourceProvider->upload(id, image, imageRect, imageRect, imageRect);
+    m_resourceProvider->upload(id, image, imageRect, imageRect, IntSize());
 
     for (uint8_t i = 0 ; i < pixelSize; ++i)
         image[i] = i;
@@ -271,8 +271,8 @@ TEST_F(CCResourceProviderTest, Upload)
     uint8_t result[16] = {0};
     {
         IntRect sourceRect(0, 0, 1, 1);
-        IntRect destRect(0, 0, 1, 1);
-        m_resourceProvider->upload(id, image, imageRect, sourceRect, destRect);
+        IntSize destOffset(0, 0);
+        m_resourceProvider->upload(id, image, imageRect, sourceRect, destOffset);
 
         uint8_t expected[16] = {0, 1, 2, 3,   0, 0, 0, 0,
                                 0, 0, 0, 0,   0, 0, 0, 0};
@@ -281,8 +281,8 @@ TEST_F(CCResourceProviderTest, Upload)
     }
     {
         IntRect sourceRect(0, 0, 1, 1);
-        IntRect destRect(1, 1, 1, 1);
-        m_resourceProvider->upload(id, image, imageRect, sourceRect, destRect);
+        IntSize destOffset(1, 1);
+        m_resourceProvider->upload(id, image, imageRect, sourceRect, destOffset);
 
         uint8_t expected[16] = {0, 1, 2, 3,   0, 0, 0, 0,
                                 0, 0, 0, 0,   0, 1, 2, 3};
@@ -291,8 +291,8 @@ TEST_F(CCResourceProviderTest, Upload)
     }
     {
         IntRect sourceRect(1, 0, 1, 1);
-        IntRect destRect(0, 1, 1, 1);
-        m_resourceProvider->upload(id, image, imageRect, sourceRect, destRect);
+        IntSize destOffset(0, 1);
+        m_resourceProvider->upload(id, image, imageRect, sourceRect, destOffset);
 
         uint8_t expected[16] = {0, 1, 2, 3,   0, 0, 0, 0,
                                 4, 5, 6, 7,   0, 1, 2, 3};
index 695832f..0b0edf3 100644 (file)
@@ -89,7 +89,7 @@ private:
 class TextureForUploadTest : public LayerTextureUpdater::Texture {
 public:
     TextureForUploadTest() : LayerTextureUpdater::Texture(adoptPtr<CCPrioritizedTexture>(0)) { }
-    virtual void updateRect(CCResourceProvider*, const IntRect& sourceRect, const IntRect& destRect) { }
+    virtual void updateRect(CCResourceProvider*, const IntRect& sourceRect, const IntSize& destOffset) { }
 };
 
 
@@ -190,7 +190,7 @@ protected:
         m_totalUploadCountExpected += count;
 
         const IntRect rect(0, 0, 300, 150);
-        const TextureUploader::Parameters upload = { &m_texture, rect, rect };
+        const TextureUploader::Parameters upload = { &m_texture, rect, IntSize() };
         for (int i = 0; i < count; i++)
             m_queue->appendFullUpload(upload);
     }
@@ -201,7 +201,7 @@ protected:
         m_totalUploadCountExpected += count;
 
         const IntRect rect(0, 0, 100, 100);
-        const TextureUploader::Parameters upload = { &m_texture, rect, rect };
+        const TextureUploader::Parameters upload = { &m_texture, rect, IntSize() };
         for (int i = 0; i < count; i++)
             m_queue->appendPartialUpload(upload);
     }
index 54f29bb..58f37c6 100644 (file)
@@ -40,7 +40,7 @@ FakeLayerTextureUpdater::Texture::~Texture()
 {
 }
 
-void FakeLayerTextureUpdater::Texture::updateRect(CCResourceProvider* resourceProvider, const IntRect&, const IntRect&)
+void FakeLayerTextureUpdater::Texture::updateRect(CCResourceProvider* resourceProvider, const IntRect&, const IntSize&)
 {
     texture()->acquireBackingTexture(resourceProvider);
     m_layer->updateRect();
index c75932a..27c7caa 100644 (file)
@@ -49,7 +49,7 @@ public:
         Texture(FakeLayerTextureUpdater*, PassOwnPtr<WebCore::CCPrioritizedTexture>);
         virtual ~Texture();
 
-        virtual void updateRect(WebCore::CCResourceProvider* , const WebCore::IntRect&, const WebCore::IntRect&) OVERRIDE;
+        virtual void updateRect(WebCore::CCResourceProvider* , const WebCore::IntRect&, const WebCore::IntSize&) OVERRIDE;
         virtual void prepareRect(const WebCore::IntRect&, WebCore::CCRenderingStats&) OVERRIDE;
 
     private:
@@ -162,7 +162,7 @@ public:
     virtual bool isBusy() { return false; }
     virtual void beginUploads() { }
     virtual void endUploads() { }
-    virtual void uploadTexture(WebCore::CCResourceProvider* resourceProvider, Parameters upload) { upload.texture->updateRect(resourceProvider, upload.sourceRect, upload.destRect); }
+    virtual void uploadTexture(WebCore::CCResourceProvider* resourceProvider, Parameters upload) { upload.texture->updateRect(resourceProvider, upload.sourceRect, upload.destOffset); }
 };
 
 }