Attachment 345967 Details for Bug 188116 – Patch for Review

[patch] Patch for Review

188116-review.patch (text/plain), 16.70 KB, created by Said Abou-Hallawa on 2018-07-27 16:16:15 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Said Abou-Hallawa

Created: 2018-07-27 16:16:15 PDT

Size: 16.70 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 720ed5eeafa..e037bc34066 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2018-07-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
>+
>+        Renderer of large image has to be marked as waitingForAsyncDecoding when it is recorded into a DisplayList
>+        https://bugs.webkit.org/show_bug.cgi?id=188116
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This should fix rendering the bitmap images through DisplayLists when they
>+        are asynchronously decoded.
>+
>+        * html/ImageBitmap.cpp:
>+        * loader/cache/CachedImage.cpp:
>+        (WebCore::CachedImage::CachedImageObserver::addClientWaitingForAsyncDecoding):
>+        * loader/cache/CachedImage.h:
>+        * platform/graphics/GraphicsContext.cpp:
>+        (WebCore::GraphicsContext::drawImage):
>+        (WebCore::GraphicsContext::drawTiledImage):
>+        * platform/graphics/GraphicsContext.h:
>+        (WebCore::ImagePaintingOptions::ImagePaintingOptions):
>+        * platform/graphics/ImageObserver.h:
>+        * platform/graphics/cairo/ImageBufferCairo.cpp:
>+        (WebCore::ImageBuffer::draw):
>+        * rendering/RenderBoxModelObject.cpp:
>+        (WebCore::RenderBoxModelObject::paintFillLayerExtended):
>+        * rendering/RenderImage.cpp:
>+        (WebCore::RenderImage::paintIntoRect):
>+
> 2018-07-25  Said Abou-Hallawa  <sabouhallawa@apple.com>
> 
>         After replaying a DisplayList, the state of the drawing context has to be restored
>diff --git a/Source/WebCore/html/ImageBitmap.cpp b/Source/WebCore/html/ImageBitmap.cpp
>index 7de346707ac..3b4e1e13002 100644
>--- a/Source/WebCore/html/ImageBitmap.cpp
>+++ b/Source/WebCore/html/ImageBitmap.cpp
>@@ -503,6 +503,7 @@ public:
> 
>     void didDraw(const Image&) override { }
> 
>+    void addClientWaitingForAsyncDecoding(CachedImageClient&) override { }
>     bool canDestroyDecodedData(const Image&) override { return true; }
>     void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* = nullptr, DecodingStatus = DecodingStatus::Invalid) override { }
>     void changedInRect(const Image&, const IntRect* = nullptr) override { }
>diff --git a/Source/WebCore/loader/cache/CachedImage.cpp b/Source/WebCore/loader/cache/CachedImage.cpp
>index 23761a603a7..b3111cad2ac 100644
>--- a/Source/WebCore/loader/cache/CachedImage.cpp
>+++ b/Source/WebCore/loader/cache/CachedImage.cpp
>@@ -383,6 +383,12 @@ void CachedImage::CachedImageObserver::didDraw(const Image& image)
>         cachedImage->didDraw(image);
> }
> 
>+void CachedImage::CachedImageObserver::addClientWaitingForAsyncDecoding(CachedImageClient& client)
>+{
>+    for (auto cachedImage : m_cachedImages)
>+        cachedImage->addClientWaitingForAsyncDecoding(client);
>+}
>+
> bool CachedImage::CachedImageObserver::canDestroyDecodedData(const Image& image)
> {
>     for (auto cachedImage : m_cachedImages) {
>diff --git a/Source/WebCore/loader/cache/CachedImage.h b/Source/WebCore/loader/cache/CachedImage.h
>index 701851f987b..0529f87a403 100644
>--- a/Source/WebCore/loader/cache/CachedImage.h
>+++ b/Source/WebCore/loader/cache/CachedImage.h
>@@ -146,6 +146,7 @@ private:
>         void decodedSizeChanged(const Image&, long long delta) final;
>         void didDraw(const Image&) final;
> 
>+        void addClientWaitingForAsyncDecoding(CachedImageClient&) final;
>         bool canDestroyDecodedData(const Image&) final;
>         void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* changeRect = nullptr, DecodingStatus = DecodingStatus::Invalid) final;
>         void changedInRect(const Image&, const IntRect*) final;
>diff --git a/Source/WebCore/platform/graphics/GraphicsContext.cpp b/Source/WebCore/platform/graphics/GraphicsContext.cpp
>index 85efb7400b2..2500cdfb3e6 100644
>--- a/Source/WebCore/platform/graphics/GraphicsContext.cpp
>+++ b/Source/WebCore/platform/graphics/GraphicsContext.cpp
>@@ -717,44 +717,70 @@ ImageDrawResult GraphicsContext::drawImage(Image& image, const FloatRect& destin
> 
> ImageDrawResult GraphicsContext::drawImage(Image& image, const FloatRect& destination, const FloatRect& source, const ImagePaintingOptions& imagePaintingOptions)
> {
>+    ImageDrawResult result = ImageDrawResult::DidNothing;
>     if (paintingDisabled())
>-        return ImageDrawResult::DidNothing;
>+        return result;
> 
>     if (m_impl)
>-        return m_impl->drawImage(image, destination, source, imagePaintingOptions);
>+        result = m_impl->drawImage(image, destination, source, imagePaintingOptions);
>+    else {
>+        InterpolationQualityMaintainer interpolationQualityForThisScope(*this, imagePaintingOptions.m_interpolationQuality);
>+        result = image.draw(*this, destination, source, imagePaintingOptions.m_compositeOperator, imagePaintingOptions.m_blendMode, imagePaintingOptions.m_decodingMode, imagePaintingOptions.m_orientationDescription);
>+    }
> 
>-    InterpolationQualityMaintainer interpolationQualityForThisScope(*this, imagePaintingOptions.m_interpolationQuality);
>-    return image.draw(*this, destination, source, imagePaintingOptions.m_compositeOperator, imagePaintingOptions.m_blendMode, imagePaintingOptions.m_decodingMode, imagePaintingOptions.m_orientationDescription);
>+    if (result == ImageDrawResult::DidRequestDecoding && image.imageObserver()) {
>+        ASSERT(imagePaintingOptions.m_client);
>+        image.imageObserver()->addClientWaitingForAsyncDecoding(*imagePaintingOptions.m_client);
>+    }
>+
>+    return result;
> }
> 
> ImageDrawResult GraphicsContext::drawTiledImage(Image& image, const FloatRect& destination, const FloatPoint& source, const FloatSize& tileSize, const FloatSize& spacing, const ImagePaintingOptions& imagePaintingOptions)
> {
>+    ImageDrawResult result = ImageDrawResult::DidNothing;
>     if (paintingDisabled())
>-        return ImageDrawResult::DidNothing;
>+        return result;
> 
>     if (m_impl)
>-        return m_impl->drawTiledImage(image, destination, source, tileSize, spacing, imagePaintingOptions);
>+        result = m_impl->drawTiledImage(image, destination, source, tileSize, spacing, imagePaintingOptions);
>+    else {
>+        InterpolationQualityMaintainer interpolationQualityForThisScope(*this, imagePaintingOptions.m_interpolationQuality);
>+        result = image.drawTiled(*this, destination, source, tileSize, spacing, imagePaintingOptions.m_compositeOperator, imagePaintingOptions.m_blendMode, imagePaintingOptions.m_decodingMode);
>+    }
> 
>-    InterpolationQualityMaintainer interpolationQualityForThisScope(*this, imagePaintingOptions.m_interpolationQuality);
>-    return image.drawTiled(*this, destination, source, tileSize, spacing, imagePaintingOptions.m_compositeOperator, imagePaintingOptions.m_blendMode, imagePaintingOptions.m_decodingMode);
>+    if (result == ImageDrawResult::DidRequestDecoding && image.imageObserver()) {
>+        ASSERT(imagePaintingOptions.m_client);
>+        image.imageObserver()->addClientWaitingForAsyncDecoding(*imagePaintingOptions.m_client);
>+    }
>+
>+    return result;
> }
> 
> ImageDrawResult GraphicsContext::drawTiledImage(Image& image, const FloatRect& destination, const FloatRect& source, const FloatSize& tileScaleFactor,
>     Image::TileRule hRule, Image::TileRule vRule, const ImagePaintingOptions& imagePaintingOptions)
> {
>+    // Just do a scale.
>+    if (hRule == Image::StretchTile && vRule == Image::StretchTile)
>+        return drawImage(image, destination, source, imagePaintingOptions);
>+
>+    ImageDrawResult result = ImageDrawResult::DidNothing;
>     if (paintingDisabled())
>-        return ImageDrawResult::DidNothing;
>+        return result;
> 
>     if (m_impl)
>-        return m_impl->drawTiledImage(image, destination, source, tileScaleFactor, hRule, vRule, imagePaintingOptions);
>-
>-    if (hRule == Image::StretchTile && vRule == Image::StretchTile) {
>-        // Just do a scale.
>-        return drawImage(image, destination, source, imagePaintingOptions);
>+        result = m_impl->drawTiledImage(image, destination, source, tileScaleFactor, hRule, vRule, imagePaintingOptions);
>+    else {
>+        InterpolationQualityMaintainer interpolationQualityForThisScope(*this, imagePaintingOptions.m_interpolationQuality);
>+        result = image.drawTiled(*this, destination, source, tileScaleFactor, hRule, vRule, imagePaintingOptions.m_compositeOperator);
>     }
>-
>-    InterpolationQualityMaintainer interpolationQualityForThisScope(*this, imagePaintingOptions.m_interpolationQuality);
>-    return image.drawTiled(*this, destination, source, tileScaleFactor, hRule, vRule, imagePaintingOptions.m_compositeOperator);
>+    
>+    if (result == ImageDrawResult::DidRequestDecoding && image.imageObserver()) {
>+        ASSERT(imagePaintingOptions.m_client);
>+        image.imageObserver()->addClientWaitingForAsyncDecoding(*imagePaintingOptions.m_client);
>+    }
>+    
>+    return result;
> }
> 
> void GraphicsContext::drawImageBuffer(ImageBuffer& image, const FloatPoint& destination, const ImagePaintingOptions& imagePaintingOptions)
>diff --git a/Source/WebCore/platform/graphics/GraphicsContext.h b/Source/WebCore/platform/graphics/GraphicsContext.h
>index c44189ab26b..3f1cdc4725d 100644
>--- a/Source/WebCore/platform/graphics/GraphicsContext.h
>+++ b/Source/WebCore/platform/graphics/GraphicsContext.h
>@@ -80,6 +80,7 @@ const int cMisspellingLinePatternWidth = 4;
> const int cMisspellingLinePatternGapWidth = 1;
> 
> class AffineTransform;
>+class CachedImageClient;
> class FloatRoundedRect;
> class Gradient;
> class GraphicsContext3D;
>@@ -210,28 +211,31 @@ struct GraphicsContextState {
> };
> 
> struct ImagePaintingOptions {
>-    ImagePaintingOptions(CompositeOperator compositeOperator = CompositeSourceOver, BlendMode blendMode = BlendModeNormal, DecodingMode decodingMode = DecodingMode::Synchronous, ImageOrientationDescription orientationDescription = ImageOrientationDescription(), InterpolationQuality interpolationQuality = InterpolationDefault)
>+    ImagePaintingOptions(CompositeOperator compositeOperator = CompositeSourceOver, BlendMode blendMode = BlendModeNormal, DecodingMode decodingMode = DecodingMode::Synchronous, CachedImageClient* client = nullptr, ImageOrientationDescription orientationDescription = ImageOrientationDescription(), InterpolationQuality interpolationQuality = InterpolationDefault)
>         : m_compositeOperator(compositeOperator)
>         , m_blendMode(blendMode)
>         , m_decodingMode(decodingMode)
>+        , m_client(client)
>         , m_orientationDescription(orientationDescription)
>         , m_interpolationQuality(interpolationQuality)
>     {
>     }
> 
>-    ImagePaintingOptions(ImageOrientationDescription orientationDescription, InterpolationQuality interpolationQuality = InterpolationDefault, CompositeOperator compositeOperator = CompositeSourceOver, BlendMode blendMode = BlendModeNormal, DecodingMode decodingMode = DecodingMode::Synchronous)
>+    ImagePaintingOptions(ImageOrientationDescription orientationDescription, InterpolationQuality interpolationQuality = InterpolationDefault, CompositeOperator compositeOperator = CompositeSourceOver, BlendMode blendMode = BlendModeNormal, DecodingMode decodingMode = DecodingMode::Synchronous, CachedImageClient* client = nullptr)
>         : m_compositeOperator(compositeOperator)
>         , m_blendMode(blendMode)
>         , m_decodingMode(decodingMode)
>+        , m_client(client)
>         , m_orientationDescription(orientationDescription)
>         , m_interpolationQuality(interpolationQuality)
>     {
>     }
> 
>-    ImagePaintingOptions(InterpolationQuality interpolationQuality, ImageOrientationDescription orientationDescription = ImageOrientationDescription(), CompositeOperator compositeOperator = CompositeSourceOver, BlendMode blendMode = BlendModeNormal, DecodingMode decodingMode = DecodingMode::Synchronous)
>+    ImagePaintingOptions(InterpolationQuality interpolationQuality, ImageOrientationDescription orientationDescription = ImageOrientationDescription(), CompositeOperator compositeOperator = CompositeSourceOver, BlendMode blendMode = BlendModeNormal, DecodingMode decodingMode = DecodingMode::Synchronous, CachedImageClient* client = nullptr)
>         : m_compositeOperator(compositeOperator)
>         , m_blendMode(blendMode)
>         , m_decodingMode(decodingMode)
>+        , m_client(client)
>         , m_orientationDescription(orientationDescription)
>         , m_interpolationQuality(interpolationQuality)
>     {
>@@ -242,6 +246,7 @@ struct ImagePaintingOptions {
>     CompositeOperator m_compositeOperator;
>     BlendMode m_blendMode;
>     DecodingMode m_decodingMode;
>+    CachedImageClient* m_client;
>     ImageOrientationDescription m_orientationDescription;
>     InterpolationQuality m_interpolationQuality;
> };
>diff --git a/Source/WebCore/platform/graphics/ImageObserver.h b/Source/WebCore/platform/graphics/ImageObserver.h
>index 8269cc4edc3..a026b3394e5 100644
>--- a/Source/WebCore/platform/graphics/ImageObserver.h
>+++ b/Source/WebCore/platform/graphics/ImageObserver.h
>@@ -30,6 +30,7 @@
> 
> namespace WebCore {
> 
>+class CachedImageClient;
> class Image;
> class IntRect;
> class URL;
>@@ -48,6 +49,7 @@ public:
> 
>     virtual void didDraw(const Image&) = 0;
> 
>+    virtual void addClientWaitingForAsyncDecoding(CachedImageClient&) = 0;
>     virtual bool canDestroyDecodedData(const Image&) = 0;
>     virtual void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* changeRect = nullptr, DecodingStatus = DecodingStatus::Invalid) = 0;
>     virtual void changedInRect(const Image&, const IntRect* changeRect = nullptr) = 0;
>diff --git a/Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp b/Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp
>index f2ea18d6cfd..4ca042353dc 100644
>--- a/Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp
>+++ b/Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp
>@@ -340,7 +340,7 @@ void ImageBuffer::draw(GraphicsContext& destinationContext, const FloatRect& des
> {
>     BackingStoreCopy copyMode = &destinationContext == &context() ? CopyBackingStore : DontCopyBackingStore;
>     RefPtr<Image> image = copyImage(copyMode);
>-    destinationContext.drawImage(*image, destRect, srcRect, ImagePaintingOptions(op, blendMode, DecodingMode::Synchronous, ImageOrientationDescription()));
>+    destinationContext.drawImage(*image, destRect, srcRect, ImagePaintingOptions(op, blendMode));
> }
> 
> void ImageBuffer::drawPattern(GraphicsContext& context, const FloatRect& destRect, const FloatRect& srcRect, const AffineTransform& patternTransform,
>diff --git a/Source/WebCore/rendering/RenderBoxModelObject.cpp b/Source/WebCore/rendering/RenderBoxModelObject.cpp
>index 70c228f5b67..ebf2d7441f9 100644
>--- a/Source/WebCore/rendering/RenderBoxModelObject.cpp
>+++ b/Source/WebCore/rendering/RenderBoxModelObject.cpp
>@@ -981,11 +981,7 @@ void RenderBoxModelObject::paintFillLayerExtended(const PaintInfo& paintInfo, co
> 
>             auto interpolation = chooseInterpolationQuality(context, *image, &bgLayer, geometry.tileSize());
>             auto decodingMode = decodingModeForImageDraw(*image, paintInfo);
>-            auto drawResult = context.drawTiledImage(*image, geometry.destRect(), toLayoutPoint(geometry.relativePhase()), geometry.tileSize(), geometry.spaceSize(), ImagePaintingOptions(compositeOp, bgLayer.blendMode(), decodingMode, ImageOrientationDescription(), interpolation));
>-            if (drawResult == ImageDrawResult::DidRequestDecoding) {
>-                ASSERT(bgImage->isCachedImage());
>-                bgImage->cachedImage()->addClientWaitingForAsyncDecoding(*this);
>-            }
>+            context.drawTiledImage(*image, geometry.destRect(), toLayoutPoint(geometry.relativePhase()), geometry.tileSize(), geometry.spaceSize(), ImagePaintingOptions(compositeOp, bgLayer.blendMode(), decodingMode, this, ImageOrientationDescription(), interpolation));
>         }
>     }
> 
>diff --git a/Source/WebCore/rendering/RenderImage.cpp b/Source/WebCore/rendering/RenderImage.cpp
>index 973fa012038..fdb98edf902 100644
>--- a/Source/WebCore/rendering/RenderImage.cpp
>+++ b/Source/WebCore/rendering/RenderImage.cpp
>@@ -617,9 +617,7 @@ ImageDrawResult RenderImage::paintIntoRect(PaintInfo& paintInfo, const FloatRect
> 
>     ImageOrientationDescription orientationDescription(shouldRespectImageOrientation(), style().imageOrientation());
>     auto decodingMode = decodingModeForImageDraw(*image, paintInfo);
>-    auto drawResult = paintInfo.context().drawImage(*img, rect, ImagePaintingOptions(compositeOperator, BlendModeNormal, decodingMode, orientationDescription, interpolation));
>-    if (drawResult == ImageDrawResult::DidRequestDecoding)
>-        imageResource().cachedImage()->addClientWaitingForAsyncDecoding(*this);
>+    auto drawResult = paintInfo.context().drawImage(*img, rect, ImagePaintingOptions(compositeOperator, BlendModeNormal, decodingMode, this, orientationDescription, interpolation));
> 
> #if USE(SYSTEM_PREVIEW)
>     if (imageElement && imageElement->isSystemPreviewImage() && drawResult == ImageDrawResult::DidDraw && RuntimeEnabledFeatures::sharedFeatures().systemPreviewEnabled())

Actions: View | Formatted Diff | Diff

Attachments on bug 188116: 345936 | 345944 | 345967 | 345990