[GTK][EFL] ImageBufferCairo should accept resolution factor
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Jun 2016 16:05:46 +0000 (16:05 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Jun 2016 16:05:46 +0000 (16:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157848

Reviewed by Martin Robinson.

Source/WebCore:

ImageBufferCairo ignored the resolution factor passed in its constructor.
This resolution factor is originally introduced for HiDPI Canvas,
and since HiDPI canvas is not enabled in the ports using Cairo,
the lack of this implementation does not cause any problems.
And now, HiDPI Canvas is removed from the tree.

However, WebKit CSS filter uses this path.
The missing implementation is required under the HiDPI environment.

Since Cairo surface can have the device scale factor transparently,
the operations onto the surface is correctly done in the logical coordinate system.
So all we need to handle carefully is the direct surface modification done
in filter effects.

In this patch, we extend the image buffer size according to the resolution factor,
as the same to the CoreGraphics' implementation (ImageBufferCG). And by setting the
device scale factor of the surface correctly, we ensure that the rest of the Cairo
painting stack works with the existing logical coordinate system. And in ImageBufferCairo,
we carefully handle the logical and backing store coordinate system.

The attached test applies the CSS filter onto the svg image. And we resize the image size,
and perform scrolling. It incurs the paint, and filter effect recalcuation.
In that path, the filter effect side assumes that the image buffer size is scaled with the
resolution factor. So without this patch, it incurs buffer overflow and leads WebProcess crash.

* platform/graphics/IntPoint.h:
(WebCore::IntPoint::scale):
* platform/graphics/cairo/ImageBufferCairo.cpp:
(WebCore::ImageBufferData::createCompositorBuffer):
(WebCore::ImageBuffer::ImageBuffer):
(WebCore::ImageBuffer::copyImage):
(WebCore::ImageBuffer::platformTransformColorSpace):
(WebCore::getImageData):
(WebCore::logicalUnit):
(WebCore::backingStoreUnit):
(WebCore::ImageBuffer::getUnmultipliedImageData):
(WebCore::ImageBuffer::getPremultipliedImageData):
(WebCore::ImageBuffer::putByteArray):
(WebCore::ImageBuffer::copyToPlatformTexture):

LayoutTests:

* fast/hidpi/filters-and-image-buffer-resolution-expected.html: Added.
* fast/hidpi/filters-and-image-buffer-resolution.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/hidpi/filters-and-image-buffer-resolution-expected.html [new file with mode: 0644]
LayoutTests/fast/hidpi/filters-and-image-buffer-resolution.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/IntPoint.h
Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp

index 586b82a..9cf2dee 100644 (file)
@@ -1,3 +1,13 @@
+2016-06-24  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [GTK][EFL] ImageBufferCairo should accept resolution factor
+        https://bugs.webkit.org/show_bug.cgi?id=157848
+
+        Reviewed by Martin Robinson.
+
+        * fast/hidpi/filters-and-image-buffer-resolution-expected.html: Added.
+        * fast/hidpi/filters-and-image-buffer-resolution.html: Added.
+
 2016-06-24  Frederic Wang  <fwang@igalia.com>
 
         Refactor RenderMathMLOperator and RenderMathMLToken to avoid using anonymous renderers.
diff --git a/LayoutTests/fast/hidpi/filters-and-image-buffer-resolution-expected.html b/LayoutTests/fast/hidpi/filters-and-image-buffer-resolution-expected.html
new file mode 100644 (file)
index 0000000..bf68cb5
--- /dev/null
@@ -0,0 +1,21 @@
+<script src="resources/ensure-hidpi.js"></script>
+<style>
+img {
+    width: 200px;
+    height: 200px;
+}
+
+p {
+    width: 200px;
+    height: 100px;
+    display: inline-block;
+    font: 20px/1 Ahem;
+}
+
+#target {
+    height: 1000px;
+}
+</style>
+<div>This is testing that image buffers for filters are correctly scaled with the device scale factor on the HiDPI environment. If it is wrong, this test causes a crash.</div>
+<img src="data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='200' height='200'><rect fill='green' width='100%' height='100%'/></svg>">
+<img id="target" src="data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='200' height='200'><rect fill='green' width='100%' height='100%'/></svg>">
diff --git a/LayoutTests/fast/hidpi/filters-and-image-buffer-resolution.html b/LayoutTests/fast/hidpi/filters-and-image-buffer-resolution.html
new file mode 100644 (file)
index 0000000..f24d2ed
--- /dev/null
@@ -0,0 +1,47 @@
+<script src="resources/ensure-hidpi.js"></script>
+<style>
+img {
+    width: 200px;
+    height: 200px;
+}
+
+p {
+    width: 200px;
+    height: 100px;
+    display: inline-block;
+    font: 20px/1 Ahem;
+}
+
+.filtered {
+    -webkit-filter: blur(0px);
+}
+</style>
+<script>
+function runTest() {
+    if (window.testRunner) {
+        scaleFactorIsSet();
+        return;
+    }
+
+    testRunner.waitUntilDone();
+    testRunner.setBackingScaleFactor(2, scaleFactorIsSet);
+}
+
+function scaleFactorIsSet() {
+    var target = document.getElementById('target');
+    target.style.height = 1000;
+    scrollBy(0, 200);
+    setTimeout(function () {
+        scrollBy(0, -200);
+        setTimeout(function () {
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }, 0);
+    }, 0);
+}
+
+window.addEventListener("load", runTest, false);
+</script>
+<div>This is testing that image buffers for filters are correctly scaled with the device scale factor on the HiDPI environment. If it is wrong, this test causes a crash.</div>
+<img src="data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='200' height='200'><rect fill='green' width='100%' height='100%'/></svg>">
+<img class="filtered" id="target" src="data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='200' height='200'><rect fill='green' width='100%' height='100%'/></svg>">
index 398fe0d..c48cde1 100644 (file)
@@ -1,3 +1,50 @@
+2016-06-24  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [GTK][EFL] ImageBufferCairo should accept resolution factor
+        https://bugs.webkit.org/show_bug.cgi?id=157848
+
+        Reviewed by Martin Robinson.
+
+        ImageBufferCairo ignored the resolution factor passed in its constructor.
+        This resolution factor is originally introduced for HiDPI Canvas,
+        and since HiDPI canvas is not enabled in the ports using Cairo,
+        the lack of this implementation does not cause any problems.
+        And now, HiDPI Canvas is removed from the tree.
+
+        However, WebKit CSS filter uses this path.
+        The missing implementation is required under the HiDPI environment.
+
+        Since Cairo surface can have the device scale factor transparently,
+        the operations onto the surface is correctly done in the logical coordinate system.
+        So all we need to handle carefully is the direct surface modification done
+        in filter effects.
+
+        In this patch, we extend the image buffer size according to the resolution factor,
+        as the same to the CoreGraphics' implementation (ImageBufferCG). And by setting the
+        device scale factor of the surface correctly, we ensure that the rest of the Cairo
+        painting stack works with the existing logical coordinate system. And in ImageBufferCairo,
+        we carefully handle the logical and backing store coordinate system.
+
+        The attached test applies the CSS filter onto the svg image. And we resize the image size,
+        and perform scrolling. It incurs the paint, and filter effect recalcuation.
+        In that path, the filter effect side assumes that the image buffer size is scaled with the
+        resolution factor. So without this patch, it incurs buffer overflow and leads WebProcess crash.
+
+        * platform/graphics/IntPoint.h:
+        (WebCore::IntPoint::scale):
+        * platform/graphics/cairo/ImageBufferCairo.cpp:
+        (WebCore::ImageBufferData::createCompositorBuffer):
+        (WebCore::ImageBuffer::ImageBuffer):
+        (WebCore::ImageBuffer::copyImage):
+        (WebCore::ImageBuffer::platformTransformColorSpace):
+        (WebCore::getImageData):
+        (WebCore::logicalUnit):
+        (WebCore::backingStoreUnit):
+        (WebCore::ImageBuffer::getUnmultipliedImageData):
+        (WebCore::ImageBuffer::getPremultipliedImageData):
+        (WebCore::ImageBuffer::putByteArray):
+        (WebCore::ImageBuffer::copyToPlatformTexture):
+
 2016-06-24  Frederic Wang  <fwang@igalia.com>
 
         Refactor RenderMathMLOperator and RenderMathMLToken to avoid using anonymous renderers.
index 3390f44..4fa9e5b 100644 (file)
@@ -82,6 +82,11 @@ public:
         m_x = lroundf(static_cast<float>(m_x * sx));
         m_y = lroundf(static_cast<float>(m_y * sy));
     }
+
+    void scale(float scale)
+    {
+        this->scale(scale, scale);
+    }
     
     IntPoint expandedTo(const IntPoint& other) const
     {
index 0453a62..586cd53 100644 (file)
@@ -121,6 +121,7 @@ void ImageBufferData::createCompositorBuffer()
 
     cairo_device_t* device = GLContext::sharingContext()->cairoDevice();
     m_compositorSurface = adoptRef(cairo_gl_surface_create_for_texture(device, CAIRO_CONTENT_COLOR_ALPHA, m_compositorTexture, m_size.width(), m_size.height()));
+    cairoSurfaceSetDeviceScale(m_compositorSurface.get(), m_resolutionScale, m_resolutionScale);
     m_compositorCr = adoptRef(cairo_create(m_compositorSurface.get()));
     cairo_set_antialias(m_compositorCr.get(), CAIRO_ANTIALIAS_NONE);
 }
@@ -185,12 +186,23 @@ void ImageBufferData::createCairoGLSurface()
 }
 #endif
 
-ImageBuffer::ImageBuffer(const FloatSize& size, float /* resolutionScale */, ColorSpace, RenderingMode renderingMode, bool& success)
+ImageBuffer::ImageBuffer(const FloatSize& size, float resolutionScale, ColorSpace, RenderingMode renderingMode, bool& success)
     : m_data(IntSize(size), renderingMode)
-    , m_size(size)
     , m_logicalSize(size)
+    , m_resolutionScale(resolutionScale)
 {
     success = false;  // Make early return mean error.
+
+    float scaledWidth = ceilf(m_resolutionScale * size.width());
+    float scaledHeight = ceilf(m_resolutionScale * size.height());
+
+    // FIXME: Should we automatically use a lower resolution?
+    if (!FloatSize(scaledWidth, scaledHeight).isExpressibleAsIntSize())
+        return;
+
+    m_size = IntSize(scaledWidth, scaledHeight);
+    m_data.m_size = m_size;
+
     if (m_size.isEmpty())
         return;
 
@@ -204,11 +216,13 @@ ImageBuffer::ImageBuffer(const FloatSize& size, float /* resolutionScale */, Col
 #else
     ASSERT(m_data.m_renderingMode != Accelerated);
 #endif
-        m_data.m_surface = adoptRef(cairo_image_surface_create(CAIRO_FORMAT_ARGB32, size.width(), size.height()));
+        m_data.m_surface = adoptRef(cairo_image_surface_create(CAIRO_FORMAT_ARGB32, m_size.width(), m_size.height()));
 
     if (cairo_surface_status(m_data.m_surface.get()) != CAIRO_STATUS_SUCCESS)
         return;  // create will notice we didn't set m_initialized and fail.
 
+    cairoSurfaceSetDeviceScale(m_data.m_surface.get(), m_resolutionScale, m_resolutionScale);
+
     RefPtr<cairo_t> cr = adoptRef(cairo_create(m_data.m_surface.get()));
     m_data.m_platformContext.setCr(cr.get());
     m_data.m_context = std::make_unique<GraphicsContext>(&m_data.m_platformContext);
@@ -231,6 +245,7 @@ RefPtr<Image> ImageBuffer::sinkIntoImage(std::unique_ptr<ImageBuffer> imageBuffe
 
 RefPtr<Image> ImageBuffer::copyImage(BackingStoreCopy copyBehavior, ScaleBehavior) const
 {
+    // copyCairoImageSurface inherits surface's device scale factor.
     if (copyBehavior == CopyBackingStore)
         return BitmapImage::create(copyCairoImageSurface(m_data.m_surface.get()));
 
@@ -283,7 +298,7 @@ void ImageBuffer::platformTransformColorSpace(const Vector<int>& lookUpTable)
             *pixel = premultipliedARGBFromColor(pixelColor);
         }
     }
-    cairo_surface_mark_dirty_rectangle(m_data.m_surface.get(), 0, 0, m_size.width(), m_size.height());
+    cairo_surface_mark_dirty_rectangle(m_data.m_surface.get(), 0, 0, m_logicalSize.width(), m_logicalSize.height());
 }
 
 RefPtr<cairo_surface_t> copySurfaceToImageAndAdjustRect(cairo_surface_t* surface, IntRect& rect)
@@ -301,7 +316,7 @@ RefPtr<cairo_surface_t> copySurfaceToImageAndAdjustRect(cairo_surface_t* surface
 }
 
 template <Multiply multiplied>
-RefPtr<Uint8ClampedArray> getImageData(const IntRect& rect, const ImageBufferData& data, const IntSize& size)
+RefPtr<Uint8ClampedArray> getImageData(const IntRect& rect, const IntRect& logicalRect, const ImageBufferData& data, const IntSize& size, const IntSize& logicalSize, float resolutionScale)
 {
     RefPtr<Uint8ClampedArray> result = Uint8ClampedArray::createUninitialized(rect.width() * rect.height() * 4);
 
@@ -330,13 +345,17 @@ RefPtr<Uint8ClampedArray> getImageData(const IntRect& rect, const ImageBufferDat
         endy = size.height();
     int numRows = endy - originy;
 
+    // The size of the derived surface is in BackingStoreCoordinateSystem.
+    // We need to set the device scale for the derived surface from this ImageBuffer.
     IntRect imageRect(originx, originy, numColumns, numRows);
     RefPtr<cairo_surface_t> imageSurface = copySurfaceToImageAndAdjustRect(data.m_surface.get(), imageRect);
+    cairoSurfaceSetDeviceScale(imageSurface.get(), resolutionScale, resolutionScale);
     originx = imageRect.x();
     originy = imageRect.y();
     if (imageSurface != data.m_surface.get()) {
-        IntRect area = intersection(rect, IntRect(0, 0, size.width(), size.height()));
-        copyRectFromOneSurfaceToAnother(data.m_surface.get(), imageSurface.get(), IntSize(-area.x(), -area.y()), IntRect(IntPoint(), area.size()), IntSize(), CAIRO_OPERATOR_SOURCE);
+        // This cairo surface operation is done in LogicalCoordinateSystem.
+        IntRect logicalArea = intersection(logicalRect, IntRect(0, 0, logicalSize.width(), logicalSize.height()));
+        copyRectFromOneSurfaceToAnother(data.m_surface.get(), imageSurface.get(), IntSize(-logicalArea.x(), -logicalArea.y()), IntRect(IntPoint(), logicalArea.size()), IntSize(), CAIRO_OPERATOR_SOURCE);
     }
 
     unsigned char* dataSrc = cairo_image_surface_get_data(imageSurface.get());
@@ -377,53 +396,91 @@ RefPtr<Uint8ClampedArray> getImageData(const IntRect& rect, const ImageBufferDat
     return result.release();
 }
 
-RefPtr<Uint8ClampedArray> ImageBuffer::getUnmultipliedImageData(const IntRect& rect, CoordinateSystem) const
+template<typename Unit>
+inline Unit logicalUnit(const Unit& value, ImageBuffer::CoordinateSystem coordinateSystemOfValue, float resolutionScale)
 {
-    return getImageData<Unmultiplied>(rect, m_data, m_size);
+    if (coordinateSystemOfValue == ImageBuffer::LogicalCoordinateSystem || resolutionScale == 1.0)
+        return value;
+    Unit result(value);
+    result.scale(1.0 / resolutionScale);
+    return result;
 }
 
-RefPtr<Uint8ClampedArray> ImageBuffer::getPremultipliedImageData(const IntRect& rect, CoordinateSystem) const
+template<typename Unit>
+inline Unit backingStoreUnit(const Unit& value, ImageBuffer::CoordinateSystem coordinateSystemOfValue, float resolutionScale)
 {
-    return getImageData<Premultiplied>(rect, m_data, m_size);
+    if (coordinateSystemOfValue == ImageBuffer::BackingStoreCoordinateSystem || resolutionScale == 1.0)
+        return value;
+    Unit result(value);
+    result.scale(resolutionScale);
+    return result;
 }
 
-void ImageBuffer::putByteArray(Multiply multiplied, Uint8ClampedArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint, CoordinateSystem)
+RefPtr<Uint8ClampedArray> ImageBuffer::getUnmultipliedImageData(const IntRect& rect, CoordinateSystem coordinateSystem) const
 {
+    IntRect logicalRect = logicalUnit(rect, coordinateSystem, m_resolutionScale);
+    IntRect backingStoreRect = backingStoreUnit(rect, coordinateSystem, m_resolutionScale);
+    return getImageData<Unmultiplied>(backingStoreRect, logicalRect, m_data, m_size, m_logicalSize, m_resolutionScale);
+}
 
-    ASSERT(sourceRect.width() > 0);
-    ASSERT(sourceRect.height() > 0);
+RefPtr<Uint8ClampedArray> ImageBuffer::getPremultipliedImageData(const IntRect& rect, CoordinateSystem coordinateSystem) const
+{
+    IntRect logicalRect = logicalUnit(rect, coordinateSystem, m_resolutionScale);
+    IntRect backingStoreRect = backingStoreUnit(rect, coordinateSystem, m_resolutionScale);
+    return getImageData<Premultiplied>(backingStoreRect, logicalRect, m_data, m_size, m_logicalSize, m_resolutionScale);
+}
 
-    int originx = sourceRect.x();
-    int destx = destPoint.x() + sourceRect.x();
+void ImageBuffer::putByteArray(Multiply multiplied, Uint8ClampedArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint, CoordinateSystem coordinateSystem)
+{
+    IntRect scaledSourceRect = backingStoreUnit(sourceRect, coordinateSystem, m_resolutionScale);
+    IntSize scaledSourceSize = backingStoreUnit(sourceSize, coordinateSystem, m_resolutionScale);
+    IntPoint scaledDestPoint = backingStoreUnit(destPoint, coordinateSystem, m_resolutionScale);
+    IntRect logicalSourceRect = logicalUnit(sourceRect, coordinateSystem, m_resolutionScale);
+    IntPoint logicalDestPoint = logicalUnit(destPoint, coordinateSystem, m_resolutionScale);
+
+    ASSERT(scaledSourceRect.width() > 0);
+    ASSERT(scaledSourceRect.height() > 0);
+
+    int originx = scaledSourceRect.x();
+    int destx = scaledDestPoint.x() + scaledSourceRect.x();
+    int logicalDestx = logicalDestPoint.x() + logicalSourceRect.x();
     ASSERT(destx >= 0);
     ASSERT(destx < m_size.width());
     ASSERT(originx >= 0);
-    ASSERT(originx <= sourceRect.maxX());
+    ASSERT(originx <= scaledSourceRect.maxX());
 
-    int endx = destPoint.x() + sourceRect.maxX();
+    int endx = scaledDestPoint.x() + scaledSourceRect.maxX();
+    int logicalEndx = logicalDestPoint.x() + logicalSourceRect.maxX();
     ASSERT(endx <= m_size.width());
 
     int numColumns = endx - destx;
+    int logicalNumColumns = logicalEndx - logicalDestx;
 
-    int originy = sourceRect.y();
-    int desty = destPoint.y() + sourceRect.y();
+    int originy = scaledSourceRect.y();
+    int desty = scaledDestPoint.y() + scaledSourceRect.y();
+    int logicalDesty = logicalDestPoint.y() + logicalSourceRect.y();
     ASSERT(desty >= 0);
     ASSERT(desty < m_size.height());
     ASSERT(originy >= 0);
-    ASSERT(originy <= sourceRect.maxY());
+    ASSERT(originy <= scaledSourceRect.maxY());
 
-    int endy = destPoint.y() + sourceRect.maxY();
+    int endy = scaledDestPoint.y() + scaledSourceRect.maxY();
+    int logicalEndy = logicalDestPoint.y() + logicalSourceRect.maxY();
     ASSERT(endy <= m_size.height());
     int numRows = endy - desty;
+    int logicalNumRows = logicalEndy - logicalDesty;
 
+    // The size of the derived surface is in BackingStoreCoordinateSystem.
+    // We need to set the device scale for the derived surface from this ImageBuffer.
     IntRect imageRect(destx, desty, numColumns, numRows);
     RefPtr<cairo_surface_t> imageSurface = copySurfaceToImageAndAdjustRect(m_data.m_surface.get(), imageRect);
+    cairoSurfaceSetDeviceScale(imageSurface.get(), m_resolutionScale, m_resolutionScale);
     destx = imageRect.x();
     desty = imageRect.y();
 
     unsigned char* pixelData = cairo_image_surface_get_data(imageSurface.get());
 
-    unsigned srcBytesPerRow = 4 * sourceSize.width();
+    unsigned srcBytesPerRow = 4 * scaledSourceSize.width();
     int stride = cairo_image_surface_get_stride(imageSurface.get());
 
     unsigned char* srcRows = source->data() + originy * srcBytesPerRow + originx * 4;
@@ -453,10 +510,13 @@ void ImageBuffer::putByteArray(Multiply multiplied, Uint8ClampedArray* source, c
         srcRows += srcBytesPerRow;
     }
 
-    cairo_surface_mark_dirty_rectangle(imageSurface.get(), destx, desty, numColumns, numRows);
+    // This cairo surface operation is done in LogicalCoordinateSystem.
+    cairo_surface_mark_dirty_rectangle(imageSurface.get(), logicalDestx, logicalDesty, logicalNumColumns, logicalNumRows);
 
-    if (imageSurface != m_data.m_surface.get())
-        copyRectFromOneSurfaceToAnother(imageSurface.get(), m_data.m_surface.get(), IntSize(), IntRect(0, 0, numColumns, numRows), IntSize(destPoint.x() + sourceRect.x(), destPoint.y() + sourceRect.y()), CAIRO_OPERATOR_SOURCE);
+    if (imageSurface != m_data.m_surface.get()) {
+        // This cairo surface operation is done in LogicalCoordinateSystem.
+        copyRectFromOneSurfaceToAnother(imageSurface.get(), m_data.m_surface.get(), IntSize(), IntRect(0, 0, logicalNumColumns, logicalNumRows), IntSize(logicalDestPoint.x() + logicalSourceRect.x(), logicalDestPoint.y() + logicalSourceRect.y()), CAIRO_OPERATOR_SOURCE);
+    }
 }
 
 #if !PLATFORM(GTK) && !PLATFORM(EFL)
@@ -517,6 +577,7 @@ PlatformLayer* ImageBuffer::platformLayer() const
 bool ImageBuffer::copyToPlatformTexture(GraphicsContext3D&, GC3Denum target, Platform3DObject destinationTexture, GC3Denum internalformat, bool premultiplyAlpha, bool flipY)
 {
 #if ENABLE(ACCELERATED_2D_CANVAS)
+    ASSERT_WITH_MESSAGE(m_resolutionScale == 1.0, "Since the HiDPI Canvas feature is removed, the resolution factor here is always 1.");
     if (premultiplyAlpha || flipY)
         return false;