Applying a filter on an SVG element, which is larger than 4096 pixels, causes this...
authorsaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 May 2015 23:32:45 +0000 (23:32 +0000)
committersaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 May 2015 23:32:45 +0000 (23:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144335

Reviewed by Daniel Bates.

Address comments raised by Darin Adler in review.

* platform/graphics/ImageBuffer.cpp:
(WebCore::ImageBuffer::sizeNeedsClamping):
(WebCore::ImageBuffer::clampedSize):
(WebCore::ImageBuffer::isSizeClamped): Deleted.
* platform/graphics/ImageBuffer.h:
The condition for the negative width/height in isSizeClamped() was wrong. Use
FloatSize::isEmpty() instead and rename the function to sizeNeedsClamping().
The new function should return the opposite of the old function return value.

* platform/graphics/filters/FilterEffect.cpp:
(WebCore::FilterEffect::apply):
(WebCore::FilterEffect::asUnmultipliedImage):
(WebCore::FilterEffect::asPremultipliedImage):
(WebCore::FilterEffect::copyUnmultipliedImage):
(WebCore::FilterEffect::copyPremultipliedImage):
(WebCore::FilterEffect::createUnmultipliedImageResult):
(WebCore::FilterEffect::createPremultipliedImageResult):
* rendering/FilterEffectRenderer.cpp:
(WebCore::FilterEffectRenderer::updateBackingStoreRect):
(WebCore::FilterEffectRendererHelper::beginFilterEffect):
* rendering/svg/RenderSVGResourceFilter.cpp:
(WebCore::RenderSVGResourceFilter::applyResource):
Call the new function and negate the condition for the return value.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/ImageBuffer.cpp
Source/WebCore/platform/graphics/ImageBuffer.h
Source/WebCore/platform/graphics/filters/FilterEffect.cpp
Source/WebCore/rendering/FilterEffectRenderer.cpp
Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp

index def455d..3928d0a 100644 (file)
@@ -1,3 +1,36 @@
+2015-05-07  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Applying a filter on an SVG element, which is larger than 4096 pixels, causes this element to be rendered shifted to the left
+        https://bugs.webkit.org/show_bug.cgi?id=144335
+
+        Reviewed by Daniel Bates.
+
+        Address comments raised by Darin Adler in review.
+
+        * platform/graphics/ImageBuffer.cpp:
+        (WebCore::ImageBuffer::sizeNeedsClamping):
+        (WebCore::ImageBuffer::clampedSize):
+        (WebCore::ImageBuffer::isSizeClamped): Deleted.
+        * platform/graphics/ImageBuffer.h:
+        The condition for the negative width/height in isSizeClamped() was wrong. Use
+        FloatSize::isEmpty() instead and rename the function to sizeNeedsClamping().
+        The new function should return the opposite of the old function return value.
+        
+        * platform/graphics/filters/FilterEffect.cpp:
+        (WebCore::FilterEffect::apply):
+        (WebCore::FilterEffect::asUnmultipliedImage):
+        (WebCore::FilterEffect::asPremultipliedImage):
+        (WebCore::FilterEffect::copyUnmultipliedImage):
+        (WebCore::FilterEffect::copyPremultipliedImage):
+        (WebCore::FilterEffect::createUnmultipliedImageResult):
+        (WebCore::FilterEffect::createPremultipliedImageResult):
+        * rendering/FilterEffectRenderer.cpp:
+        (WebCore::FilterEffectRenderer::updateBackingStoreRect):
+        (WebCore::FilterEffectRendererHelper::beginFilterEffect):
+        * rendering/svg/RenderSVGResourceFilter.cpp:
+        (WebCore::RenderSVGResourceFilter::applyResource):
+        Call the new function and negate the condition for the return value.
+
 2015-05-07  Anders Carlsson  <andersca@apple.com>
 
         Build fix.
index 1390cdb..52217d6 100644 (file)
@@ -36,29 +36,26 @@ namespace WebCore {
 static const float MaxClampedLength = 4096;
 static const float MaxClampedArea = MaxClampedLength * MaxClampedLength;
 
-bool ImageBuffer::isSizeClamped(const FloatSize& size)
+bool ImageBuffer::sizeNeedsClamping(const FloatSize& size)
 {
-    if (size.width() < 0 && size.height() < 0)
-        return false;
-
     if (size.isEmpty())
-        return true;
+        return false;
 
-    return floorf(size.height()) * floorf(size.width()) <= MaxClampedArea;
+    return floorf(size.height()) * floorf(size.width()) > MaxClampedArea;
 }
 
-bool ImageBuffer::isSizeClamped(const FloatSize& size, FloatSize& scale)
+bool ImageBuffer::sizeNeedsClamping(const FloatSize& size, FloatSize& scale)
 {
     FloatSize scaledSize(size);
     scaledSize.scale(scale.width(), scale.height());
 
-    if (isSizeClamped(scaledSize))
-        return true;
+    if (!sizeNeedsClamping(scaledSize))
+        return false;
 
     // The area of scaled size is bigger than the upper limit, adjust the scale to fit.
     scale.scale(sqrtf(MaxClampedArea / (scaledSize.width() * scaledSize.height())));
-    ASSERT(isSizeClamped(size, scale));
-    return false;
+    ASSERT(!sizeNeedsClamping(size, scale));
+    return true;
 }
 
 FloatSize ImageBuffer::clampedSize(const FloatSize& size)
@@ -73,8 +70,8 @@ FloatSize ImageBuffer::clampedSize(const FloatSize& size, FloatSize& scale)
 
     FloatSize clampedSize = ImageBuffer::clampedSize(size);
     scale = FloatSize(clampedSize.width() / size.width(), clampedSize.height() / size.height());
-    ASSERT(isSizeClamped(clampedSize));
-    ASSERT(isSizeClamped(size, scale));
+    ASSERT(!sizeNeedsClamping(clampedSize));
+    ASSERT(!sizeNeedsClamping(size, scale));
     return clampedSize;
 }
 
index 320d8df..f2ee506 100644 (file)
@@ -126,8 +126,8 @@ public:
     void setSpaceSize(const FloatSize& space) { m_space = space; }
 
     // These functions are used when clamping the ImageBuffer which is created for filter, masker or clipper.
-    static bool isSizeClamped(const FloatSize&);
-    static bool isSizeClamped(const FloatSize&, FloatSize& scale);
+    static bool sizeNeedsClamping(const FloatSize&);
+    static bool sizeNeedsClamping(const FloatSize&, FloatSize& scale);
     static FloatSize clampedSize(const FloatSize&);
     static FloatSize clampedSize(const FloatSize&, FloatSize& scale);
     static FloatRect clampedRect(const FloatRect&);
index bdd6adc..8ddfc81 100644 (file)
@@ -147,7 +147,7 @@ void FilterEffect::apply()
     determineAbsolutePaintRect();
     setResultColorSpace(m_operatingColorSpace);
 
-    if (m_absolutePaintRect.isEmpty() || !ImageBuffer::isSizeClamped(m_absolutePaintRect.size()))
+    if (m_absolutePaintRect.isEmpty() || ImageBuffer::sizeNeedsClamping(m_absolutePaintRect.size()))
         return;
 
     if (requiresValidPreMultipliedPixels()) {
@@ -313,7 +313,7 @@ ImageBuffer* FilterEffect::openCLImageToImageBuffer()
 PassRefPtr<Uint8ClampedArray> FilterEffect::asUnmultipliedImage(const IntRect& rect)
 {
     IntSize scaledSize(rect.size());
-    ASSERT(ImageBuffer::isSizeClamped(scaledSize));
+    ASSERT(!ImageBuffer::sizeNeedsClamping(scaledSize));
     scaledSize.scale(m_filter.filterScale());
     RefPtr<Uint8ClampedArray> imageData = Uint8ClampedArray::createUninitialized(scaledSize.width() * scaledSize.height() * 4);
     copyUnmultipliedImage(imageData.get(), rect);
@@ -323,7 +323,7 @@ PassRefPtr<Uint8ClampedArray> FilterEffect::asUnmultipliedImage(const IntRect& r
 PassRefPtr<Uint8ClampedArray> FilterEffect::asPremultipliedImage(const IntRect& rect)
 {
     IntSize scaledSize(rect.size());
-    ASSERT(ImageBuffer::isSizeClamped(scaledSize));
+    ASSERT(!ImageBuffer::sizeNeedsClamping(scaledSize));
     scaledSize.scale(m_filter.filterScale());
     RefPtr<Uint8ClampedArray> imageData = Uint8ClampedArray::createUninitialized(scaledSize.width() * scaledSize.height() * 4);
     copyPremultipliedImage(imageData.get(), rect);
@@ -389,7 +389,7 @@ void FilterEffect::copyUnmultipliedImage(Uint8ClampedArray* destination, const I
             m_unmultipliedImageResult = m_imageBufferResult->getUnmultipliedImageData(IntRect(IntPoint(), m_absolutePaintRect.size()));
         else {
             IntSize inputSize(m_absolutePaintRect.size());
-            ASSERT(ImageBuffer::isSizeClamped(inputSize));
+            ASSERT(!ImageBuffer::sizeNeedsClamping(inputSize));
             inputSize.scale(m_filter.filterScale());
             m_unmultipliedImageResult = Uint8ClampedArray::createUninitialized(inputSize.width() * inputSize.height() * 4);
             unsigned char* sourceComponent = m_premultipliedImageResult->data();
@@ -425,7 +425,7 @@ void FilterEffect::copyPremultipliedImage(Uint8ClampedArray* destination, const
             m_premultipliedImageResult = m_imageBufferResult->getPremultipliedImageData(IntRect(IntPoint(), m_absolutePaintRect.size()));
         else {
             IntSize inputSize(m_absolutePaintRect.size());
-            ASSERT(ImageBuffer::isSizeClamped(inputSize));
+            ASSERT(!ImageBuffer::sizeNeedsClamping(inputSize));
             inputSize.scale(m_filter.filterScale());
             m_premultipliedImageResult = Uint8ClampedArray::createUninitialized(inputSize.width() * inputSize.height() * 4);
             unsigned char* sourceComponent = m_unmultipliedImageResult->data();
@@ -469,7 +469,7 @@ Uint8ClampedArray* FilterEffect::createUnmultipliedImageResult()
         return nullptr;
 
     IntSize resultSize(m_absolutePaintRect.size());
-    ASSERT(ImageBuffer::isSizeClamped(resultSize));
+    ASSERT(!ImageBuffer::sizeNeedsClamping(resultSize));
     resultSize.scale(m_filter.filterScale());
     m_unmultipliedImageResult = Uint8ClampedArray::createUninitialized(resultSize.width() * resultSize.height() * 4);
     return m_unmultipliedImageResult.get();
@@ -483,7 +483,7 @@ Uint8ClampedArray* FilterEffect::createPremultipliedImageResult()
         return nullptr;
 
     IntSize resultSize(m_absolutePaintRect.size());
-    ASSERT(ImageBuffer::isSizeClamped(resultSize));
+    ASSERT(!ImageBuffer::sizeNeedsClamping(resultSize));
     resultSize.scale(m_filter.filterScale());
     m_premultipliedImageResult = Uint8ClampedArray::createUninitialized(resultSize.width() * resultSize.height() * 4);
     return m_premultipliedImageResult.get();
index 4c8cadc..62dac4b 100644 (file)
@@ -305,7 +305,7 @@ bool FilterEffectRenderer::build(RenderElement* renderer, const FilterOperations
 
 bool FilterEffectRenderer::updateBackingStoreRect(const FloatRect& filterRect)
 {
-    if (filterRect.isEmpty() || !ImageBuffer::isSizeClamped(filterRect.size()))
+    if (filterRect.isEmpty() || ImageBuffer::sizeNeedsClamping(filterRect.size()))
         return false;
 
     if (filterRect == sourceImageRect())
@@ -401,7 +401,7 @@ bool FilterEffectRendererHelper::beginFilterEffect()
     filter->allocateBackingStoreIfNeeded();
     // Paint into the context that represents the SourceGraphic of the filter.
     GraphicsContext* sourceGraphicsContext = filter->inputContext();
-    if (!sourceGraphicsContext || filter->filterRegion().isEmpty() || !ImageBuffer::isSizeClamped(filter->filterRegion().size())) {
+    if (!sourceGraphicsContext || filter->filterRegion().isEmpty() || ImageBuffer::sizeNeedsClamping(filter->filterRegion().size())) {
         // Disable the filters and continue.
         m_haveFilterEffect = false;
         return false;
index 6770e8b..e31865d 100644 (file)
@@ -151,7 +151,7 @@ bool RenderSVGResourceFilter::applyResource(RenderElement& renderer, const Rende
 
     // Determine scale factor for filter. The size of intermediate ImageBuffers shouldn't be bigger than kMaxFilterSize.
     FloatRect tempSourceRect = absoluteDrawingRegion;
-    ImageBuffer::isSizeClamped(tempSourceRect.size(), scale);
+    ImageBuffer::sizeNeedsClamping(tempSourceRect.size(), scale);
     tempSourceRect.scale(scale.width(), scale.height());
 
     // Set the scale level in SVGFilter.
@@ -166,7 +166,7 @@ bool RenderSVGResourceFilter::applyResource(RenderElement& renderer, const Rende
     FloatRect subRegion = lastEffect->maxEffectRect();
     // At least one FilterEffect has a too big image size,
     // recalculate the effect sizes with new scale factors.
-    if (!ImageBuffer::isSizeClamped(subRegion.size(), scale)) {
+    if (ImageBuffer::sizeNeedsClamping(subRegion.size(), scale)) {
         filterData->filter->setFilterResolution(scale);
         RenderSVGResourceFilterPrimitive::determineFilterPrimitiveSubregion(*lastEffect);
     }