Bug 43507 – Turn ImageBuffer::image/ImageBuffer::imageForRendering to ImageBuffer::image/ImageBuffer::copiedImage

• Component: Images
• Status: RESOLVED FIXED

[Martin Robinson]

Most of the time ImageBuffer::image should be a live representation of an image buffer, but in some cases it should be a copy (e.g. creating patterns from 2D canvas rendering contexts). The current naming scheme was confusing enough that Cairo is making a large number of unnecessary full-canvas pixel copies.

[Martin Robinson]

Additional information:
01:56:29 PM) dhyatt: mrobinson: we're pretty sure CG is doing many unnecessary copies as well
(01:56:38 PM) dhyatt: mrobinson: so it's not just a cairo issue :)

[Martin Robinson]

Created attachment 63782 [details]
WIP patch

[Martin Robinson]

I've posted a work-in-progress patch. I'd really appreciate it if someone could take a quick look at it before I get much further. This patch essentially splits ::image() into ::image() (shallow copy) and ::copiedImage (deep copy). For the CG port these seem to be equivalent, as CGBitmapContextCreateImage has copy-on-write behavior (according to http://developer.apple.com/mac/library/documentation/GraphicsImaging/Reference/CGBitmapContext/Reference/reference.html#//apple_ref/c/func/CGBitmapContextCreateImage ). I've also implemented the necessary logic for Skia, Qt and Cairo (though I can only test Cairo currently).

Since ImageBuffer no longer caches the image, the calls need to worry more about reference counting. ::image() and ::copiedImage return PassRefPtr's now, which spidered up the call chain unfortunately. Perhaps there is a better solution there.

[Dave Hyatt]

I did a very similar patch locally and ended up keeping the caching of the shallow copy and only returning a PassRefPtr for the deep copy.  My concern is whether or not CG is really doing copy-on-write or not.  If they aren't, then removing the cache in the shallow copy case is going to be a perf hit.

Let me ask some CG engineers about it.

[Dave Hyatt]

I have a patch I'm going to post shortly.

[Dave Hyatt]

Created attachment 64253 [details]
Patch

This patch not ready for review... just posting it to see if everything builds.

[Dave Hyatt]

Comment on attachment 64253 [details]
Patch

Flagging for review for build info.

[James Robinson]

Comment on attachment 64253 [details]
Patch

> Index: WebCore/platform/graphics/ImageBuffer.h
> ===================================================================
> +#if PLATFORM(CG) || PLATFORM(QT)
> +        void draw(GraphicsContext*, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect = FloatRect(0, 0, -1, -1),
> +                             CompositeOperator = CompositeSourceOver, bool useLowQualityScale = false);
> +        void drawPattern(GraphicsContext*, const FloatRect& srcRect, const AffineTransform& patternTransform,
> +                         const FloatPoint& phase, ColorSpace styleColorSpace, CompositeOperator, const FloatRect& destRect);
>  #else
> -        Image* imageForRendering() const { return image(); }
> +        void draw(GraphicsContext*, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect = FloatRect(0, 0, -1, -1),

This won't compile on non-CG/QT - the first parameter has to be named.  Otherwise, this patch compiles fine on Chromium+skia

[James Robinson]

I think Stephen had some comments.  It shouldn't be too much work to hook this up for Skia as well, I'm just not personally super familiar with how the image types are supposed to interact in skia.

[Stephen White]

Comment on attachment 64253 [details]
Patch

> Index: WebCore/WebCore.xcodeproj/project.pbxproj
> ===================================================================
> --- WebCore/WebCore.xcodeproj/project.pbxproj	(revision 65160)
> +++ WebCore/WebCore.xcodeproj/project.pbxproj	(working copy)
> @@ -20215,7 +20215,6 @@
>  			isa = PBXProject;
>  			buildConfigurationList = 149C284308902B11008A9EFC /* Build configuration list for PBXProject "WebCore" */;
>  			compatibilityVersion = "Xcode 2.4";
> -			developmentRegion = English;
>  			hasScannedForEncodings = 1;
>  			knownRegions = (
>  				English,
> Index: WebCore/css/CSSCanvasValue.cpp
> ===================================================================
> --- WebCore/css/CSSCanvasValue.cpp	(revision 65160)
> +++ WebCore/css/CSSCanvasValue.cpp	(working copy)
> @@ -90,7 +90,11 @@ Image* CSSCanvasValue::image(RenderObjec
>      HTMLCanvasElement* elt = element(renderer->document());
>      if (!elt || !elt->buffer())
>          return 0;
> -    return elt->buffer()->image();
> +        
> +    // FIXME: For platforms that can render a live image, this is pointlessly slow.
> +    // The whole way this is architected is silly for those platforms.
> +    RefPtr<Image> copiedImage = elt->buffer()->copiedImage();
> +    return copiedImage.get(); 
>  }
>  
>  } // namespace WebCore
> Index: WebCore/html/HTMLCanvasElement.cpp
> ===================================================================
> --- WebCore/html/HTMLCanvasElement.cpp	(revision 65160)
> +++ WebCore/html/HTMLCanvasElement.cpp	(working copy)
> @@ -209,9 +209,6 @@ CanvasRenderingContext* HTMLCanvasElemen
>  
>  void HTMLCanvasElement::willDraw(const FloatRect& rect)
>  {
> -    if (m_imageBuffer)
> -        m_imageBuffer->clearImage();
> -
>      if (RenderBox* ro = renderBox()) {
>          FloatRect destRect = ro->contentBoxRect();
>          FloatRect r = mapRect(rect, FloatRect(0, 0, size().width(), size().height()), destRect);
> @@ -283,11 +280,8 @@ void HTMLCanvasElement::paint(GraphicsCo
>  
>      if (hasCreatedImageBuffer()) {
>          ImageBuffer* imageBuffer = buffer();
> -        if (imageBuffer) {
> -            Image* image = imageBuffer->imageForRendering();
> -            if (image)
> -                context->drawImage(image, DeviceColorSpace, r);
> -        }
> +        if (imageBuffer)
> +            imageBuffer->draw(context, DeviceColorSpace, r);
>      }
>  
>  #if ENABLE(3D_CANVAS)
> Index: WebCore/html/canvas/CanvasPattern.cpp
> ===================================================================
> --- WebCore/html/canvas/CanvasPattern.cpp	(revision 65160)
> +++ WebCore/html/canvas/CanvasPattern.cpp	(working copy)
> @@ -57,7 +57,7 @@ void CanvasPattern::parseRepetitionType(
>      ec = SYNTAX_ERR;
>  }
>  
> -CanvasPattern::CanvasPattern(Image* image, bool repeatX, bool repeatY, bool originClean)
> +CanvasPattern::CanvasPattern(PassRefPtr<Image> image, bool repeatX, bool repeatY, bool originClean)
>      : m_pattern(Pattern::create(image, repeatX, repeatY))
>      , m_originClean(originClean)
>  {
> Index: WebCore/html/canvas/CanvasPattern.h
> ===================================================================
> --- WebCore/html/canvas/CanvasPattern.h	(revision 65160)
> +++ WebCore/html/canvas/CanvasPattern.h	(working copy)
> @@ -41,7 +41,7 @@ namespace WebCore {
>      public:
>          static void parseRepetitionType(const String&, bool& repeatX, bool& repeatY, ExceptionCode&);
>  
> -        static PassRefPtr<CanvasPattern> create(Image* image, bool repeatX, bool repeatY, bool originClean)
> +        static PassRefPtr<CanvasPattern> create(PassRefPtr<Image> image, bool repeatX, bool repeatY, bool originClean)
>          {
>              return adoptRef(new CanvasPattern(image, repeatX, repeatY, originClean));
>          }
> @@ -51,7 +51,7 @@ namespace WebCore {
>          bool originClean() const { return m_originClean; }
>  
>      private:
> -        CanvasPattern(Image*, bool repeatX, bool repeatY, bool originClean);
> +        CanvasPattern(PassRefPtr<Image>, bool repeatX, bool repeatY, bool originClean);
>  
>          RefPtr<Pattern> m_pattern;
>          bool m_originClean;
> Index: WebCore/html/canvas/CanvasRenderingContext2D.cpp
> ===================================================================
> --- WebCore/html/canvas/CanvasRenderingContext2D.cpp	(revision 65160)
> +++ WebCore/html/canvas/CanvasRenderingContext2D.cpp	(working copy)
> @@ -1264,7 +1264,12 @@ void CanvasRenderingContext2D::drawImage
>  
>      sourceCanvas->makeRenderingResultsAvailable();
>  
> -    c->drawImage(buffer->image(), DeviceColorSpace, destRect, sourceRect, state().m_globalComposite);
> +    if (c == buffer->context()) {
> +        // We're drawing into our own buffer.  In order for this to work, we need to copy the source buffer first.
> +        RefPtr<Image> copiedImage = buffer->copiedImage();
> +        c->drawImage(copiedImage.get(), DeviceColorSpace, destRect, sourceRect, state().m_globalComposite);
> +    } else
> +        buffer->draw(c,  DeviceColorSpace, destRect, sourceRect, state().m_globalComposite);
>      willDraw(destRect); // This call comes after drawImage, since the buffer we draw into may be our own, and we need to make sure it is dirty.
>                          // FIXME: Arguably willDraw should become didDraw and occur after drawing calls and not before them to avoid problems like this.
>  }
> @@ -1464,7 +1469,7 @@ PassRefPtr<CanvasPattern> CanvasRenderin
>      CanvasPattern::parseRepetitionType(repetitionType, repeatX, repeatY, ec);
>      if (ec)
>          return 0;
> -    return CanvasPattern::create(canvas->buffer()->image(), repeatX, repeatY, canvas->originClean());
> +    return CanvasPattern::create(canvas->buffer()->copiedImage(), repeatX, repeatY, canvas->originClean());
>  }
>  
>  void CanvasRenderingContext2D::willDraw(const FloatRect& r, unsigned options)
> @@ -1820,7 +1825,7 @@ void CanvasRenderingContext2D::drawTextI
>          maskImageContext->drawBidiText(font, textRun, location);
>  
>          c->save();
> -        c->clipToImageBuffer(maskRect, maskImage.get());
> +        maskImage->clip(c, maskRect);
>          drawStyle->applyFillColor(c);
>          c->fillRect(maskRect);
>          c->restore();
> Index: WebCore/html/canvas/WebGLRenderingContext.cpp
> ===================================================================
> --- WebCore/html/canvas/WebGLRenderingContext.cpp	(revision 65160)
> +++ WebCore/html/canvas/WebGLRenderingContext.cpp	(working copy)
> @@ -161,7 +161,6 @@ void WebGLRenderingContext::paintRenderi
>      if (m_markedCanvasDirty) {
>          // FIXME: It should not be necessary to clear the image before doing a readback.
>          // Investigate why this is needed and remove if possible.
> -        canvas()->buffer()->clearImage();
>          m_markedCanvasDirty = false;
>          m_context->paintRenderingResultsToCanvas(this);
>      }
> @@ -2196,7 +2195,9 @@ void WebGLRenderingContext::texImage2D(u
>          m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
>          return;
>      }
> -    texImage2DImpl(target, level, internalformat, format, type, canvas->buffer()->image(),
> +    
> +    RefPtr<Image> copiedImage = canvas->buffer()->copiedImage();
> +    texImage2DImpl(target, level, internalformat, format, type, copiedImage.get(),
>                     m_unpackFlipY, m_unpackPremultiplyAlpha, ec);
>  }
>  
> @@ -2295,7 +2296,9 @@ void WebGLRenderingContext::texImage2D(u
>          m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
>          return;
>      }
> -    texImage2DImpl(target, level, GraphicsContext3D::RGBA, GraphicsContext3D::RGBA, GraphicsContext3D::UNSIGNED_BYTE, canvas->buffer()->image(), flipY, premultiplyAlpha, ec);
> +    
> +    RefPtr<Image> copiedImage = canvas->buffer()->copiedImage();
> +    texImage2DImpl(target, level, GraphicsContext3D::RGBA, GraphicsContext3D::RGBA, GraphicsContext3D::UNSIGNED_BYTE, copiedImage.get(), flipY, premultiplyAlpha, ec);
>  }
>  
>  void WebGLRenderingContext::texImage2D(unsigned target, unsigned level, HTMLVideoElement* video,
> @@ -2455,7 +2458,9 @@ void WebGLRenderingContext::texSubImage2
>          m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
>          return;
>      }
> -    texSubImage2DImpl(target, level, xoffset, yoffset, format, type, canvas->buffer()->image(),
> +    
> +    RefPtr<Image> copiedImage = canvas->buffer()->copiedImage();
> +    texSubImage2DImpl(target, level, xoffset, yoffset, format, type, copiedImage.get(),
>                        m_unpackFlipY, m_unpackPremultiplyAlpha, ec);
>  }
>  
> @@ -2554,7 +2559,9 @@ void WebGLRenderingContext::texSubImage2
>          m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
>          return;
>      }
> -    texSubImage2DImpl(target, level, xoffset, yoffset, GraphicsContext3D::RGBA, GraphicsContext3D::UNSIGNED_BYTE, canvas->buffer()->image(),
> +    
> +    RefPtr<Image> copiedImage = canvas->buffer()->copiedImage();
> +    texSubImage2DImpl(target, level, xoffset, yoffset, GraphicsContext3D::RGBA, GraphicsContext3D::UNSIGNED_BYTE, copiedImage.get(),
>                        flipY, premultiplyAlpha, ec);
>  }
>  
> Index: WebCore/platform/graphics/GeneratedImage.cpp
> ===================================================================
> --- WebCore/platform/graphics/GeneratedImage.cpp	(revision 65160)
> +++ WebCore/platform/graphics/GeneratedImage.cpp	(working copy)
> @@ -63,11 +63,8 @@ void GeneratedImage::drawPattern(Graphic
>      GraphicsContext* graphicsContext = imageBuffer->context();
>      graphicsContext->fillRect(FloatRect(FloatPoint(), adjustedSize), *m_generator.get());
>  
> -    // Grab the final image from the image buffer.
> -    Image* bitmap = imageBuffer->image();
> -
> -    // Now just call drawTiled on that image.
> -    bitmap->drawPattern(context, adjustedSrcRect, patternTransform, phase, styleColorSpace, compositeOp, destRect);
> +    // Tile the image buffer into the context.
> +    imageBuffer->drawPattern(context, adjustedSrcRect, patternTransform, phase, styleColorSpace, compositeOp, destRect);
>  }
>  
>  }
> Index: WebCore/platform/graphics/GraphicsContext.h
> ===================================================================
> --- WebCore/platform/graphics/GraphicsContext.h	(revision 65160)
> +++ WebCore/platform/graphics/GraphicsContext.h	(working copy)
> @@ -248,7 +248,6 @@ namespace WebCore {
>          void clipOutEllipseInRect(const IntRect&);
>          void clipOutRoundedRect(const IntRect&, const IntSize& topLeft, const IntSize& topRight, const IntSize& bottomLeft, const IntSize& bottomRight);
>          void clipPath(WindRule);
> -        void clipToImageBuffer(const FloatRect&, const ImageBuffer*);
>          void clipConvexPolygon(size_t numPoints, const FloatPoint*, bool antialias = true);
>  
>          int textDrawingMode();
> Index: WebCore/platform/graphics/Image.h
> ===================================================================
> --- WebCore/platform/graphics/Image.h	(revision 65160)
> +++ WebCore/platform/graphics/Image.h	(working copy)
> @@ -150,6 +150,9 @@ public:
>      static PassRefPtr<Image> loadPlatformThemeIcon(const char* name, int size);
>  #endif
>  
> +    virtual void drawPattern(GraphicsContext*, const FloatRect& srcRect, const AffineTransform& patternTransform,
> +                             const FloatPoint& phase, ColorSpace styleColorSpace, CompositeOperator, const FloatRect& destRect);
> +
>  protected:
>      Image(ImageObserver* = 0);
>  
> @@ -167,9 +170,6 @@ protected:
>      virtual bool mayFillWithSolidColor() { return false; }
>      virtual Color solidColor() const { return Color(); }
>      
> -    virtual void drawPattern(GraphicsContext*, const FloatRect& srcRect, const AffineTransform& patternTransform,
> -                             const FloatPoint& phase, ColorSpace styleColorSpace, CompositeOperator, const FloatRect& destRect);
> -
>  private:
>      RefPtr<SharedBuffer> m_data; // The encoded raw data for the image. 
>      ImageObserver* m_imageObserver;
> Index: WebCore/platform/graphics/ImageBuffer.h
> ===================================================================
> --- WebCore/platform/graphics/ImageBuffer.h	(revision 65160)
> +++ WebCore/platform/graphics/ImageBuffer.h	(working copy)
> @@ -29,6 +29,7 @@
>  #define ImageBuffer_h
>  
>  #include "AffineTransform.h"
> +#include "FloatRect.h"
>  #include "Image.h"
>  #include "IntSize.h"
>  #include "ImageBufferData.h"
> @@ -73,15 +74,52 @@ namespace WebCore {
>          const IntSize& size() const { return m_size; }
>          GraphicsContext* context() const;
>  
> -        Image* image() const;
> -#if PLATFORM(QT)
> -        Image* imageForRendering() const;
> +        PassRefPtr<Image> copiedImage() const; // Return a new image that is a copy of the buffer.
> +
> +        void clip(GraphicsContext*, const FloatRect&) const;
> +
> +        // The drawing methods draw the contents of the buffer without copying it.
> +        void draw(GraphicsContext* context, ColorSpace styleColorSpace, const IntPoint& p, CompositeOperator op = CompositeSourceOver)
> +        {
> +            draw(context, styleColorSpace, p, IntRect(0, 0, -1, -1), op);
> +        }
> +
> +        void draw(GraphicsContext* context, ColorSpace styleColorSpace, const IntRect& r, CompositeOperator op = CompositeSourceOver, bool useLowQualityScale = false)
> +        {
> +            draw(context, styleColorSpace, r, IntRect(0, 0, -1, -1), op, useLowQualityScale);
> +        }
> +        
> +        void draw(GraphicsContext* context, ColorSpace styleColorSpace, const IntPoint& destPoint, const IntRect& srcRect, CompositeOperator op = CompositeSourceOver)
> +        {
> +            draw(context, styleColorSpace, IntRect(destPoint, srcRect.size()), srcRect, op);
> +        }
> +        
> +        void draw(GraphicsContext* context, ColorSpace styleColorSpace, const IntRect& destRect, const IntRect& srcRect, CompositeOperator op = CompositeSourceOver, bool useLowQualityScale = false)
> +        {
> +            draw(context, styleColorSpace, FloatRect(destRect), srcRect, op, useLowQualityScale);
> +        }
> +        
> +#if PLATFORM(CG) || PLATFORM(QT)
> +        void draw(GraphicsContext*, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect = FloatRect(0, 0, -1, -1),
> +                             CompositeOperator = CompositeSourceOver, bool useLowQualityScale = false);
> +        void drawPattern(GraphicsContext*, const FloatRect& srcRect, const AffineTransform& patternTransform,
> +                         const FloatPoint& phase, ColorSpace styleColorSpace, CompositeOperator, const FloatRect& destRect);
>  #else
> -        Image* imageForRendering() const { return image(); }
> +        void draw(GraphicsContext*, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect = FloatRect(0, 0, -1, -1),
> +                             CompositeOperator op = CompositeSourceOver, bool useLowQualityScale = false)
> +        {
> +            RefPtr<Image> imageCopy = copiedImage();
> +            context->drawImage(imageCopy.get(), styleColorSpace, destRect, srcRect, op, useLowQualityScale);
> +        }
> +        
> +        void drawPattern(GraphicsContext* context, const FloatRect& srcRect, const AffineTransform& patternTransform,
> +                         const FloatPoint& phase, ColorSpace styleColorSpace, CompositeOperator op, const FloatRect& destRect)
> +        {
> +            RefPtr<Image> imageCopy = copiedImage();
> +            imageCopy->drawPattern(context, srcRect, patternTransform, phase, styleColorSpace, op, destRect);
> +        }
>  #endif
>  
> -        void clearImage() { m_image.clear(); }
> -
>          PassRefPtr<ImageData> getUnmultipliedImageData(const IntRect&) const;
>          PassRefPtr<ImageData> getPremultipliedImageData(const IntRect&) const;
>  
> @@ -101,7 +139,6 @@ namespace WebCore {
>  
>          IntSize m_size;
>          OwnPtr<GraphicsContext> m_context;
> -        mutable RefPtr<Image> m_image;
>  
>  #if !PLATFORM(CG)
>          Vector<int> m_linearRgbLUT;
> Index: WebCore/platform/graphics/Pattern.cpp
> ===================================================================
> --- WebCore/platform/graphics/Pattern.cpp	(revision 65160)
> +++ WebCore/platform/graphics/Pattern.cpp	(working copy)
> @@ -31,7 +31,7 @@
>  
>  namespace WebCore {
>  
> -Pattern::Pattern(Image* image, bool repeatX, bool repeatY)
> +Pattern::Pattern(PassRefPtr<Image> image, bool repeatX, bool repeatY)
>      : m_tileImage(image)
>      , m_repeatX(repeatX)
>      , m_repeatY(repeatY)
> Index: WebCore/platform/graphics/Pattern.h
> ===================================================================
> --- WebCore/platform/graphics/Pattern.h	(revision 65160)
> +++ WebCore/platform/graphics/Pattern.h	(working copy)
> @@ -29,6 +29,7 @@
>  #define Pattern_h
>  
>  #include "AffineTransform.h"
> +#include "Image.h"
>  
>  #include <wtf/PassRefPtr.h>
>  #include <wtf/RefCounted.h>
> @@ -64,11 +65,10 @@ typedef void* PlatformPatternPtr;
>  namespace WebCore {
>  
>  class AffineTransform;
> -class Image;
>  
>  class Pattern : public RefCounted<Pattern> {
>  public:
> -    static PassRefPtr<Pattern> create(Image* tileImage, bool repeatX, bool repeatY)
> +    static PassRefPtr<Pattern> create(PassRefPtr<Image> tileImage, bool repeatX, bool repeatY)
>      {
>          return adoptRef(new Pattern(tileImage, repeatX, repeatY));
>      }
> @@ -91,7 +91,7 @@ public:
>      bool repeatY() const { return m_repeatY; }
>  
>  private:
> -    Pattern(Image*, bool repeatX, bool repeatY);
> +    Pattern(PassRefPtr<Image>, bool repeatX, bool repeatY);
>  
>      RefPtr<Image> m_tileImage;
>      bool m_repeatX;
> Index: WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp
> ===================================================================
> --- WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp	(revision 65160)
> +++ WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp	(working copy)
> @@ -908,14 +908,6 @@ void GraphicsContext::addInnerRoundedRec
>      cairo_set_fill_rule(cr, savedFillRule);
>  }
>  
> -void GraphicsContext::clipToImageBuffer(const FloatRect& rect, const ImageBuffer* imageBuffer)
> -{
> -    if (paintingDisabled())
> -        return;
> -
> -    notImplemented();
> -}
> -
>  void GraphicsContext::setPlatformShadow(FloatSize const& size, float, Color const&, ColorSpace)
>  {
>      // Cairo doesn't support shadows natively, they are drawn manually in the draw*
> Index: WebCore/platform/graphics/cairo/ImageBufferCairo.cpp
> ===================================================================
> --- WebCore/platform/graphics/cairo/ImageBufferCairo.cpp	(revision 65160)
> +++ WebCore/platform/graphics/cairo/ImageBufferCairo.cpp	(working copy)
> @@ -97,26 +97,18 @@ GraphicsContext* ImageBuffer::context() 
>      return m_context.get();
>  }
>  
> -Image* ImageBuffer::image() const
> +PassRefPtr<Image> ImageBuffer::copiedImage() const
>  {
> -    if (!m_image) {
> -        // It's assumed that if image() is called, the actual rendering to the
> -        // GraphicsContext must be done.
> -        ASSERT(context());
> -
> -        // This creates a COPY of the image and will cache that copy. This means
> -        // that if subsequent operations take place on the context, neither the
> -        // currently-returned image, nor the results of future image() calls,
> -        // will contain that operation.
> -        //
> -        // This seems silly, but is the way the CG port works: image() is
> -        // intended to be used only when rendering is "complete."
> -        cairo_surface_t* newsurface = copySurface(m_data.m_surface);
> +    // BitmapImage will release the passed in surface on destruction
> +    return BitmapImage::create(copySurface(m_data.m_surface));
> +}
>  
> -        // BitmapImage will release the passed in surface on destruction
> -        m_image = BitmapImage::create(newsurface);
> -    }
> -    return m_image.get();
> +void ImageBuffer::clip(GraphicsContext* context, const FloatRect&) const
> +{
> +    if (context->paintingDisabled())
> +        return;
> +
> +    notImplemented();
>  }
>  
>  void ImageBuffer::platformTransformColorSpace(const Vector<int>& lookUpTable)
> Index: WebCore/platform/graphics/cg/GraphicsContextCG.cpp
> ===================================================================
> --- WebCore/platform/graphics/cg/GraphicsContextCG.cpp	(revision 65160)
> +++ WebCore/platform/graphics/cg/GraphicsContextCG.cpp	(working copy)
> @@ -753,18 +753,6 @@ void GraphicsContext::addInnerRoundedRec
>      CGContextEOClip(context);
>  }
>  
> -void GraphicsContext::clipToImageBuffer(const FloatRect& rect, const ImageBuffer* imageBuffer)
> -{
> -    if (paintingDisabled())
> -        return;
> -
> -    CGContextTranslateCTM(platformContext(), rect.x(), rect.y() + rect.height());
> -    CGContextScaleCTM(platformContext(), 1, -1);
> -    CGContextClipToMask(platformContext(), FloatRect(FloatPoint(), rect.size()), imageBuffer->image()->getCGImageRef());
> -    CGContextScaleCTM(platformContext(), 1, -1);
> -    CGContextTranslateCTM(platformContext(), -rect.x(), -rect.y() - rect.height());
> -}
> -
>  void GraphicsContext::beginTransparencyLayer(float opacity)
>  {
>      if (paintingDisabled())
> Index: WebCore/platform/graphics/cg/ImageBufferCG.cpp
> ===================================================================
> --- WebCore/platform/graphics/cg/ImageBufferCG.cpp	(revision 65160)
> +++ WebCore/platform/graphics/cg/ImageBufferCG.cpp	(working copy)
> @@ -56,10 +56,10 @@ ImageBuffer::ImageBuffer(const IntSize& 
>      , m_size(size)
>  {
>      success = false;  // Make early return mean failure.
> -    unsigned bytesPerRow;
>      if (size.width() < 0 || size.height() < 0)
>          return;
> -    bytesPerRow = size.width();
> +
> +    unsigned bytesPerRow = size.width();
>      if (imageColorSpace != GrayScale) {
>          // Protect against overflow
>          if (bytesPerRow > 0x3FFFFFFF)
> @@ -67,6 +67,7 @@ ImageBuffer::ImageBuffer(const IntSize& 
>          bytesPerRow *= 4;
>      }
>  
> +    size_t dataSize = size.height() * bytesPerRow;
>      if (!tryFastCalloc(size.height(), bytesPerRow).getValue(m_data.m_data))
>          return;
>  
> @@ -84,14 +85,16 @@ ImageBuffer::ImageBuffer(const IntSize& 
>          case LinearRGB:
>              colorSpace.adoptCF(CGColorSpaceCreateWithName(kCGColorSpaceGenericRGBLinear));
>              break;
> +            
>  #endif
>          default:
>              colorSpace.adoptCF(CGColorSpaceCreateDeviceRGB());
>              break;
>      }
>  
> +    CGBitmapInfo bitmapInfo = (imageColorSpace == GrayScale) ? kCGImageAlphaNone : kCGImageAlphaPremultipliedLast;
>      RetainPtr<CGContextRef> cgContext(AdoptCF, CGBitmapContextCreate(m_data.m_data, size.width(), size.height(), 8, bytesPerRow,
> -        colorSpace.get(), (imageColorSpace == GrayScale) ? kCGImageAlphaNone : kCGImageAlphaPremultipliedLast));
> +                                                                     colorSpace.get(), bitmapInfo));
>      if (!cgContext)
>          return;
>  
> @@ -99,6 +102,12 @@ ImageBuffer::ImageBuffer(const IntSize& 
>      m_context->scale(FloatSize(1, -1));
>      m_context->translate(0, -size.height());
>      success = true;
> +    
> +    // Create a live image that wraps the data.
> +    RetainPtr<CGDataProviderRef> dataProvider(AdoptCF, CGDataProviderCreateWithData(0, m_data.m_data, dataSize, 0));
> +    CGImageRef cgImage = CGImageCreate(m_size.width(), m_size.height(), 8, imageColorSpace != GrayScale ? 32 : 8, bytesPerRow,
> +                                       colorSpace.get(), bitmapInfo, dataProvider.get(), 0, true, kCGRenderingIntentDefault);
> +    m_data.m_image = BitmapImage::create(cgImage);
>  }
>  
>  ImageBuffer::~ImageBuffer()
> @@ -111,17 +120,35 @@ GraphicsContext* ImageBuffer::context() 
>      return m_context.get();
>  }
>  
> -Image* ImageBuffer::image() const
> +PassRefPtr<Image> ImageBuffer::copiedImage() const
>  {
> -    if (!m_image) {
> -        // It's assumed that if image() is called, the actual rendering to the
> -        // GraphicsContext must be done.
> -        ASSERT(context());
> -        CGImageRef cgImage = CGBitmapContextCreateImage(context()->platformContext());
> -        // BitmapImage will release the passed in CGImage on destruction
> -        m_image = BitmapImage::create(cgImage);
> -    }
> -    return m_image.get();
> +    // BitmapImage will release the passed in CGImage on destruction
> +    return BitmapImage::create(CGBitmapContextCreateImage(context()->platformContext()));
> +}
> +
> +void ImageBuffer::draw(GraphicsContext* context, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect,
> +                       CompositeOperator op, bool useLowQualityScale)
> +{
> +    context->drawImage(m_data.m_image.get(), styleColorSpace, destRect, srcRect, op, useLowQualityScale);
> +}
> +
> +void ImageBuffer::drawPattern(GraphicsContext* context, const FloatRect& srcRect, const AffineTransform& patternTransform,
> +                              const FloatPoint& phase, ColorSpace styleColorSpace, CompositeOperator op, const FloatRect& destRect)
> +{
> +    m_data.m_image->drawPattern(context, srcRect, patternTransform, phase, styleColorSpace, op, destRect);
> +}
> +
> +void ImageBuffer::clip(GraphicsContext* context, const FloatRect& rect) const
> +{
> +    if (context->paintingDisabled())
> +        return;
> +
> +    CGContextRef platformContext = context->platformContext();
> +    CGContextTranslateCTM(platformContext, rect.x(), rect.y() + rect.height());
> +    CGContextScaleCTM(platformContext, 1, -1);
> +    CGContextClipToMask(platformContext, FloatRect(FloatPoint(), rect.size()), m_data.m_image->getCGImageRef());
> +    CGContextScaleCTM(platformContext, 1, -1);
> +    CGContextTranslateCTM(platformContext, -rect.x(), -rect.y() - rect.height());
>  }
>  
>  template <Multiply multiplied>
> Index: WebCore/platform/graphics/cg/ImageBufferData.h
> ===================================================================
> --- WebCore/platform/graphics/cg/ImageBufferData.h	(revision 65160)
> +++ WebCore/platform/graphics/cg/ImageBufferData.h	(working copy)
> @@ -26,6 +26,9 @@
>  #ifndef ImageBufferData_h
>  #define ImageBufferData_h
>  
> +#include <wtf/RefPtr.h>
> +#include "Image.h"
> +
>  namespace WebCore {
>  
>  class IntSize;
> @@ -35,6 +38,7 @@ public:
>      ImageBufferData(const IntSize&);
>  
>      void* m_data;
> +    RefPtr<Image> m_image;
>  };
>  
>  }  // namespace WebCore
> Index: WebCore/platform/graphics/filters/FEColorMatrix.cpp
> ===================================================================
> --- WebCore/platform/graphics/filters/FEColorMatrix.cpp	(revision 65160)
> +++ WebCore/platform/graphics/filters/FEColorMatrix.cpp	(working copy)
> @@ -164,7 +164,7 @@ void FEColorMatrix::apply(Filter* filter
>      if (!filterContext)
>          return;
>  
> -    filterContext->drawImage(m_in->resultImage()->image(), DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()));
> +    m_in->resultImage()->draw(filterContext, DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()));
>  
>      IntRect imageRect(IntPoint(), resultImage()->size());
>      PassRefPtr<ImageData> imageData(resultImage()->getUnmultipliedImageData(imageRect));
> Index: WebCore/platform/graphics/filters/FEComposite.cpp
> ===================================================================
> --- WebCore/platform/graphics/filters/FEComposite.cpp	(revision 65160)
> +++ WebCore/platform/graphics/filters/FEComposite.cpp	(working copy)
> @@ -131,26 +131,26 @@ void FEComposite::apply(Filter* filter)
>      FloatRect srcRect = FloatRect(0.f, 0.f, -1.f, -1.f);
>      switch (m_type) {
>      case FECOMPOSITE_OPERATOR_OVER:
> -        filterContext->drawImage(m_in2->resultImage()->image(), DeviceColorSpace, calculateDrawingRect(m_in2->scaledSubRegion()));
> -        filterContext->drawImage(m_in->resultImage()->image(), DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()));
> +        m_in2->resultImage()->draw(filterContext, DeviceColorSpace, calculateDrawingRect(m_in2->scaledSubRegion()));
> +        m_in->resultImage()->draw(filterContext, DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()));
>          break;
>      case FECOMPOSITE_OPERATOR_IN:
>          filterContext->save();
> -        filterContext->clipToImageBuffer(calculateDrawingRect(m_in2->scaledSubRegion()), m_in2->resultImage());
> -        filterContext->drawImage(m_in->resultImage()->image(), DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()));
> +        m_in2->resultImage()->clip(filterContext, calculateDrawingRect(m_in2->scaledSubRegion()));
> +        m_in->resultImage()->draw(filterContext, DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()));
>          filterContext->restore();
>          break;
>      case FECOMPOSITE_OPERATOR_OUT:
> -        filterContext->drawImage(m_in->resultImage()->image(), DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()));
> -        filterContext->drawImage(m_in2->resultImage()->image(), DeviceColorSpace, calculateDrawingRect(m_in2->scaledSubRegion()), srcRect, CompositeDestinationOut);
> +        m_in->resultImage()->draw(filterContext, DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()));
> +        m_in2->resultImage()->draw(filterContext, DeviceColorSpace, calculateDrawingRect(m_in2->scaledSubRegion()), srcRect, CompositeDestinationOut);
>          break;
>      case FECOMPOSITE_OPERATOR_ATOP:
> -        filterContext->drawImage(m_in2->resultImage()->image(), DeviceColorSpace, calculateDrawingRect(m_in2->scaledSubRegion()));
> -        filterContext->drawImage(m_in->resultImage()->image(), DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()), srcRect, CompositeSourceAtop);
> +        m_in2->resultImage()->draw(filterContext, DeviceColorSpace, calculateDrawingRect(m_in2->scaledSubRegion()));
> +        m_in->resultImage()->draw(filterContext, DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()), srcRect, CompositeSourceAtop);
>          break;
>      case FECOMPOSITE_OPERATOR_XOR:
> -        filterContext->drawImage(m_in2->resultImage()->image(), DeviceColorSpace, calculateDrawingRect(m_in2->scaledSubRegion()));
> -        filterContext->drawImage(m_in->resultImage()->image(), DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()), srcRect, CompositeXOR);
> +        m_in2->resultImage()->draw(filterContext, DeviceColorSpace, calculateDrawingRect(m_in2->scaledSubRegion()));
> +        m_in->resultImage()->draw(filterContext, DeviceColorSpace, calculateDrawingRect(m_in->scaledSubRegion()), srcRect, CompositeXOR);
>          break;
>      case FECOMPOSITE_OPERATOR_ARITHMETIC: {
>          IntRect effectADrawingRect = calculateDrawingIntRect(m_in->scaledSubRegion());
> Index: WebCore/platform/graphics/filters/SourceAlpha.cpp
> ===================================================================
> --- WebCore/platform/graphics/filters/SourceAlpha.cpp	(revision 65160)
> +++ WebCore/platform/graphics/filters/SourceAlpha.cpp	(working copy)
> @@ -63,9 +63,9 @@ void SourceAlpha::apply(Filter* filter)
>  
>      setIsAlphaImage(true);
>  
> -    FloatRect imageRect(FloatPoint(), filter->sourceImage()->image()->size());
> +    FloatRect imageRect(FloatPoint(), filter->sourceImage()->size());
>      filterContext->save();
> -    filterContext->clipToImageBuffer(imageRect, filter->sourceImage());
> +    filter->sourceImage()->clip(filterContext, imageRect);
>      filterContext->fillRect(imageRect, Color::black, DeviceColorSpace);
>      filterContext->restore();
>  }
> Index: WebCore/platform/graphics/filters/SourceGraphic.cpp
> ===================================================================
> --- WebCore/platform/graphics/filters/SourceGraphic.cpp	(revision 65160)
> +++ WebCore/platform/graphics/filters/SourceGraphic.cpp	(working copy)
> @@ -60,7 +60,7 @@ void SourceGraphic::apply(Filter* filter
>      if (!filterContext)
>          return;
>  
> -    filterContext->drawImage(filter->sourceImage()->image(), DeviceColorSpace, IntPoint());
> +    filter->sourceImage()->draw(filterContext, DeviceColorSpace, IntPoint());
>  }
>  
>  void SourceGraphic::dump()
> Index: WebCore/platform/graphics/qt/GraphicsContextQt.cpp
> ===================================================================
> --- WebCore/platform/graphics/qt/GraphicsContextQt.cpp	(revision 65160)
> +++ WebCore/platform/graphics/qt/GraphicsContextQt.cpp	(working copy)
> @@ -50,6 +50,7 @@
>  #include "Path.h"
>  #include "Pattern.h"
>  #include "Pen.h"
> +#include "TransparencyLayer.h"
>  
>  #include <QBrush>
>  #include <QDebug>
> @@ -166,48 +167,6 @@ static inline Qt::FillRule toQtFillRule(
>      return Qt::OddEvenFill;
>  }
>  
> -struct TransparencyLayer : FastAllocBase {
> -    TransparencyLayer(const QPainter* p, const QRect &rect, qreal opacity, QPixmap& alphaMask)
> -        : pixmap(rect.width(), rect.height())
> -        , opacity(opacity)
> -        , alphaMask(alphaMask)
> -        , saveCounter(1) // see the comment for saveCounter
> -    {
> -        offset = rect.topLeft();
> -        pixmap.fill(Qt::transparent);
> -        painter.begin(&pixmap);
> -        painter.setRenderHint(QPainter::Antialiasing, p->testRenderHint(QPainter::Antialiasing));
> -        painter.translate(-offset);
> -        painter.setPen(p->pen());
> -        painter.setBrush(p->brush());
> -        painter.setTransform(p->transform(), true);
> -        painter.setOpacity(p->opacity());
> -        painter.setFont(p->font());
> -        if (painter.paintEngine()->hasFeature(QPaintEngine::PorterDuff))
> -            painter.setCompositionMode(p->compositionMode());
> -        // if the path is an empty region, this assignment disables all painting
> -        if (!p->clipPath().isEmpty())
> -            painter.setClipPath(p->clipPath());
> -    }
> -
> -    TransparencyLayer()
> -    {
> -    }
> -
> -    QPixmap pixmap;
> -    QPoint offset;
> -    QPainter painter;
> -    qreal opacity;
> -    // for clipToImageBuffer
> -    QPixmap alphaMask;
> -    // saveCounter is only used in combination with alphaMask
> -    // otherwise, its value is unspecified
> -    int saveCounter;
> -private:
> -    TransparencyLayer(const TransparencyLayer &) {}
> -    TransparencyLayer & operator=(const TransparencyLayer &) { return *this; }
> -};
> -
>  class GraphicsContextPlatformPrivate : public Noncopyable {
>  public:
>      GraphicsContextPlatformPrivate(QPainter* painter);
> @@ -1226,23 +1185,6 @@ void GraphicsContext::clipOutEllipseInRe
>      }
>  }
>  
> -void GraphicsContext::clipToImageBuffer(const FloatRect& floatRect, const ImageBuffer* image)
> -{
> -    if (paintingDisabled())
> -        return;
> -
> -    QPixmap* nativeImage = image->image()->nativeImageForCurrentFrame();
> -    if (!nativeImage)
> -        return;
> -
> -    IntRect rect(floatRect);
> -    QPixmap alphaMask = *nativeImage;
> -    if (alphaMask.width() != rect.width() || alphaMask.height() != rect.height())
> -        alphaMask = alphaMask.scaled(rect.width(), rect.height());
> -
> -    m_data->layers.push(new TransparencyLayer(m_data->p(), m_data->p()->transform().mapRect(rect), 1.0, alphaMask));
> -}
> -
>  void GraphicsContext::addInnerRoundedRectClip(const IntRect& rect,
>                                                int thickness)
>  {
> Index: WebCore/platform/graphics/qt/ImageBufferData.h
> ===================================================================
> --- WebCore/platform/graphics/qt/ImageBufferData.h	(revision 65160)
> +++ WebCore/platform/graphics/qt/ImageBufferData.h	(working copy)
> @@ -26,6 +26,9 @@
>  #ifndef ImageBufferData_h
>  #define ImageBufferData_h
>  
> +#include <wtf/RefPtr.h>
> +#include "Image.h"
> +
>  #include <QPainter>
>  #include <QPixmap>
>  
> @@ -41,6 +44,7 @@ public:
>  
>      QPixmap m_pixmap;
>      OwnPtr<QPainter> m_painter;
> +    RefPtr<Image> m_image;
>  };
>  
>  }  // namespace WebCore
> Index: WebCore/platform/graphics/qt/ImageBufferQt.cpp
> ===================================================================
> --- WebCore/platform/graphics/qt/ImageBufferQt.cpp	(revision 65160)
> +++ WebCore/platform/graphics/qt/ImageBufferQt.cpp	(working copy)
> @@ -33,6 +33,7 @@
>  #include "ImageData.h"
>  #include "MIMETypeRegistry.h"
>  #include "StillImageQt.h"
> +#include "TransparencyLayer.h"
>  #include <wtf/text/CString.h>
>  
>  #include <QBuffer>
> @@ -74,6 +75,8 @@ ImageBufferData::ImageBufferData(const I
>      brush.setColor(Qt::black);
>      painter->setBrush(brush);
>      painter->setCompositionMode(QPainter::CompositionMode_SourceOver);
> +    
> +    m_data.m_image = StillImage::createForRendering(&m_data.m_pixmap);
>  }
>  
>  ImageBuffer::ImageBuffer(const IntSize& size, ImageColorSpace, bool& success)
> @@ -98,24 +101,38 @@ GraphicsContext* ImageBuffer::context() 
>      return m_context.get();
>  }
>  
> -Image* ImageBuffer::imageForRendering() const
> +PassRefPtr<Image> ImageBuffer::copiedImage() const
>  {
> -    if (!m_image)
> -        m_image = StillImage::createForRendering(&m_data.m_pixmap);
> +    return StillImage::create(m_data.m_pixmap);
> +}
>  
> -    return m_image.get();
> +void ImageBuffer::draw(GraphicsContext* context, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect,
> +                       CompositeOperator op, bool useLowQualityScale)
> +{
> +    context->drawImage(m_data.m_image.get(), styleColorSpace, destRect, srcRect, op, useLowQualityScale);
>  }
>  
> -Image* ImageBuffer::image() const
> +void ImageBuffer::drawPattern(GraphicsContext* context, const FloatRect& srcRect, const AffineTransform& patternTransform,
> +                              const FloatPoint& phase, ColorSpace styleColorSpace, CompositeOperator op, const FloatRect& destRect)
>  {
> -    if (!m_image) {
> -        // It's assumed that if image() is called, the actual rendering to the
> -        // GraphicsContext must be done.
> -        ASSERT(context());
> -        m_image = StillImage::create(m_data.m_pixmap);
> -    }
> +    m_data.m_image->drawPattern(context, srcRect, patternTransform, phase, styleColorSpace, op, destRect);
> +}
> +
> +void ImageBuffer::clip(GraphicsContext* context, const FloatRect& floatRect)
> +{
> +    if (context->paintingDisabled())
> +        return;
> +
> +    QPixmap* nativeImage = m_data.m_image->nativeImageForCurrentFrame();
> +    if (!nativeImage)
> +        return;
> +
> +    IntRect rect(floatRect);
> +    QPixmap alphaMask = *nativeImage;
> +    if (alphaMask.width() != rect.width() || alphaMask.height() != rect.height())
> +        alphaMask = alphaMask.scaled(rect.width(), rect.height());
>  
> -    return m_image.get();
> +    m_data->layers.push(new TransparencyLayer(m_data->p(), m_data->p()->transform().mapRect(rect), 1.0, alphaMask));
>  }
>  
>  void ImageBuffer::platformTransformColorSpace(const Vector<int>& lookUpTable)
> Index: WebCore/platform/graphics/qt/TransparencyLayer.h
> ===================================================================
> --- WebCore/platform/graphics/qt/TransparencyLayer.h	(revision 0)
> +++ WebCore/platform/graphics/qt/TransparencyLayer.h	(revision 0)
> @@ -0,0 +1,97 @@
> +/*
> + * Copyright (C) 2006 Dirk Mueller <[e-mail]>
> + * Copyright (C) 2006 Zack Rusin <[e-mail]>
> + * Copyright (C) 2006 George Staikos <[e-mail]>
> + * Copyright (C) 2006 Simon Hausmann <[e-mail]>
> + * Copyright (C) 2006 Allan Sandfeld Jensen <[e-mail]>
> + * Copyright (C) 2006 Nikolas Zimmermann <[e-mail]>
> + * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
> + * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies).
> + * Copyright (C) 2008 Dirk Schulze <[e-mail]>
> + *
> + * 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 COMPUTER, INC. ``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 COMPUTER, INC. OR
> + * 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 TransparencyLayer_h
> +#define TransparencyLayer_h
> +
> +#include <QBrush>
> +#include <QDebug>
> +#include <QGradient>
> +#include <QPaintDevice>
> +#include <QPaintEngine>
> +#include <QPainter>
> +#include <QPainterPath>
> +#include <QPixmap>
> +#include <QPolygonF>
> +#include <QStack>
> +#include <QVector>
> +
> +namespace WebCore {
> +
> +struct TransparencyLayer : FastAllocBase {
> +    TransparencyLayer(const QPainter* p, const QRect &rect, qreal opacity, QPixmap& alphaMask)
> +        : pixmap(rect.width(), rect.height())
> +        , opacity(opacity)
> +        , alphaMask(alphaMask)
> +        , saveCounter(1) // see the comment for saveCounter
> +    {
> +        offset = rect.topLeft();
> +        pixmap.fill(Qt::transparent);
> +        painter.begin(&pixmap);
> +        painter.setRenderHint(QPainter::Antialiasing, p->testRenderHint(QPainter::Antialiasing));
> +        painter.translate(-offset);
> +        painter.setPen(p->pen());
> +        painter.setBrush(p->brush());
> +        painter.setTransform(p->transform(), true);
> +        painter.setOpacity(p->opacity());
> +        painter.setFont(p->font());
> +        if (painter.paintEngine()->hasFeature(QPaintEngine::PorterDuff))
> +            painter.setCompositionMode(p->compositionMode());
> +        // if the path is an empty region, this assignment disables all painting
> +        if (!p->clipPath().isEmpty())
> +            painter.setClipPath(p->clipPath());
> +    }
> +
> +    TransparencyLayer()
> +    {
> +    }
> +
> +    QPixmap pixmap;
> +    QPoint offset;
> +    QPainter painter;
> +    qreal opacity;
> +    // for clipToImageBuffer
> +    QPixmap alphaMask;
> +    // saveCounter is only used in combination with alphaMask
> +    // otherwise, its value is unspecified
> +    int saveCounter;
> +private:
> +    TransparencyLayer(const TransparencyLayer &) {}
> +    TransparencyLayer & operator=(const TransparencyLayer &) { return *this; }
> +};
> +
> +} // namespace WebCore
> +
> +#endif // TransparencyLayer_h
> Index: WebCore/platform/graphics/skia/GraphicsContextSkia.cpp
> ===================================================================
> --- WebCore/platform/graphics/skia/GraphicsContextSkia.cpp	(revision 65160)
> +++ WebCore/platform/graphics/skia/GraphicsContextSkia.cpp	(working copy)
> @@ -455,17 +455,6 @@ void GraphicsContext::clipPath(WindRule 
>      platformContext()->clipPathAntiAliased(path);
>  }
>  
> -void GraphicsContext::clipToImageBuffer(const FloatRect& rect,
> -                                        const ImageBuffer* imageBuffer)
> -{
> -    if (paintingDisabled())
> -        return;
> -
> -#if OS(LINUX) || OS(WINDOWS)
> -    platformContext()->beginLayerClippedToImage(rect, imageBuffer);
> -#endif
> -}
> -
>  void GraphicsContext::concatCTM(const AffineTransform& affine)
>  {
>      if (paintingDisabled())
> Index: WebCore/platform/graphics/skia/ImageBufferSkia.cpp
> ===================================================================
> --- WebCore/platform/graphics/skia/ImageBufferSkia.cpp	(revision 65160)
> +++ WebCore/platform/graphics/skia/ImageBufferSkia.cpp	(working copy)
> @@ -87,20 +87,19 @@ GraphicsContext* ImageBuffer::context() 
>      return m_context.get();
>  }
>  
> -Image* ImageBuffer::image() const
> +PassRefPtr<Image> ImageBuffer::copiedImage() const
>  {
> -    if (!m_image) {
> -        // This creates a COPY of the image and will cache that copy. This means
> -        // that if subsequent operations take place on the context, neither the
> -        // currently-returned image, nor the results of future image() calls,
> -        // will contain that operation.
> -        //
> -        // This seems silly, but is the way the CG port works: image() is
> -        // intended to be used only when rendering is "complete."
> -        m_image = BitmapImageSingleFrameSkia::create(
> -            *m_data.m_platformContext.bitmap());
> -    }
> -    return m_image.get();
> +    return BitmapImageSingleFrameSkia::create(*m_data.m_platformContext.bitmap());
> +}
> +
> +void ImageBuffer::clip(GraphicsContext* context, const FloatRect& rect) const
> +{
> +    if (context->paintingDisabled())
> +        return;
> +
> +#if OS(LINUX) || OS(WINDOWS)
> +    context->platformContext()->beginLayerClippedToImage(rect, this);
> +#endif
>  }
>  
>  void ImageBuffer::platformTransformColorSpace(const Vector<int>& lookUpTable)
> Index: WebCore/platform/graphics/wx/GraphicsContextWx.cpp
> ===================================================================
> --- WebCore/platform/graphics/wx/GraphicsContextWx.cpp	(revision 65160)
> +++ WebCore/platform/graphics/wx/GraphicsContextWx.cpp	(working copy)
> @@ -378,11 +378,6 @@ void GraphicsContext::canvasClip(const P
>      clip(path);
>  }
>  
> -void GraphicsContext::clipToImageBuffer(const FloatRect&, const ImageBuffer*)
> -{
> -    notImplemented();
> -}
> -
>  AffineTransform GraphicsContext::getCTM() const
>  { 
>  #if USE(WXGC)
> Index: WebCore/platform/graphics/wx/ImageBufferWx.cpp
> ===================================================================
> --- WebCore/platform/graphics/wx/ImageBufferWx.cpp	(revision 65160)
> +++ WebCore/platform/graphics/wx/ImageBufferWx.cpp	(working copy)
> @@ -82,12 +82,17 @@ String ImageBuffer::toDataURL(const Stri
>      return String();
>  }
>  
> -Image* ImageBuffer::image() const
> +PassRefPtr<Image> ImageBuffer::copiedImage() const
>  {
>      notImplemented();
>      return 0;
>  }
>  
> +void ImageBuffer::clip(GraphicsContext*, const FloatRect&) const
> +{
> +    notImplemented();
> +}
> +
>  void ImageBuffer::platformTransformColorSpace(const Vector<int>&)
>  {
>      notImplemented();
> Index: WebCore/platform/mac/ScrollbarThemeMac.mm
> ===================================================================
> --- WebCore/platform/mac/ScrollbarThemeMac.mm	(revision 65160)
> +++ WebCore/platform/mac/ScrollbarThemeMac.mm	(working copy)
> @@ -396,7 +396,7 @@ bool ScrollbarThemeMac::paint(Scrollbar*
>              return true;
>          
>          HIThemeDrawTrack(&trackInfo, 0, imageBuffer->context()->platformContext(), kHIThemeOrientationNormal);
> -        context->drawImage(imageBuffer->image(), DeviceColorSpace, scrollbar->frameRect().location());
> +        imageBuffer->draw(context, DeviceColorSpace, scrollbar->frameRect().location());
>      }
>  
>      return true;
> Index: WebCore/rendering/RenderBoxModelObject.cpp
> ===================================================================
> --- WebCore/rendering/RenderBoxModelObject.cpp	(revision 65160)
> +++ WebCore/rendering/RenderBoxModelObject.cpp	(working copy)
> @@ -531,7 +531,7 @@ void RenderBoxModelObject::paintFillLaye
>          
>          // The mask has been created.  Now we just need to clip to it.
>          context->save();
> -        context->clipToImageBuffer(maskRect, maskImage.get());
> +        maskImage->clip(context, maskRect);
>      }
>      
>      StyleImage* bg = bgLayer->image();
> Index: WebCore/rendering/RenderSVGResourceClipper.cpp
> ===================================================================
> --- WebCore/rendering/RenderSVGResourceClipper.cpp	(revision 65160)
> +++ WebCore/rendering/RenderSVGResourceClipper.cpp	(working copy)
> @@ -174,7 +174,7 @@ bool RenderSVGResourceClipper::applyClip
>      if (!clipperData->clipMaskImage)
>          return false;
>  
> -    context->clipToImageBuffer(repaintRect, clipperData->clipMaskImage.get());
> +    clipperData->clipMaskImage->clip(context, repaintRect);
>      return true;
>  }
>  
> Index: WebCore/rendering/RenderSVGResourceFilter.cpp
> ===================================================================
> --- WebCore/rendering/RenderSVGResourceFilter.cpp	(revision 65160)
> +++ WebCore/rendering/RenderSVGResourceFilter.cpp	(working copy)
> @@ -271,7 +271,7 @@ void RenderSVGResourceFilter::postApplyR
>  #if !PLATFORM(CG)
>              resultImage->transformColorSpace(LinearRGB, DeviceRGB);
>  #endif
> -            context->drawImage(resultImage->image(), object->style()->colorSpace(), lastEffect->subRegion());
> +            resultImage->draw(context, object->style()->colorSpace(), lastEffect->subRegion());
>          }
>      }
>  
> Index: WebCore/rendering/RenderSVGResourceGradient.cpp
> ===================================================================
> --- WebCore/rendering/RenderSVGResourceGradient.cpp	(revision 65160)
> +++ WebCore/rendering/RenderSVGResourceGradient.cpp	(working copy)
> @@ -123,7 +123,7 @@ static inline AffineTransform clipToText
>      // So the actual masking process has to be done in the device coordinate space as well.
>      AffineTransform transform(absoluteTransformFromContext(context));
>      context->concatCTM(transform.inverse());
> -    context->clipToImageBuffer(transform.mapRect(textRootBlock->repaintRectInLocalCoordinates()), imageBuffer.get());
> +    imageBuffer->clip(context, transform.mapRect(textRootBlock->repaintRectInLocalCoordinates()));
>      context->concatCTM(transform);
>  
>      AffineTransform matrix;
> Index: WebCore/rendering/RenderSVGResourceMasker.cpp
> ===================================================================
> --- WebCore/rendering/RenderSVGResourceMasker.cpp	(revision 65160)
> +++ WebCore/rendering/RenderSVGResourceMasker.cpp	(working copy)
> @@ -105,7 +105,7 @@ bool RenderSVGResourceMasker::applyResou
>      if (!maskerData->maskImage)
>          return false;
>  
> -    context->clipToImageBuffer(maskerData->maskRect, maskerData->maskImage.get());
> +    maskerData->maskImage->clip(context, maskerData->maskRect);
>      return true;
>  }
>  
> Index: WebCore/rendering/RenderSVGResourcePattern.cpp
> ===================================================================
> --- WebCore/rendering/RenderSVGResourcePattern.cpp	(revision 65160)
> +++ WebCore/rendering/RenderSVGResourcePattern.cpp	(working copy)
> @@ -298,14 +298,15 @@ PassOwnPtr<ImageBuffer> RenderSVGResourc
>  
>  void RenderSVGResourcePattern::buildPattern(PatternData* patternData, PassOwnPtr<ImageBuffer> tileImage) const
>  {
> -    if (!tileImage->image()) {
> +    RefPtr<Image> copiedImage = tileImage->copiedImage();
> +    if (!copiedImage) {
>          patternData->pattern = 0;
>          return;
>      }
> -
> -    IntRect tileRect = tileImage->image()->rect();
> +    
> +    IntRect tileRect = copiedImage->rect();
>      if (tileRect.width() <= patternData->boundaries.width() && tileRect.height() <= patternData->boundaries.height()) {
> -        patternData->pattern = Pattern::create(tileImage->image(), true, true);
> +        patternData->pattern = Pattern::create(copiedImage, true, true);
>          return;
>      }
>  
> @@ -331,13 +332,13 @@ void RenderSVGResourcePattern::buildPatt
>          newTileImageContext->translate(0, patternData->boundaries.height());
>          for (int j = numX; j > 0; --j) {
>              newTileImageContext->translate(patternData->boundaries.width(), 0);
> -            newTileImageContext->drawImage(tileImage->image(), style()->colorSpace(), tileRect, tileRect);
> +            newTileImageContext->drawImage(copiedImage.get(), style()->colorSpace(), tileRect, tileRect);
>          }
>          newTileImageContext->translate(-patternData->boundaries.width() * numX, 0);
>      }
>      newTileImageContext->restore();
>  
> -    patternData->pattern = Pattern::create(newTileImage->image(), true, true);
> +    patternData->pattern = Pattern::create(newTileImage->copiedImage(), true, true);
>  }
>  
>  }
> Index: WebCore/rendering/RenderThemeMac.mm
> ===================================================================
> --- WebCore/rendering/RenderThemeMac.mm	(revision 65160)
> +++ WebCore/rendering/RenderThemeMac.mm	(working copy)
> @@ -963,7 +963,8 @@ bool RenderThemeMac::paintProgressBar(Re
>          paintInfo.context->translate(2 * rect.x() + rect.width(), 0);
>          paintInfo.context->scale(FloatSize(-1, 1));
>      }
> -    paintInfo.context->drawImage(imageBuffer->image(), DeviceColorSpace, rect.location());
> +    
> +    imageBuffer->draw(paintInfo.context, DeviceColorSpace, rect.location());
>  
>      paintInfo.context->restore();
>      return false;
> Index: WebCore/svg/SVGFEImageElement.cpp
> ===================================================================
> --- WebCore/svg/SVGFEImageElement.cpp	(revision 65160)
> +++ WebCore/svg/SVGFEImageElement.cpp	(working copy)
> @@ -137,7 +137,7 @@ PassRefPtr<FilterEffect> SVGFEImageEleme
>          SVGRenderSupport::renderSubtreeToImage(m_targetImage.get(), renderer);
>      }
>  
> -    return FEImage::create(m_targetImage ? m_targetImage->image() : m_cachedImage->image(), preserveAspectRatio());
> +    return FEImage::create(m_targetImage ? m_targetImage->copiedImage() : m_cachedImage->image(), preserveAspectRatio());
>  }
>  
>  void SVGFEImageElement::addSubresourceAttributeURLs(ListHashSet<KURL>& urls) const
> Index: WebCore/svg/graphics/SVGImage.cpp
> ===================================================================
> --- WebCore/svg/graphics/SVGImage.cpp	(revision 65160)
> +++ WebCore/svg/graphics/SVGImage.cpp	(working copy)
> @@ -221,12 +221,13 @@ NativeImagePtr SVGImage::nativeImageForC
>      if (!m_frameCache) {
>          if (!m_page)
>              return 0;
> -        m_frameCache = ImageBuffer::create(size());
> -        if (!m_frameCache) // failed to allocate image
> +        OwnPtr<ImageBuffer> imageBuffer = ImageBuffer::create(size());
> +        if (!imageBuffer) // failed to allocate image
>              return 0;
> -        draw(m_frameCache->context(), rect(), rect(), DeviceColorSpace, CompositeSourceOver);
> +        draw(imageBuffer->context(), rect(), rect(), DeviceColorSpace, CompositeSourceOver);
> +        m_frameCache = imageBuffer->copiedImage();
>      }
> -    return m_frameCache->image()->nativeImageForCurrentFrame();
> +    return m_frameCache->nativeImageForCurrentFrame(); 
>  }
>  
>  bool SVGImage::dataChanged(bool allDataReceived)
> Index: WebCore/svg/graphics/SVGImage.h
> ===================================================================
> --- WebCore/svg/graphics/SVGImage.h	(revision 65160)
> +++ WebCore/svg/graphics/SVGImage.h	(working copy)
> @@ -72,7 +72,7 @@ namespace WebCore {
>          
>          OwnPtr<SVGImageChromeClient> m_chromeClient;
>          OwnPtr<Page> m_page;
> -        OwnPtr<ImageBuffer> m_frameCache;
> +        RefPtr<Image> m_frameCache;
>      };
>  }
>  
> Index: WebCore/svg/graphics/filters/SVGFEMerge.cpp
> ===================================================================
> --- WebCore/svg/graphics/filters/SVGFEMerge.cpp	(revision 65160)
> +++ WebCore/svg/graphics/filters/SVGFEMerge.cpp	(working copy)
> @@ -79,7 +79,7 @@ void FEMerge::apply(Filter* filter)
>  
>      for (unsigned i = 0; i < m_mergeInputs.size(); i++) {
>          FloatRect destRect = calculateDrawingRect(m_mergeInputs[i]->scaledSubRegion());
> -        filterContext->drawImage(m_mergeInputs[i]->resultImage()->image(), DeviceColorSpace, destRect);
> +        m_mergeInputs[i]->resultImage()->draw(filterContext, DeviceColorSpace, destRect);
>      }
>  }
>  
> Index: WebCore/svg/graphics/filters/SVGFEOffset.cpp
> ===================================================================
> --- WebCore/svg/graphics/filters/SVGFEOffset.cpp	(revision 65160)
> +++ WebCore/svg/graphics/filters/SVGFEOffset.cpp	(working copy)
> @@ -91,7 +91,7 @@ void FEOffset::apply(Filter* filter)
>                                    m_in->scaledSubRegion().width(),
>                                    m_in->scaledSubRegion().height());
>  
> -    filterContext->drawImage(m_in->resultImage()->image(), DeviceColorSpace, dstRect);
> +    m_in->resultImage()->draw(filterContext, DeviceColorSpace, dstRect);
>  }
>  
>  void FEOffset::dump()
> Index: WebCore/svg/graphics/filters/SVGFETile.cpp
> ===================================================================
> --- WebCore/svg/graphics/filters/SVGFETile.cpp	(revision 65160)
> +++ WebCore/svg/graphics/filters/SVGFETile.cpp	(working copy)
> @@ -72,8 +72,8 @@ void FETile::apply(Filter* filter)
>  
>      OwnPtr<ImageBuffer> tileImage = ImageBuffer::create(tileRect.size());
>      GraphicsContext* tileImageContext = tileImage->context();
> -    tileImageContext->drawImage(m_in->resultImage()->image(), DeviceColorSpace, IntPoint());
> -    RefPtr<Pattern> pattern = Pattern::create(tileImage->image(), true, true);
> +    m_in->resultImage()->draw(tileImageContext, DeviceColorSpace, IntPoint());
> +    RefPtr<Pattern> pattern = Pattern::create(tileImage->copiedImage(), true, true);
>  
>      AffineTransform matrix;
>      matrix.translate(m_in->scaledSubRegion().x() - scaledSubRegion().x(), m_in->scaledSubRegion().y() - scaledSubRegion().y());

A few drive-by comments (the ImageBuffer::draw stuff will be helpful BTW, thanks):

WebCore/css/CSSCanvasValue.cpp:97
 +      return copiedImage.get(); 
This looks like it would return a bare ptr whose Image was freed at the end of this function.  Am I missing something?

WebCore/html/canvas/WebGLRenderingContext.cpp: 
 +          canvas()->buffer()->clearImage();
Could the FIXME above be removed now, too?

[Dave Hyatt]

Yeah I noticed and yanked that FIXME.

You're right about CSSCanvasValue... I'll have to patch a bunch more to propagate a PassRefPtr up.  Unfortunately that's a virtual function, so I have to change many classes. :)

[Eric Seidel (no email)]

I'm (selfishly) curious how much of an improvement this patch gives us on the "benchmarks" posted at http://www.themaninblue.com/writing/perspective/2010/03/22/

[Dave Hyatt]

Created attachment 64370 [details]
New patch - incorporate suggestions

[Dave Hyatt]

(In reply to comment #12)
> I'm (selfishly) curious how much of an improvement this patch gives us on the "benchmarks" posted at http://www.themaninblue.com/writing/perspective/2010/03/22/

http://themaninblue.com/experiment/AnimationBenchmark/canvas/ goes from 37fps to 85fps on my machine.

http://themaninblue.com/experiment/AnimationBenchmark/canvas/?particles=1000 goes from 25fps to 43fps on my machine.

http://themaninblue.com/experiment/AnimationBenchmark/canvas/?particles=500&shadows=true goes from 18 to 32fps on my machine.

[Dave Hyatt]

Created attachment 64376 [details]
Patch incorporating more feedback

This is getting pretty close, but one WebGL test fails on Mac.  I can't spot the error.

[Stephen White]

Comment on attachment 64376 [details]
Patch incorporating more feedback

WebCore/platform/graphics/GraphicsContext.h:34
 +  #include "ImageBuffer.h"
Due to some nasty cycles between ImageBufferData.h and PlatformContextSkia.h, making this an include rather than a forward-declare breaks the build on chromium/win.  There may be a way to fix it on the skia side, but if you can avoid changing this it'd be great.

[Stephen White]

Comment on attachment 64376 [details]
Patch incorporating more feedback

WebCore/platform/graphics/skia/ImageBufferSkia.cpp:108
 +                         CompositeOperator op = CompositeSourceOver, bool useLowQualityScale = false)
Default args in function definition (the EWS bot would eventually tell you, I'm just trying to help out while they're MIA).

[Adam Roben (:aroben)]

(In reply to comment #4)
> I did a very similar patch locally and ended up keeping the caching of the shallow copy and only returning a PassRefPtr for the deep copy.  My concern is whether or not CG is really doing copy-on-write or not.  If they aren't, then removing the cache in the shallow copy case is going to be a perf hit.
> 
> Let me ask some CG engineers about it.

Dave, did you find out the answer to this question?

[Dave Hyatt]

Created attachment 64502 [details]
Patch

[Dave Hyatt]

(In reply to comment #18)
> (In reply to comment #4)
> > I did a very similar patch locally and ended up keeping the caching of the shallow copy and only returning a PassRefPtr for the deep copy.  My concern is whether or not CG is really doing copy-on-write or not.  If they aren't, then removing the cache in the shallow copy case is going to be a perf hit.
> > 
> > Let me ask some CG engineers about it.
> 
> Dave, did you find out the answer to this question?

Yeah, it always copies, so it's a function that should be avoided whenever that isn't the desired behavior.

[Dave Hyatt]

Created attachment 64505 [details]
Patch for real review

Ok this one is for real.  Ready for official review.

[Stephen White]

Comment on attachment 64505 [details]
Patch for real review

I'm not a reviewer, but I think this will probably still break a few builds:

WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:119
 +      RefPtr<Image> imageCopy = copiedImage();
Should be copyImage().

WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:126
 +      RefPtr<Image> imageCopy = copiedImage();
Ibid.

WebCore/platform/graphics/qt/ImageBufferQt.cpp:119
 +          RefPtr<Image> copy = copiedImage();
Ibid.

WebCore/platform/graphics/qt/ImageBufferQt.cpp:130
 +          RefPtr<Image> copy = copiedImage();
Ibid.

WebCore/platform/graphics/skia/ImageBufferSkia.cpp:110
 +      RefPtr<Image> imageCopy = copiedImage();
Ibid.

WebCore/platform/graphics/skia/ImageBufferSkia.cpp:117
 +      RefPtr<Image> imageCopy = copiedImage();
Ibid.

WebCore/platform/graphics/wx/ImageBufferWx.cpp:105
 +      RefPtr<Image> imageCopy = copiedImage();
Ibid.

WebCore/platform/graphics/wx/ImageBufferWx.cpp:112
 +      RefPtr<Image> imageCopy = copiedImage();
Ibid.

Other than that, looks great to me!  Excited to see this change go in.

[Dave Hyatt]

Created attachment 64512 [details]
ToT changed some things, had to resync.

[Dave Hyatt]

Created attachment 64515 [details]
Argh, project file changed again.

Trying to keep this current so I can get some build info from other platforms before landing.

[WebKit Review Bot]

Attachment 64515 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/graphics/qt/ImageBufferData.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/platform/graphics/cg/ImageBufferData.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 42 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dave Hyatt]

Landed in r65449.

[WebKit Review Bot]

http://trac.webkit.org/changeset/65449 might have broken SnowLeopard Intel Release (Tests)

[WebKit Review Bot]

Attachment 64515 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3721246