Changeset 202421 in webkit


Ignore:
Timestamp:
Jun 24, 2016 9:05:46 AM (8 years ago)
Author:
Yusuke Suzuki
Message:

[GTK][EFL] ImageBufferCairo should accept resolution factor
​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.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r202420 r202421  
     12016-06-24  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [GTK][EFL] ImageBufferCairo should accept resolution factor
     4        https://bugs.webkit.org/show_bug.cgi?id=157848
     5
     6        Reviewed by Martin Robinson.
     7
     8        * fast/hidpi/filters-and-image-buffer-resolution-expected.html: Added.
     9        * fast/hidpi/filters-and-image-buffer-resolution.html: Added.
     10
    1112016-06-24  Frederic Wang  <fwang@igalia.com>
    212

  • trunk/Source/WebCore/ChangeLog

    r202420 r202421  
     12016-06-24  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [GTK][EFL] ImageBufferCairo should accept resolution factor
     4        https://bugs.webkit.org/show_bug.cgi?id=157848
     5
     6        Reviewed by Martin Robinson.
     7
     8        ImageBufferCairo ignored the resolution factor passed in its constructor.
     9        This resolution factor is originally introduced for HiDPI Canvas,
     10        and since HiDPI canvas is not enabled in the ports using Cairo,
     11        the lack of this implementation does not cause any problems.
     12        And now, HiDPI Canvas is removed from the tree.
     13
     14        However, WebKit CSS filter uses this path.
     15        The missing implementation is required under the HiDPI environment.
     16
     17        Since Cairo surface can have the device scale factor transparently,
     18        the operations onto the surface is correctly done in the logical coordinate system.
     19        So all we need to handle carefully is the direct surface modification done
     20        in filter effects.
     21
     22        In this patch, we extend the image buffer size according to the resolution factor,
     23        as the same to the CoreGraphics' implementation (ImageBufferCG). And by setting the
     24        device scale factor of the surface correctly, we ensure that the rest of the Cairo
     25        painting stack works with the existing logical coordinate system. And in ImageBufferCairo,
     26        we carefully handle the logical and backing store coordinate system.
     27
     28        The attached test applies the CSS filter onto the svg image. And we resize the image size,
     29        and perform scrolling. It incurs the paint, and filter effect recalcuation.
     30        In that path, the filter effect side assumes that the image buffer size is scaled with the
     31        resolution factor. So without this patch, it incurs buffer overflow and leads WebProcess crash.
     32
     33        * platform/graphics/IntPoint.h:
     34        (WebCore::IntPoint::scale):
     35        * platform/graphics/cairo/ImageBufferCairo.cpp:
     36        (WebCore::ImageBufferData::createCompositorBuffer):
     37        (WebCore::ImageBuffer::ImageBuffer):
     38        (WebCore::ImageBuffer::copyImage):
     39        (WebCore::ImageBuffer::platformTransformColorSpace):
     40        (WebCore::getImageData):
     41        (WebCore::logicalUnit):
     42        (WebCore::backingStoreUnit):
     43        (WebCore::ImageBuffer::getUnmultipliedImageData):
     44        (WebCore::ImageBuffer::getPremultipliedImageData):
     45        (WebCore::ImageBuffer::putByteArray):
     46        (WebCore::ImageBuffer::copyToPlatformTexture):
     47
    1482016-06-24  Frederic Wang  <fwang@igalia.com>
    249

  • trunk/Source/WebCore/platform/graphics/IntPoint.h

    r194466 r202421  
    8383        m_y = lroundf(static_cast<float>(m_y * sy));
    8484    }
     85
     86    void scale(float scale)
     87    {
     88        this->scale(scale, scale);
     89    }
    8590   
    8691    IntPoint expandedTo(const IntPoint& other) const

  • trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp

    r198655 r202421  
    122122    cairo_device_t* device = GLContext::sharingContext()->cairoDevice();
    123123    m_compositorSurface = adoptRef(cairo_gl_surface_create_for_texture(device, CAIRO_CONTENT_COLOR_ALPHA, m_compositorTexture, m_size.width(), m_size.height()));
     124    cairoSurfaceSetDeviceScale(m_compositorSurface.get(), m_resolutionScale, m_resolutionScale);
    124125    m_compositorCr = adoptRef(cairo_create(m_compositorSurface.get()));
    125126    cairo_set_antialias(m_compositorCr.get(), CAIRO_ANTIALIAS_NONE);
    … …  
    186187#endif
    187188
    188 ImageBuffer::ImageBuffer(const FloatSize& size, float /* resolutionScale */, ColorSpace, RenderingMode renderingMode, bool& success)
     189ImageBuffer::ImageBuffer(const FloatSize& size, float resolutionScale, ColorSpace, RenderingMode renderingMode, bool& success)
    189190    : m_data(IntSize(size), renderingMode)
    190     , m_size(size)
    191191    , m_logicalSize(size)
     192    , m_resolutionScale(resolutionScale)
    192193{
    193194    success = false;  // Make early return mean error.
     195
     196    float scaledWidth = ceilf(m_resolutionScale * size.width());
     197    float scaledHeight = ceilf(m_resolutionScale * size.height());
     198
     199    // FIXME: Should we automatically use a lower resolution?
     200    if (!FloatSize(scaledWidth, scaledHeight).isExpressibleAsIntSize())
     201        return;
     202
     203    m_size = IntSize(scaledWidth, scaledHeight);
     204    m_data.m_size = m_size;
     205
    194206    if (m_size.isEmpty())
    195207        return;
    … …  
    205217    ASSERT(m_data.m_renderingMode != Accelerated);
    206218#endif
    207         m_data.m_surface = adoptRef(cairo_image_surface_create(CAIRO_FORMAT_ARGB32, size.width(), size.height()));
     219        m_data.m_surface = adoptRef(cairo_image_surface_create(CAIRO_FORMAT_ARGB32, m_size.width(), m_size.height()));
    208220
    209221    if (cairo_surface_status(m_data.m_surface.get()) != CAIRO_STATUS_SUCCESS)
    210222        return;  // create will notice we didn't set m_initialized and fail.
     223
     224    cairoSurfaceSetDeviceScale(m_data.m_surface.get(), m_resolutionScale, m_resolutionScale);
    211225
    212226    RefPtr<cairo_t> cr = adoptRef(cairo_create(m_data.m_surface.get()));
    … …  
    232246RefPtr<Image> ImageBuffer::copyImage(BackingStoreCopy copyBehavior, ScaleBehavior) const
    233247{
     248    // copyCairoImageSurface inherits surface's device scale factor.
    234249    if (copyBehavior == CopyBackingStore)
    235250        return BitmapImage::create(copyCairoImageSurface(m_data.m_surface.get()));
    … …  
    284299        }
    285300    }
    286     cairo_surface_mark_dirty_rectangle(m_data.m_surface.get(), 0, 0, m_size.width(), m_size.height());
     301    cairo_surface_mark_dirty_rectangle(m_data.m_surface.get(), 0, 0, m_logicalSize.width(), m_logicalSize.height());
    287302}
    288303
    … …  
    302317
    303318template <Multiply multiplied>
    304 RefPtr<Uint8ClampedArray> getImageData(const IntRect& rect, const ImageBufferData& data, const IntSize& size)
     319RefPtr<Uint8ClampedArray> getImageData(const IntRect& rect, const IntRect& logicalRect, const ImageBufferData& data, const IntSize& size, const IntSize& logicalSize, float resolutionScale)
    305320{
    306321    RefPtr<Uint8ClampedArray> result = Uint8ClampedArray::createUninitialized(rect.width() * rect.height() * 4);
    … …  
    331346    int numRows = endy - originy;
    332347
     348    // The size of the derived surface is in BackingStoreCoordinateSystem.
     349    // We need to set the device scale for the derived surface from this ImageBuffer.
    333350    IntRect imageRect(originx, originy, numColumns, numRows);
    334351    RefPtr<cairo_surface_t> imageSurface = copySurfaceToImageAndAdjustRect(data.m_surface.get(), imageRect);
     352    cairoSurfaceSetDeviceScale(imageSurface.get(), resolutionScale, resolutionScale);
    335353    originx = imageRect.x();
    336354    originy = imageRect.y();
    337355    if (imageSurface != data.m_surface.get()) {
    338         IntRect area = intersection(rect, IntRect(0, 0, size.width(), size.height()));
    339         copyRectFromOneSurfaceToAnother(data.m_surface.get(), imageSurface.get(), IntSize(-area.x(), -area.y()), IntRect(IntPoint(), area.size()), IntSize(), CAIRO_OPERATOR_SOURCE);
     356        // This cairo surface operation is done in LogicalCoordinateSystem.
     357        IntRect logicalArea = intersection(logicalRect, IntRect(0, 0, logicalSize.width(), logicalSize.height()));
     358        copyRectFromOneSurfaceToAnother(data.m_surface.get(), imageSurface.get(), IntSize(-logicalArea.x(), -logicalArea.y()), IntRect(IntPoint(), logicalArea.size()), IntSize(), CAIRO_OPERATOR_SOURCE);
    340359    }
    341360
    … …  
    378397}
    379398
    380 RefPtr<Uint8ClampedArray> ImageBuffer::getUnmultipliedImageData(const IntRect& rect, CoordinateSystem) const
    381 {
    382     return getImageData<Unmultiplied>(rect, m_data, m_size);
    383 }
    384 
    385 RefPtr<Uint8ClampedArray> ImageBuffer::getPremultipliedImageData(const IntRect& rect, CoordinateSystem) const
    386 {
    387     return getImageData<Premultiplied>(rect, m_data, m_size);
    388 }
    389 
    390 void ImageBuffer::putByteArray(Multiply multiplied, Uint8ClampedArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint, CoordinateSystem)
    391 {
    392 
    393     ASSERT(sourceRect.width() > 0);
    394     ASSERT(sourceRect.height() > 0);
    395 
    396     int originx = sourceRect.x();
    397     int destx = destPoint.x() + sourceRect.x();
     399template<typename Unit>
     400inline Unit logicalUnit(const Unit& value, ImageBuffer::CoordinateSystem coordinateSystemOfValue, float resolutionScale)
     401{
     402    if (coordinateSystemOfValue == ImageBuffer::LogicalCoordinateSystem || resolutionScale == 1.0)
     403        return value;
     404    Unit result(value);
     405    result.scale(1.0 / resolutionScale);
     406    return result;
     407}
     408
     409template<typename Unit>
     410inline Unit backingStoreUnit(const Unit& value, ImageBuffer::CoordinateSystem coordinateSystemOfValue, float resolutionScale)
     411{
     412    if (coordinateSystemOfValue == ImageBuffer::BackingStoreCoordinateSystem || resolutionScale == 1.0)
     413        return value;
     414    Unit result(value);
     415    result.scale(resolutionScale);
     416    return result;
     417}
     418
     419RefPtr<Uint8ClampedArray> ImageBuffer::getUnmultipliedImageData(const IntRect& rect, CoordinateSystem coordinateSystem) const
     420{
     421    IntRect logicalRect = logicalUnit(rect, coordinateSystem, m_resolutionScale);
     422    IntRect backingStoreRect = backingStoreUnit(rect, coordinateSystem, m_resolutionScale);
     423    return getImageData<Unmultiplied>(backingStoreRect, logicalRect, m_data, m_size, m_logicalSize, m_resolutionScale);
     424}
     425
     426RefPtr<Uint8ClampedArray> ImageBuffer::getPremultipliedImageData(const IntRect& rect, CoordinateSystem coordinateSystem) const
     427{
     428    IntRect logicalRect = logicalUnit(rect, coordinateSystem, m_resolutionScale);
     429    IntRect backingStoreRect = backingStoreUnit(rect, coordinateSystem, m_resolutionScale);
     430    return getImageData<Premultiplied>(backingStoreRect, logicalRect, m_data, m_size, m_logicalSize, m_resolutionScale);
     431}
     432
     433void ImageBuffer::putByteArray(Multiply multiplied, Uint8ClampedArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint, CoordinateSystem coordinateSystem)
     434{
     435    IntRect scaledSourceRect = backingStoreUnit(sourceRect, coordinateSystem, m_resolutionScale);
     436    IntSize scaledSourceSize = backingStoreUnit(sourceSize, coordinateSystem, m_resolutionScale);
     437    IntPoint scaledDestPoint = backingStoreUnit(destPoint, coordinateSystem, m_resolutionScale);
     438    IntRect logicalSourceRect = logicalUnit(sourceRect, coordinateSystem, m_resolutionScale);
     439    IntPoint logicalDestPoint = logicalUnit(destPoint, coordinateSystem, m_resolutionScale);
     440
     441    ASSERT(scaledSourceRect.width() > 0);
     442    ASSERT(scaledSourceRect.height() > 0);
     443
     444    int originx = scaledSourceRect.x();
     445    int destx = scaledDestPoint.x() + scaledSourceRect.x();
     446    int logicalDestx = logicalDestPoint.x() + logicalSourceRect.x();
    398447    ASSERT(destx >= 0);
    399448    ASSERT(destx < m_size.width());
    400449    ASSERT(originx >= 0);
    401     ASSERT(originx <= sourceRect.maxX());
    402 
    403     int endx = destPoint.x() + sourceRect.maxX();
     450    ASSERT(originx <= scaledSourceRect.maxX());
     451
     452    int endx = scaledDestPoint.x() + scaledSourceRect.maxX();
     453    int logicalEndx = logicalDestPoint.x() + logicalSourceRect.maxX();
    404454    ASSERT(endx <= m_size.width());
    405455
    406456    int numColumns = endx - destx;
    407 
    408     int originy = sourceRect.y();
    409     int desty = destPoint.y() + sourceRect.y();
     457    int logicalNumColumns = logicalEndx - logicalDestx;
     458
     459    int originy = scaledSourceRect.y();
     460    int desty = scaledDestPoint.y() + scaledSourceRect.y();
     461    int logicalDesty = logicalDestPoint.y() + logicalSourceRect.y();
    410462    ASSERT(desty >= 0);
    411463    ASSERT(desty < m_size.height());
    412464    ASSERT(originy >= 0);
    413     ASSERT(originy <= sourceRect.maxY());
    414 
    415     int endy = destPoint.y() + sourceRect.maxY();
     465    ASSERT(originy <= scaledSourceRect.maxY());
     466
     467    int endy = scaledDestPoint.y() + scaledSourceRect.maxY();
     468    int logicalEndy = logicalDestPoint.y() + logicalSourceRect.maxY();
    416469    ASSERT(endy <= m_size.height());
    417470    int numRows = endy - desty;
    418 
     471    int logicalNumRows = logicalEndy - logicalDesty;
     472
     473    // The size of the derived surface is in BackingStoreCoordinateSystem.
     474    // We need to set the device scale for the derived surface from this ImageBuffer.
    419475    IntRect imageRect(destx, desty, numColumns, numRows);
    420476    RefPtr<cairo_surface_t> imageSurface = copySurfaceToImageAndAdjustRect(m_data.m_surface.get(), imageRect);
     477    cairoSurfaceSetDeviceScale(imageSurface.get(), m_resolutionScale, m_resolutionScale);
    421478    destx = imageRect.x();
    422479    desty = imageRect.y();
    … …  
    424481    unsigned char* pixelData = cairo_image_surface_get_data(imageSurface.get());
    425482
    426     unsigned srcBytesPerRow = 4 * sourceSize.width();
     483    unsigned srcBytesPerRow = 4 * scaledSourceSize.width();
    427484    int stride = cairo_image_surface_get_stride(imageSurface.get());
    428485
    … …  
    454511    }
    455512
    456     cairo_surface_mark_dirty_rectangle(imageSurface.get(), destx, desty, numColumns, numRows);
    457 
    458     if (imageSurface != m_data.m_surface.get())
    459         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);
     513    // This cairo surface operation is done in LogicalCoordinateSystem.
     514    cairo_surface_mark_dirty_rectangle(imageSurface.get(), logicalDestx, logicalDesty, logicalNumColumns, logicalNumRows);
     515
     516    if (imageSurface != m_data.m_surface.get()) {
     517        // This cairo surface operation is done in LogicalCoordinateSystem.
     518        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);
     519    }
    460520}
    461521
    … …  
    518578{
    519579#if ENABLE(ACCELERATED_2D_CANVAS)
     580    ASSERT_WITH_MESSAGE(m_resolutionScale == 1.0, "Since the HiDPI Canvas feature is removed, the resolution factor here is always 1.");
    520581    if (premultiplyAlpha || flipY)
    521582        return false;
Note: See TracChangeset for help on using the changeset viewer.