Title: [120507] trunk/Source/WebCore
Revision
120507
Author
jam...@google.com
Date
2012-06-15 16:13:00 -0700 (Fri, 15 Jun 2012)

Log Message

[chromium] Use SkBitmap in ImageLayerChromium
https://bugs.webkit.org/show_bug.cgi?id=89134

Reviewed by Adrienne Walker.

GraphicsLayer::setContentsToImage(Image*) is called whenever an image layer's image is or might have changed.
In Chromium, this used to hang on to a RefPtr<WebCore::Image> until the compositor was ready to upload texture contents.
This is potentially a bit fishy since the Image itself might not be in exactly the same state when we get around
to uploading textures and it also creates a bad dependency from ImageLayerChromium on WebCore::Image.

This patch grabs the underlying SkBitmap in the setContentsTo call and passes that into ImageLayerChromium
instead. I've also removed the venerable but redundant PlatformImage concept since all of chromium's images are
skia bitmaps these days.

Covered by existing tests, particularly compositing/images/ and compositing/color-matching/.

* WebCore.gypi:
* platform/graphics/chromium/GraphicsLayerChromium.cpp:
(WebCore::GraphicsLayerChromium::setContentsToImage):
* platform/graphics/chromium/ImageLayerChromium.cpp:
(WebCore::ImageLayerTextureUpdater::updateTextureRect):
(WebCore::ImageLayerTextureUpdater::setBitmap):
(ImageLayerTextureUpdater):
(WebCore::ImageLayerChromium::ImageLayerChromium):
(WebCore::ImageLayerChromium::setBitmap):
(WebCore::ImageLayerChromium::update):
(WebCore::ImageLayerChromium::contentBounds):
(WebCore::ImageLayerChromium::drawsContent):
* platform/graphics/chromium/ImageLayerChromium.h:
(ImageLayerChromium):
* platform/graphics/chromium/PlatformImage.cpp: Removed.
* platform/graphics/chromium/PlatformImage.h: Removed.

Modified Paths

Removed Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (120506 => 120507)


--- trunk/Source/WebCore/ChangeLog	2012-06-15 23:09:50 UTC (rev 120506)
+++ trunk/Source/WebCore/ChangeLog	2012-06-15 23:13:00 UTC (rev 120507)
@@ -1,3 +1,38 @@
+2012-06-14  James Robinson  <jam...@chromium.org>
+
+        [chromium] Use SkBitmap in ImageLayerChromium
+        https://bugs.webkit.org/show_bug.cgi?id=89134
+
+        Reviewed by Adrienne Walker.
+
+        GraphicsLayer::setContentsToImage(Image*) is called whenever an image layer's image is or might have changed.
+        In Chromium, this used to hang on to a RefPtr<WebCore::Image> until the compositor was ready to upload texture contents.
+        This is potentially a bit fishy since the Image itself might not be in exactly the same state when we get around
+        to uploading textures and it also creates a bad dependency from ImageLayerChromium on WebCore::Image.
+
+        This patch grabs the underlying SkBitmap in the setContentsTo call and passes that into ImageLayerChromium
+        instead. I've also removed the venerable but redundant PlatformImage concept since all of chromium's images are
+        skia bitmaps these days.
+
+        Covered by existing tests, particularly compositing/images/ and compositing/color-matching/.
+
+        * WebCore.gypi:
+        * platform/graphics/chromium/GraphicsLayerChromium.cpp:
+        (WebCore::GraphicsLayerChromium::setContentsToImage):
+        * platform/graphics/chromium/ImageLayerChromium.cpp:
+        (WebCore::ImageLayerTextureUpdater::updateTextureRect):
+        (WebCore::ImageLayerTextureUpdater::setBitmap):
+        (ImageLayerTextureUpdater):
+        (WebCore::ImageLayerChromium::ImageLayerChromium):
+        (WebCore::ImageLayerChromium::setBitmap):
+        (WebCore::ImageLayerChromium::update):
+        (WebCore::ImageLayerChromium::contentBounds):
+        (WebCore::ImageLayerChromium::drawsContent):
+        * platform/graphics/chromium/ImageLayerChromium.h:
+        (ImageLayerChromium):
+        * platform/graphics/chromium/PlatformImage.cpp: Removed.
+        * platform/graphics/chromium/PlatformImage.h: Removed.
+
 2012-06-15  Eli Fidler  <efid...@rim.com>
 
         [BlackBerry] Use platform font settings for the standard settings.

Modified: trunk/Source/WebCore/WebCore.gypi (120506 => 120507)


--- trunk/Source/WebCore/WebCore.gypi	2012-06-15 23:09:50 UTC (rev 120506)
+++ trunk/Source/WebCore/WebCore.gypi	2012-06-15 23:13:00 UTC (rev 120507)
@@ -3630,8 +3630,6 @@
             'platform/graphics/chromium/PlatformCanvas.h',
             'platform/graphics/chromium/PlatformColor.h',
             'platform/graphics/chromium/PlatformIcon.h',
-            'platform/graphics/chromium/PlatformImage.cpp',
-            'platform/graphics/chromium/PlatformImage.h',
             'platform/graphics/chromium/ProgramBinding.cpp',
             'platform/graphics/chromium/ProgramBinding.h',
             'platform/graphics/chromium/RateLimiter.cpp',

Modified: trunk/Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp (120506 => 120507)


--- trunk/Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp	2012-06-15 23:09:50 UTC (rev 120506)
+++ trunk/Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp	2012-06-15 23:13:00 UTC (rev 120507)
@@ -55,6 +55,7 @@
 #include "ImageLayerChromium.h"
 #include "LayerChromium.h"
 #include "LinkHighlight.h"
+#include "NativeImageSkia.h"
 #include "PlatformContextSkia.h"
 #include "PlatformString.h"
 #include "SkMatrix44.h"
@@ -464,7 +465,8 @@
             childrenChanged = true;
         }
         ImageLayerChromium* imageLayer = static_cast<ImageLayerChromium*>(m_contentsLayer.unwrap<LayerChromium>());
-        imageLayer->setContents(image);
+        NativeImageSkia* nativeImage = image->nativeImageForCurrentFrame();
+        imageLayer->setBitmap(nativeImage->bitmap());
         imageLayer->setOpaque(image->isBitmapImage() && !image->currentFrameHasAlpha());
         updateContentsRect();
     } else {

Modified: trunk/Source/WebCore/platform/graphics/chromium/ImageLayerChromium.cpp (120506 => 120507)


--- trunk/Source/WebCore/platform/graphics/chromium/ImageLayerChromium.cpp	2012-06-15 23:09:50 UTC (rev 120506)
+++ trunk/Source/WebCore/platform/graphics/chromium/ImageLayerChromium.cpp	2012-06-15 23:13:00 UTC (rev 120507)
@@ -54,7 +54,7 @@
         {
         }
 
-        virtual void updateRect(CCGraphicsContext* context, TextureAllocator* allocator, const IntRect& sourceRect, const IntRect& destRect)
+        virtual void updateRect(CCGraphicsContext* context, TextureAllocator* allocator, const IntRect& sourceRect, const IntRect& destRect) OVERRIDE
         {
             textureUpdater()->updateTextureRect(context, allocator, texture(), sourceRect, destRect);
         }
@@ -77,55 +77,42 @@
         return adoptPtr(new Texture(this, ManagedTexture::create(manager)));
     }
 
-    virtual SampledTexelFormat sampledTexelFormat(GC3Denum textureFormat)
+    virtual SampledTexelFormat sampledTexelFormat(GC3Denum textureFormat) OVERRIDE
     {
         return PlatformColor::sameComponentOrder(textureFormat) ?
                 LayerTextureUpdater::SampledTexelFormatRGBA : LayerTextureUpdater::SampledTexelFormatBGRA;
     }
 
-    virtual void updateLayerRect(const IntRect& contentRect, const IntSize& tileSize, int /* borderTexels */, float /* contentsScale */, IntRect* /* resultingOpaqueRect */)
+    void updateTextureRect(CCGraphicsContext* context, TextureAllocator* allocator, ManagedTexture* texture, const IntRect& sourceRect, const IntRect& destRect)
     {
-        m_texSubImage.setSubImageSize(tileSize);
-    }
-
-    virtual void updateTextureRect(CCGraphicsContext* context, TextureAllocator* allocator, ManagedTexture* texture, const IntRect& sourceRect, const IntRect& destRect)
-    {
         texture->bindTexture(context, allocator);
 
         // Source rect should never go outside the image pixels, even if this
         // is requested because the texture extends outside the image.
         IntRect clippedSourceRect = sourceRect;
-        clippedSourceRect.intersect(imageRect());
+        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());
 
-        m_texSubImage.upload(m_image.pixels(), imageRect(), clippedSourceRect, clippedDestRect, texture->format(), context);
+        SkAutoLockPixels lock(m_bitmap);
+        m_texSubImage.upload(static_cast<const uint8_t*>(m_bitmap.getPixels()), imageRect, clippedSourceRect, clippedDestRect, texture->format(), context);
     }
 
-    void updateFromImage(NativeImagePtr nativeImage)
+    void setBitmap(const SkBitmap& bitmap)
     {
-        m_image.updateFromImage(nativeImage);
+        m_bitmap = bitmap;
     }
 
-    IntSize imageSize() const
-    {
-        return m_image.size();
-    }
-
 private:
     explicit ImageLayerTextureUpdater(bool useMapTexSubImage)
         : m_texSubImage(useMapTexSubImage)
     {
     }
 
-    IntRect imageRect() const
-    {
-        return IntRect(IntPoint::zero(), m_image.size());
-    }
-
-    PlatformImage m_image;
+    SkBitmap m_bitmap;
     LayerTextureSubImage m_texSubImage;
 };
 
@@ -136,7 +123,6 @@
 
 ImageLayerChromium::ImageLayerChromium()
     : TiledLayerChromium()
-    , m_imageForCurrentFrame(0)
 {
 }
 
@@ -144,17 +130,16 @@
 {
 }
 
-void ImageLayerChromium::setContents(Image* contents)
+void ImageLayerChromium::setBitmap(const SkBitmap& bitmap)
 {
-    // setContents() currently gets called whenever there is any
+    // setBitmap() currently gets called whenever there is any
     // style change that affects the layer even if that change doesn't
     // affect the actual contents of the image (e.g. a CSS animation).
     // With this check in place we avoid unecessary texture uploads.
-    if ((m_contents == contents) && (m_contents->nativeImageForCurrentFrame() == m_imageForCurrentFrame))
+    if (bitmap.pixelRef() && bitmap.pixelRef() == m_bitmap.pixelRef())
         return;
 
-    m_contents = contents;
-    m_imageForCurrentFrame = m_contents->nativeImageForCurrentFrame();
+    m_bitmap = bitmap;
     setNeedsDisplay();
 }
 
@@ -162,7 +147,7 @@
 {
     createTextureUpdaterIfNeeded();
     if (m_needsDisplay) {
-        m_textureUpdater->updateFromImage(m_contents->nativeImageForCurrentFrame());
+        m_textureUpdater->setBitmap(m_bitmap);
         updateTileSizeAndTilingOption();
         invalidateRect(IntRect(IntPoint(), contentBounds()));
         m_needsDisplay = false;
@@ -189,14 +174,12 @@
 
 IntSize ImageLayerChromium::contentBounds() const
 {
-    if (!m_contents)
-        return IntSize();
-    return m_contents->size();
+    return IntSize(m_bitmap.width(), m_bitmap.height());
 }
 
 bool ImageLayerChromium::drawsContent() const
 {
-    return m_contents && TiledLayerChromium::drawsContent();
+    return !m_bitmap.isNull() && TiledLayerChromium::drawsContent();
 }
 
 bool ImageLayerChromium::needsContentsScale() const

Modified: trunk/Source/WebCore/platform/graphics/chromium/ImageLayerChromium.h (120506 => 120507)


--- trunk/Source/WebCore/platform/graphics/chromium/ImageLayerChromium.h	2012-06-15 23:09:50 UTC (rev 120506)
+++ trunk/Source/WebCore/platform/graphics/chromium/ImageLayerChromium.h	2012-06-15 23:13:00 UTC (rev 120507)
@@ -35,11 +35,10 @@
 #if USE(ACCELERATED_COMPOSITING)
 
 #include "ContentLayerChromium.h"
-#include "PlatformImage.h"
+#include "SkBitmap.h"
 
 namespace WebCore {
 
-class Image;
 class ImageLayerTextureUpdater;
 
 // A Layer that contains only an Image element.
@@ -52,7 +51,7 @@
     virtual void update(CCTextureUpdater&, const CCOcclusionTracker*) OVERRIDE;
     virtual bool needsContentsScale() const OVERRIDE;
 
-    void setContents(Image* image);
+    void setBitmap(const SkBitmap& image);
 
 private:
     ImageLayerChromium();
@@ -63,8 +62,7 @@
     virtual void createTextureUpdaterIfNeeded() OVERRIDE;
     virtual IntSize contentBounds() const OVERRIDE;
 
-    NativeImagePtr m_imageForCurrentFrame;
-    RefPtr<Image> m_contents;
+    SkBitmap m_bitmap;
 
     RefPtr<ImageLayerTextureUpdater> m_textureUpdater;
 };

Deleted: trunk/Source/WebCore/platform/graphics/chromium/PlatformImage.cpp (120506 => 120507)


--- trunk/Source/WebCore/platform/graphics/chromium/PlatformImage.cpp	2012-06-15 23:09:50 UTC (rev 120506)
+++ trunk/Source/WebCore/platform/graphics/chromium/PlatformImage.cpp	2012-06-15 23:13:00 UTC (rev 120507)
@@ -1,60 +0,0 @@
-/*
- * Copyright (C) 2011 Google Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- * 1.  Redistributions of source code must retain the above copyright
- *     notice, this list of conditions and the following disclaimer.
- * 2.  Redistributions in binary form must reproduce the above copyright
- *     notice, this list of conditions and the following disclaimer in the
- *     documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
- * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
- * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
- * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
- * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
- * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "config.h"
-#include "PlatformImage.h"
-
-#include "NativeImageSkia.h"
-#include "PlatformContextSkia.h"
-
-namespace WebCore {
-
-PlatformImage::PlatformImage()
-{
-}
-
-void PlatformImage::updateFromImage(NativeImagePtr nativeImage)
-{
-    // The layer contains an Image.
-    NativeImageSkia* skiaImage = static_cast<NativeImageSkia*>(nativeImage);
-    ASSERT(skiaImage);
-    const SkBitmap& skiaBitmap = skiaImage->bitmap();
-
-    IntSize bitmapSize(skiaBitmap.width(), skiaBitmap.height());
-
-    size_t bufferSize = bitmapSize.width() * bitmapSize.height() * 4;
-    if (m_size != bitmapSize) {
-        m_pixelData = adoptArrayPtr(new uint8_t[bufferSize]);
-        memset(m_pixelData.get(), 0, bufferSize);
-        m_size = bitmapSize;
-    }
-
-    SkAutoLockPixels lock(skiaBitmap);
-    // FIXME: do we need to support more image configurations?
-    ASSERT(skiaBitmap.config()== SkBitmap::kARGB_8888_Config);
-    skiaBitmap.copyPixelsTo(m_pixelData.get(), bufferSize);
-}
-
-} // namespace WebCore

Deleted: trunk/Source/WebCore/platform/graphics/chromium/PlatformImage.h (120506 => 120507)


--- trunk/Source/WebCore/platform/graphics/chromium/PlatformImage.h	2012-06-15 23:09:50 UTC (rev 120506)
+++ trunk/Source/WebCore/platform/graphics/chromium/PlatformImage.h	2012-06-15 23:13:00 UTC (rev 120507)
@@ -1,53 +0,0 @@
-/*
- * Copyright (C) 2011 Google Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- * 1.  Redistributions of source code must retain the above copyright
- *     notice, this list of conditions and the following disclaimer.
- * 2.  Redistributions in binary form must reproduce the above copyright
- *     notice, this list of conditions and the following disclaimer in the
- *     documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
- * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
- * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
- * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
- * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
- * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef PlatformImage_h
-#define PlatformImage_h
-
-#include "ImageSource.h"
-#include "IntSize.h"
-#include <stdint.h>
-#include <wtf/Noncopyable.h>
-#include <wtf/OwnArrayPtr.h>
-
-namespace WebCore {
-
-class PlatformImage {
-    WTF_MAKE_NONCOPYABLE(PlatformImage);
-public:
-    PlatformImage();
-
-    void updateFromImage(NativeImagePtr);
-    const uint8_t* pixels() const { return m_pixelData ? &m_pixelData[0] : 0; }
-    IntSize size() const { return m_size; }
-
-private:
-    OwnArrayPtr<uint8_t> m_pixelData;
-    IntSize m_size;
-};
-
-} // namespace WebCore
-
-#endif
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to