Account for transform in SVG background images
authorpdr@google.com <pdr@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Feb 2013 01:19:25 +0000 (01:19 +0000)
committerpdr@google.com <pdr@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Feb 2013 01:19:25 +0000 (01:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=110295

Reviewed by Dirk Schulze.

Source/WebCore:

Tiled SVG background images are rendererd by drawing the SVG content into a temporary
image buffer, then stamping out a tiled pattern using this buffer. Previously the
image buffer did not account for CSS transforms which could result in pixelated backgrounds.

This patch takes advantage of the context's transform when sizing the temporary tiling
image buffer. Because the context's transform also includes scale, this patch simplifies
the SVG image code to no longer track scale.

Test: svg/as-background-image/svg-transformed-background.html

* loader/cache/CachedImage.cpp:
(WebCore):
(WebCore::CachedImage::imageForRenderer):

    CachedImage::lookupOrCreateImageForRenderer no longer creates images so it has been
    refactored into just "imageForRenderer". Previously there were two versions of
    lookupOrCreateImageForRenderer; these have been folded into imageForRenderer.

* loader/cache/CachedImage.h:
* svg/graphics/SVGImage.cpp:
(WebCore::SVGImage::drawPatternForContainer):

    To create the temporary tiling image buffer, the final size in screen coordinates is
    needed. This is now computed using the current context's CTM. Because the CTM
    already includes the page scale, all page scale tracking can be removed.

    The adjustments to srcRect and the pattern transform are the same as before, just
    refactored to use imageBufferScale which has x and y components.

* svg/graphics/SVGImage.h:
* svg/graphics/SVGImageCache.cpp:
(WebCore::SVGImageCache::setContainerSizeForRenderer):

    Because the page scale needed to be cached between calls to
    setContainerSizeForRenderer, this function was written to modify an existing cache
    entry. Because the page scale no longer needs to be tracked, this code has been
    simplified to re-write any existing cache entry.

(WebCore::SVGImageCache::imageSizeForRenderer):

    This function has been simplified by calling SVGImageForContainer::size() instead
    of computing this value manually. The value returned remains the same, containing
    the container size multiplied by zoom.

(WebCore::SVGImageCache::imageForRenderer):

    Previously we set the page scale on every call to imageForRenderer. Because page scale
    no longer needs to be tracked, this function has been simplified to simply return
    the cached SVGImageForContainer.

* svg/graphics/SVGImageForContainer.cpp:
(WebCore::SVGImageForContainer::drawPattern):
* svg/graphics/SVGImageForContainer.h:
(WebCore::SVGImageForContainer::create):
(WebCore::SVGImageForContainer::SVGImageForContainer):
(SVGImageForContainer):

LayoutTests:

* svg/as-background-image/svg-transformed-background-expected.html: Added.
* svg/as-background-image/svg-transformed-background.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/as-background-image/svg-transformed-background-expected.html [new file with mode: 0644]
LayoutTests/svg/as-background-image/svg-transformed-background.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedImage.cpp
Source/WebCore/loader/cache/CachedImage.h
Source/WebCore/svg/graphics/SVGImage.cpp
Source/WebCore/svg/graphics/SVGImage.h
Source/WebCore/svg/graphics/SVGImageCache.cpp
Source/WebCore/svg/graphics/SVGImageForContainer.cpp
Source/WebCore/svg/graphics/SVGImageForContainer.h

index c83eaa7..51ad22d 100644 (file)
@@ -1,3 +1,13 @@
+2013-02-20  Philip Rogers  <pdr@google.com>
+
+        Account for transform in SVG background images
+        https://bugs.webkit.org/show_bug.cgi?id=110295
+
+        Reviewed by Dirk Schulze.
+
+        * svg/as-background-image/svg-transformed-background-expected.html: Added.
+        * svg/as-background-image/svg-transformed-background.html: Added.
+
 2013-02-20  Ojan Vafai  <ojan@chromium.org>
 
         Positioned, replaced elements with intrinsic width keywords compute the wrong width
diff --git a/LayoutTests/svg/as-background-image/svg-transformed-background-expected.html b/LayoutTests/svg/as-background-image/svg-transformed-background-expected.html
new file mode 100644 (file)
index 0000000..e2597a2
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+#circles {
+  position: absolute;
+  left: 10px;
+  top: 40px;
+  width: 400px;
+  height: 100px;
+  background-image:url("data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' width='100' height='100'><circle cx='50' cy='50' r='50' fill='green' /></svg>");
+}
+</style>
+</head>
+<body>
+Test for WK110295: This test passes if there are four green circles with sharp edges.<br/>
+<div id="circles"></div>
+</body>
+</html>
+
diff --git a/LayoutTests/svg/as-background-image/svg-transformed-background.html b/LayoutTests/svg/as-background-image/svg-transformed-background.html
new file mode 100644 (file)
index 0000000..6d683c3
--- /dev/null
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+#circle {
+  position: absolute;
+  left: 10px;
+  top: 40px;
+  width: 40px;
+  height: 20px;
+  background-image:url("data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' width='20' height='20'><circle cx='10' cy='10' r='10' fill='green' /></svg>");
+  transform:scale(5);
+  transform-origin:0 0;
+  -webkit-transform:scale(5);
+  -webkit-transform-origin:0 0;
+}
+#ellipse {
+  position: absolute;
+  left: 210px;
+  top: 40px;
+  width: 80px;
+  height: 10px;
+  background-image:url("data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' width='40' height='10'><ellipse cx='20' cy='5' rx='20' ry='5' fill='green' /></svg>");
+  transform:scale(2.5, 10);
+  transform-origin:0 0;
+  -webkit-transform:scale(2.5, 10);
+  -webkit-transform-origin:0 0;
+}
+</style>
+</head>
+<body>
+Test for WK110295: This test passes if there are four green circles with sharp edges.<br/>
+<div id="circle"></div>
+<div id="ellipse"></div>
+</body>
+</html>
+
index 11e113a..bffffdb 100644 (file)
@@ -1,3 +1,67 @@
+2013-02-20  Philip Rogers  <pdr@google.com>
+
+        Account for transform in SVG background images
+        https://bugs.webkit.org/show_bug.cgi?id=110295
+
+        Reviewed by Dirk Schulze.
+
+        Tiled SVG background images are rendererd by drawing the SVG content into a temporary
+        image buffer, then stamping out a tiled pattern using this buffer. Previously the
+        image buffer did not account for CSS transforms which could result in pixelated backgrounds.
+
+        This patch takes advantage of the context's transform when sizing the temporary tiling
+        image buffer. Because the context's transform also includes scale, this patch simplifies
+        the SVG image code to no longer track scale.
+
+        Test: svg/as-background-image/svg-transformed-background.html
+
+        * loader/cache/CachedImage.cpp:
+        (WebCore):
+        (WebCore::CachedImage::imageForRenderer):
+
+            CachedImage::lookupOrCreateImageForRenderer no longer creates images so it has been
+            refactored into just "imageForRenderer". Previously there were two versions of
+            lookupOrCreateImageForRenderer; these have been folded into imageForRenderer.
+
+        * loader/cache/CachedImage.h:
+        * svg/graphics/SVGImage.cpp:
+        (WebCore::SVGImage::drawPatternForContainer):
+
+            To create the temporary tiling image buffer, the final size in screen coordinates is
+            needed. This is now computed using the current context's CTM. Because the CTM
+            already includes the page scale, all page scale tracking can be removed.
+
+            The adjustments to srcRect and the pattern transform are the same as before, just
+            refactored to use imageBufferScale which has x and y components.
+
+        * svg/graphics/SVGImage.h:
+        * svg/graphics/SVGImageCache.cpp:
+        (WebCore::SVGImageCache::setContainerSizeForRenderer):
+
+            Because the page scale needed to be cached between calls to
+            setContainerSizeForRenderer, this function was written to modify an existing cache
+            entry. Because the page scale no longer needs to be tracked, this code has been
+            simplified to re-write any existing cache entry.
+
+        (WebCore::SVGImageCache::imageSizeForRenderer):
+
+            This function has been simplified by calling SVGImageForContainer::size() instead
+            of computing this value manually. The value returned remains the same, containing
+            the container size multiplied by zoom.
+
+        (WebCore::SVGImageCache::imageForRenderer):
+
+            Previously we set the page scale on every call to imageForRenderer. Because page scale
+            no longer needs to be tracked, this function has been simplified to simply return
+            the cached SVGImageForContainer.
+
+        * svg/graphics/SVGImageForContainer.cpp:
+        (WebCore::SVGImageForContainer::drawPattern):
+        * svg/graphics/SVGImageForContainer.h:
+        (WebCore::SVGImageForContainer::create):
+        (WebCore::SVGImageForContainer::SVGImageForContainer):
+        (SVGImageForContainer):
+
 2013-02-20  Jer Noble  <jer.noble@apple.com>
 
         Crash in com.apple.WebKit2.WebProcessService at com.apple.avfoundation: __73-[AVAssetResourceLoader _attemptDelegateHandlingOfRequestWithDictionary:]_block_invoke + 51
index 2c891e7..b7610be 100644 (file)
@@ -157,25 +157,6 @@ bool CachedImage::willPaintBrokenImage() const
     return errorOccurred() && m_shouldPaintBrokenImage;
 }
 
-#if ENABLE(SVG)
-inline Image* CachedImage::lookupOrCreateImageForRenderer(const RenderObject* renderer)
-{
-    if (!m_image)
-        return 0;
-    if (!m_image->isSVGImage())
-        return m_image.get();
-    Image* useImage = m_svgImageCache->imageForRenderer(renderer);
-    if (useImage == Image::nullImage())
-        return m_image.get();
-    return useImage;
-}
-#else
-inline Image* CachedImage::lookupOrCreateImageForRenderer(const RenderObject*)
-{
-    return m_image.get();
-}
-#endif
-
 Image* CachedImage::image()
 {
     ASSERT(!isPurgeable());
@@ -204,10 +185,19 @@ Image* CachedImage::imageForRenderer(const RenderObject* renderer)
         return brokenImage(1).first;
     }
 
-    if (m_image)
-        return lookupOrCreateImageForRenderer(renderer);
+    if (!m_image)
+        return Image::nullImage();
 
-    return Image::nullImage();
+#if ENABLE(SVG)
+    if (m_image->isSVGImage()) {
+        Image* image = m_svgImageCache->imageForRenderer(renderer);
+        if (image != Image::nullImage())
+            return image;
+    }
+#else
+    UNUSED_PARAM(renderer);
+#endif
+    return m_image.get();
 }
 
 void CachedImage::setContainerSizeForRenderer(const CachedImageClient* renderer, const IntSize& containerSize, float containerZoom)
index 5c08f18..8bc447a 100644 (file)
@@ -97,8 +97,6 @@ public:
     virtual void reportMemoryUsage(MemoryObjectInfo*) const OVERRIDE;
 
 private:
-    Image* lookupOrCreateImageForRenderer(const RenderObject*);
-
     void clear();
 
     void createImage();
index 0ebb703..9ea5f84 100644 (file)
@@ -141,26 +141,30 @@ void SVGImage::drawForContainer(GraphicsContext* context, const FloatSize contai
     setImageObserver(observer);
 }
 
-void SVGImage::drawPatternForContainer(GraphicsContext* context, const FloatSize containerSize, float pageScale, float zoom, const FloatRect& srcRect,
+void SVGImage::drawPatternForContainer(GraphicsContext* context, const FloatSize containerSize, float zoom, const FloatRect& srcRect,
     const AffineTransform& patternTransform, const FloatPoint& phase, ColorSpace colorSpace, CompositeOperator compositeOp, const FloatRect& dstRect)
 {
-    ASSERT(pageScale);
-
     FloatRect zoomedContainerRect = FloatRect(FloatPoint(), containerSize);
     zoomedContainerRect.scale(zoom);
-    FloatRect zoomedAndScaledContainerRect = zoomedContainerRect;
-    zoomedAndScaledContainerRect.scale(pageScale);
 
-    // FIXME(WK110065): This should take advantage of the ImageBuffer resolution instead of scaling the buffer manually.
-    OwnPtr<ImageBuffer> buffer = ImageBuffer::create(expandedIntSize(zoomedAndScaledContainerRect.size()), 1);
-    drawForContainer(buffer->context(), containerSize, zoom, zoomedAndScaledContainerRect, zoomedContainerRect, ColorSpaceDeviceRGB, CompositeSourceOver, BlendModeNormal);
+    // The ImageBuffer size needs to be scaled to match the final resolution.
+    AffineTransform transform = context->getCTM();
+    FloatSize imageBufferScale = FloatSize(transform.xScale(), transform.yScale());
+    ASSERT(imageBufferScale.width());
+    ASSERT(imageBufferScale.height());
+
+    FloatRect imageBufferSize = zoomedContainerRect;
+    imageBufferSize.scale(imageBufferScale.width(), imageBufferScale.height());
+
+    OwnPtr<ImageBuffer> buffer = ImageBuffer::create(expandedIntSize(imageBufferSize.size()), 1);
+    drawForContainer(buffer->context(), containerSize, zoom, imageBufferSize, zoomedContainerRect, ColorSpaceDeviceRGB, CompositeSourceOver, BlendModeNormal);
     RefPtr<Image> image = buffer->copyImage(CopyBackingStore, Unscaled);
 
-    // Adjust the source rect and transform for image buffer scale due to pageScale.
+    // Adjust the source rect and transform due to the image buffer's scaling.
     FloatRect scaledSrcRect = srcRect;
-    scaledSrcRect.scale(pageScale);
+    scaledSrcRect.scale(imageBufferScale.width(), imageBufferScale.height());
     AffineTransform unscaledPatternTransform(patternTransform);
-    unscaledPatternTransform.scale(1 / pageScale);
+    unscaledPatternTransform.scale(1 / imageBufferScale.width(), 1 / imageBufferScale.height());
 
     image->drawPattern(context, scaledSrcRect, unscaledPatternTransform, phase, colorSpace, compositeOp, dstRect);
 }
index 766cfc5..3850b7c 100644 (file)
@@ -89,8 +89,8 @@ private:
     SVGImage(ImageObserver*);
     virtual void draw(GraphicsContext*, const FloatRect& fromRect, const FloatRect& toRect, ColorSpace styleColorSpace, CompositeOperator, BlendMode);
     void drawForContainer(GraphicsContext*, const FloatSize, float, const FloatRect&, const FloatRect&, ColorSpace, CompositeOperator, BlendMode);
-    void drawPatternForContainer(GraphicsContext*, const FloatSize, float, float, const FloatRect&, const AffineTransform&,
-        const FloatPoint&, ColorSpace, CompositeOperator, const FloatRect&);
+    void drawPatternForContainer(GraphicsContext*, const FloatSize, float, const FloatRect&, const AffineTransform&, const FloatPoint&, ColorSpace,
+        CompositeOperator, const FloatRect&);
 
     OwnPtr<SVGImageChromeClient> m_chromeClient;
     OwnPtr<Page> m_page;
index f1e6b8e..63ee611 100644 (file)
@@ -56,18 +56,12 @@ void SVGImageCache::setContainerSizeForRenderer(const CachedImageClient* client,
 {
     ASSERT(client);
     ASSERT(!containerSize.isEmpty());
+    ASSERT(containerZoom);
 
     FloatSize containerSizeWithoutZoom(containerSize);
     containerSizeWithoutZoom.scale(1 / containerZoom);
 
-    ImageForContainerMap::iterator imageIt = m_imageForContainerMap.find(client);
-    if (imageIt == m_imageForContainerMap.end()) {
-        RefPtr<SVGImageForContainer> image = SVGImageForContainer::create(m_svgImage, containerSizeWithoutZoom, 1, containerZoom);
-        m_imageForContainerMap.set(client, image);
-    } else {
-        imageIt->value->setSize(containerSizeWithoutZoom);
-        imageIt->value->setZoom(containerZoom);
-    }
+    m_imageForContainerMap.set(client, SVGImageForContainer::create(m_svgImage, containerSizeWithoutZoom, containerZoom));
 }
 
 IntSize SVGImageCache::imageSizeForRenderer(const RenderObject* renderer) const
@@ -81,14 +75,8 @@ IntSize SVGImageCache::imageSizeForRenderer(const RenderObject* renderer) const
         return imageSize;
 
     RefPtr<SVGImageForContainer> image = it->value;
-    FloatSize size = image->containerSize();
-    if (!size.isEmpty()) {
-        size.scale(image->zoom());
-        imageSize.setWidth(size.width());
-        imageSize.setHeight(size.height());
-    }
-
-    return imageSize;
+    ASSERT(!image->size().isEmpty());
+    return image->size();
 }
 
 // FIXME: This doesn't take into account the animation timeline so animations will not
@@ -98,18 +86,12 @@ Image* SVGImageCache::imageForRenderer(const RenderObject* renderer)
     if (!renderer)
         return Image::nullImage();
 
-    // The cache needs to know the size of the renderer before querying an image for it.
     ImageForContainerMap::iterator it = m_imageForContainerMap.find(renderer);
     if (it == m_imageForContainerMap.end())
         return Image::nullImage();
 
     RefPtr<SVGImageForContainer> image = it->value;
-    ASSERT(!image->containerSize().isEmpty());
-
-    // FIXME: Set the page scale in setContainerSizeForRenderer instead of looking it up here.
-    Page* page = renderer->document()->page();
-    image->setPageScale(page->deviceScaleFactor() * page->pageScaleFactor());
-
+    ASSERT(!image->size().isEmpty());
     return image.get();
 }
 
index 485c49f..d076fea 100644 (file)
@@ -45,7 +45,7 @@ void SVGImageForContainer::draw(GraphicsContext* context, const FloatRect& dstRe
 void SVGImageForContainer::drawPattern(GraphicsContext* context, const FloatRect& srcRect, const AffineTransform& patternTransform,
     const FloatPoint& phase, ColorSpace colorSpace, CompositeOperator compositeOp, const FloatRect& dstRect, BlendMode)
 {
-    m_image->drawPatternForContainer(context, m_containerSize, m_pageScale, m_zoom, srcRect, patternTransform, phase, colorSpace, compositeOp, dstRect);
+    m_image->drawPatternForContainer(context, m_containerSize, m_zoom, srcRect, patternTransform, phase, colorSpace, compositeOp, dstRect);
 }
 
 } // namespace WebCore
index fed96da..b3228e5 100644 (file)
@@ -38,9 +38,9 @@ namespace WebCore {
 
 class SVGImageForContainer : public Image {
 public:
-    static PassRefPtr<SVGImageForContainer> create(SVGImage* image, const FloatSize& containerSize, float pageScale, float zoom)
+    static PassRefPtr<SVGImageForContainer> create(SVGImage* image, const FloatSize& containerSize, float zoom)
     {
-        return adoptRef(new SVGImageForContainer(image, containerSize, pageScale, zoom));
+        return adoptRef(new SVGImageForContainer(image, containerSize, zoom));
     }
 
     virtual bool isSVGImage() const OVERRIDE { return true; }
@@ -62,25 +62,20 @@ public:
     // FIXME: Implement this to be less conservative.
     virtual bool currentFrameKnownToBeOpaque() OVERRIDE { return false; }
 
-    FloatSize containerSize() { return m_containerSize; }
-    float pageScale() { return m_pageScale; }
-    float zoom() { return m_zoom; }
-
-    void setSize(FloatSize& size) { m_containerSize = size; }
-    void setZoom(float zoom) { m_zoom = zoom; }
-    void setPageScale(float pageScale) { m_pageScale = pageScale; }
-
 private:
-    SVGImageForContainer(SVGImage* image, const FloatSize& containerSize, float pageScale, float zoom)
-        : m_image(image), m_containerSize(containerSize), m_pageScale(pageScale), m_zoom(zoom) { }
+    SVGImageForContainer(SVGImage* image, const FloatSize& containerSize, float zoom)
+        : m_image(image)
+        , m_containerSize(containerSize)
+        , m_zoom(zoom)
+    {
+    }
 
     virtual void destroyDecodedData(bool /*destroyAll*/ = true) { }
     virtual unsigned decodedSize() const { return 0; }
 
     SVGImage* m_image;
-    FloatSize m_containerSize;
-    float m_pageScale;
-    float m_zoom;
+    const FloatSize m_containerSize;
+    const float m_zoom;
 };
 }