Changeset 216901 in webkit


Ignore:
Timestamp:
May 15, 2017 10:14:50 PM (7 years ago)
Author:
commit-queue@webkit.org
Message:

Do not delete asynchronously decoded frames for large images if their clients are in the viewport
​https://bugs.webkit.org/show_bug.cgi?id=170640

Patch by Said Abou-Hallawa <​sabouhallawa@apple.com> on 2017-05-15
Reviewed by Simon Fraser.

Source/WebCore:

The image flickering problem happens when a large image is visible in the
view port and for some reason, the decoded frame gets destroyed. When this
image is repainted, BitmapImage::draw() does not find a valid decoded frame
for that image. It then requests an async decoding for the image and just
draws nothing in the image rectangle. Drawing no content between two drawing
phases in which the image is drawn causes the unwanted flickering.

To fix this issue we need to protect the decoded frames of all the images
in the view port from being destroyed. When BitmapImage::destroyDecodedData()
is called, it is going to check, through the ImageObserver, whether any
of its clients is visible. And if so, the current decoded frame won't be
destroyed.

Tests: Modifying existing tests.

  • loader/cache/CachedImage.cpp:

(WebCore::CachedImage::CachedImageObserver::decodedSizeChanged):
(WebCore::CachedImage::CachedImageObserver::didDraw):
(WebCore::CachedImage::CachedImageObserver::canDestroyDecodedData):
(WebCore::CachedImage::CachedImageObserver::imageFrameAvailable):
(WebCore::CachedImage::CachedImageObserver::changedInRect):
(WebCore::CachedImage::decodedSizeChanged):
(WebCore::CachedImage::didDraw):
(WebCore::CachedImage::canDestroyDecodedData): Finds out whether it's okay
to discard the image decoded data or not.
(WebCore::CachedImage::imageFrameAvailable):
(WebCore::CachedImage::changedInRect):

  • loader/cache/CachedImage.h:
  • loader/cache/CachedImageClient.h:

(WebCore::CachedImageClient::canDestroyDecodedData):

  • loader/cache/MemoryCache.cpp:

(WebCore::MemoryCache::destroyDecodedDataForAllImages): This function is
currently not used. Use in the internal destroyDecodedDataForAllImages()
but unlike what CachedImage::destroyDecodedData() does, make it destroy
the decoded frames without deleting the image itself.

  • loader/cache/MemoryCache.h:
  • platform/graphics/BitmapImage.cpp:

(WebCore::BitmapImage::destroyDecodedData):
(WebCore::BitmapImage::draw):
(WebCore::BitmapImage::canDestroyCurrentFrameDecodedData):
(WebCore::BitmapImage::advanceAnimation):
(WebCore::BitmapImage::internalAdvanceAnimation):
(WebCore::BitmapImage::imageFrameAvailableAtIndex):

  • platform/graphics/BitmapImage.h:
  • platform/graphics/GraphicsContext3D.cpp:

(WebCore::GraphicsContext3D::packImageData):

  • platform/graphics/ImageFrameCache.cpp:

(WebCore::ImageFrameCache::decodedSizeChanged):
(ImageFrameCache::cacheAsyncFrameNativeImageAtIndex): The assertion in this
function is wrong. frameIsCompleteAtIndex() can be false when the an image
decoding is requested but can be true when the decoding finishes.

  • platform/graphics/ImageObserver.h:
  • platform/graphics/cairo/ImageCairo.cpp:

(WebCore::Image::drawPattern):

  • platform/graphics/cg/ImageCG.cpp:

(WebCore::Image::drawPattern):

  • platform/graphics/cg/ImageDecoderCG.cpp:

(WebCore::ImageDecoder::frameIsCompleteAtIndex):

  • platform/graphics/cg/PDFDocumentImage.cpp:

(WebCore::PDFDocumentImage::decodedSizeChanged):
(WebCore::PDFDocumentImage::draw):

  • platform/graphics/texmap/TextureMapperTiledBackingStore.cpp:

(WebCore::TextureMapperTiledBackingStore::updateContentsFromImageIfNeeded):

  • platform/graphics/win/ImageDirect2D.cpp:

(WebCore::Image::drawPattern):

  • rendering/RenderElement.cpp:

(WebCore::RenderElement::isVisibleInDocumentRect):
(WebCore::RenderElement::isVisibleInViewport):
(WebCore::RenderElement::imageFrameAvailable):
(WebCore::RenderElement::repaintForPausedImageAnimationsIfNeeded):
(WebCore::RenderElement::shouldRepaintInVisibleRect): Deleted. Function
is renamed to isVisibleInViewport() for better readability.

  • rendering/RenderElement.h:
  • svg/graphics/SVGImage.cpp:

(WebCore::SVGImage::draw):

  • svg/graphics/SVGImageClients.h:
  • testing/Internals.cpp:

(WebCore::Internals::destroyDecodedDataForAllImages):

  • testing/Internals.h:
  • testing/Internals.idl:

Source/WebKit/mac:

Enable the async decoding for large images.

  • WebView/WebView.mm:

(-[WebView _preferencesChanged:]):

Source/WebKit2:

Enable the async decoding for large images.

  • WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::updatePreferences):

LayoutTests:

  • fast/images/async-image-background-image-repeated.html:
  • fast/images/async-image-background-image.html:
  • fast/images/sprite-sheet-image-draw.html:
Location:
trunk
Files:
32 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r216892 r216901  
     12017-05-15  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        Do not delete asynchronously decoded frames for large images if their clients are in the viewport
     4        https://bugs.webkit.org/show_bug.cgi?id=170640
     5
     6        Reviewed by Simon Fraser.
     7
     8        * fast/images/async-image-background-image-repeated.html:
     9        * fast/images/async-image-background-image.html:
     10        * fast/images/sprite-sheet-image-draw.html:
     11
    1122017-05-15  Youenn Fablet  <youenn@apple.com>
    213

  • trunk/LayoutTests/fast/images/async-image-background-image-repeated.html

    r216450 r216901  
    5858 
    5959                    Promise.all(promises).then(() => {
     60                        // Ensure internals.destroyDecodedDataForAllImages() will not destroy
     61                        // the images' decoded data because it is inside the viewport.
     62                        internals.destroyDecodedDataForAllImages();
    6063                        testRunner.notifyDone();
    6164                    });

  • trunk/LayoutTests/fast/images/async-image-background-image.html

    r216225 r216901  
    4545                    // Wait for the image frame to finish decoding before finishing the test.
    4646                    element.addEventListener("webkitImageFrameReady", function() {
     47                        // Ensure internals.destroyDecodedDataForAllImages() will not destroy
     48                        // the image's decoded data because it is inside the viewport.
     49                        internals.destroyDecodedDataForAllImages();
    4750                        testRunner.notifyDone();
    4851                    }, false);

  • trunk/LayoutTests/fast/images/sprite-sheet-image-draw.html

    r216450 r216901  
    3838                    // Wait for the image frame to finish decoding before finishing the test.
    3939                    element.addEventListener("webkitImageFrameReady", function() {
     40                        // Ensure internals.destroyDecodedDataForAllImages() will not destroy
     41                        // the image's decoded data because it is inside the viewport.
     42                        internals.destroyDecodedDataForAllImages();
    4043                        testRunner.notifyDone();
    4144                    }, false);

  • trunk/Source/WebCore/ChangeLog

    r216898 r216901  
     12017-05-15  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        Do not delete asynchronously decoded frames for large images if their clients are in the viewport
     4        https://bugs.webkit.org/show_bug.cgi?id=170640
     5
     6        Reviewed by Simon Fraser.
     7
     8        The image flickering problem happens when a large image is visible in the
     9        view port and for some reason, the decoded frame gets destroyed. When this
     10        image is repainted, BitmapImage::draw() does not find a valid decoded frame
     11        for that image. It then requests an async decoding for the image and just
     12        draws nothing in the image rectangle. Drawing no content between two drawing
     13        phases in which the image is drawn causes the unwanted flickering.
     14
     15        To fix this issue we need to protect the decoded frames of all the images
     16        in the view port from being destroyed. When BitmapImage::destroyDecodedData()
     17        is called, it is going to check, through the ImageObserver, whether any
     18        of its clients is visible. And if so, the current decoded frame won't be
     19        destroyed.
     20
     21        Tests: Modifying existing tests.
     22
     23        * loader/cache/CachedImage.cpp:
     24        (WebCore::CachedImage::CachedImageObserver::decodedSizeChanged):
     25        (WebCore::CachedImage::CachedImageObserver::didDraw):
     26        (WebCore::CachedImage::CachedImageObserver::canDestroyDecodedData):
     27        (WebCore::CachedImage::CachedImageObserver::imageFrameAvailable):
     28        (WebCore::CachedImage::CachedImageObserver::changedInRect):
     29        (WebCore::CachedImage::decodedSizeChanged):
     30        (WebCore::CachedImage::didDraw):
     31        (WebCore::CachedImage::canDestroyDecodedData): Finds out whether it's okay
     32        to discard the image decoded data or not.
     33        (WebCore::CachedImage::imageFrameAvailable):
     34        (WebCore::CachedImage::changedInRect):
     35        * loader/cache/CachedImage.h:
     36        * loader/cache/CachedImageClient.h:
     37        (WebCore::CachedImageClient::canDestroyDecodedData):
     38        * loader/cache/MemoryCache.cpp:
     39        (WebCore::MemoryCache::destroyDecodedDataForAllImages): This function is
     40        currently not used. Use in the internal destroyDecodedDataForAllImages()
     41        but unlike what CachedImage::destroyDecodedData() does, make it destroy
     42        the decoded frames without deleting the image itself.
     43        * loader/cache/MemoryCache.h:
     44        * platform/graphics/BitmapImage.cpp:
     45        (WebCore::BitmapImage::destroyDecodedData):
     46        (WebCore::BitmapImage::draw):
     47        (WebCore::BitmapImage::canDestroyCurrentFrameDecodedData):
     48        (WebCore::BitmapImage::advanceAnimation):
     49        (WebCore::BitmapImage::internalAdvanceAnimation):
     50        (WebCore::BitmapImage::imageFrameAvailableAtIndex):
     51        * platform/graphics/BitmapImage.h:
     52        * platform/graphics/GraphicsContext3D.cpp:
     53        (WebCore::GraphicsContext3D::packImageData):
     54        * platform/graphics/ImageFrameCache.cpp:
     55        (WebCore::ImageFrameCache::decodedSizeChanged):
     56        (ImageFrameCache::cacheAsyncFrameNativeImageAtIndex): The assertion in this
     57        function is wrong. frameIsCompleteAtIndex() can be false when the an image
     58        decoding is requested but can be true when the decoding finishes.
     59        * platform/graphics/ImageObserver.h:
     60        * platform/graphics/cairo/ImageCairo.cpp:
     61        (WebCore::Image::drawPattern):
     62        * platform/graphics/cg/ImageCG.cpp:
     63        (WebCore::Image::drawPattern):
     64        * platform/graphics/cg/ImageDecoderCG.cpp:
     65        (WebCore::ImageDecoder::frameIsCompleteAtIndex):
     66        * platform/graphics/cg/PDFDocumentImage.cpp:
     67        (WebCore::PDFDocumentImage::decodedSizeChanged):
     68        (WebCore::PDFDocumentImage::draw):
     69        * platform/graphics/texmap/TextureMapperTiledBackingStore.cpp:
     70        (WebCore::TextureMapperTiledBackingStore::updateContentsFromImageIfNeeded):
     71        * platform/graphics/win/ImageDirect2D.cpp:
     72        (WebCore::Image::drawPattern):
     73        * rendering/RenderElement.cpp:
     74        (WebCore::RenderElement::isVisibleInDocumentRect):
     75        (WebCore::RenderElement::isVisibleInViewport):
     76        (WebCore::RenderElement::imageFrameAvailable):
     77        (WebCore::RenderElement::repaintForPausedImageAnimationsIfNeeded):
     78        (WebCore::RenderElement::shouldRepaintInVisibleRect): Deleted. Function
     79        is renamed to isVisibleInViewport() for better readability.
     80        * rendering/RenderElement.h:
     81        * svg/graphics/SVGImage.cpp:
     82        (WebCore::SVGImage::draw):
     83        * svg/graphics/SVGImageClients.h:
     84        * testing/Internals.cpp:
     85        (WebCore::Internals::destroyDecodedDataForAllImages):
     86        * testing/Internals.h:
     87        * testing/Internals.idl:
     88
    1892017-05-15  Youenn Fablet  <youenn@apple.com>
    290

  • trunk/Source/WebCore/loader/cache/CachedImage.cpp

    r216702 r216901  
    331331}
    332332
    333 void CachedImage::CachedImageObserver::decodedSizeChanged(const Image* image, long long delta)
     333void CachedImage::CachedImageObserver::decodedSizeChanged(const Image& image, long long delta)
    334334{
    335335    for (auto cachedImage : m_cachedImages)
    … …  
    337337}
    338338
    339 void CachedImage::CachedImageObserver::didDraw(const Image* image)
     339void CachedImage::CachedImageObserver::didDraw(const Image& image)
    340340{
    341341    for (auto cachedImage : m_cachedImages)
    … …  
    343343}
    344344
    345 void CachedImage::CachedImageObserver::imageFrameAvailable(const Image* image, ImageAnimatingState animatingState, const IntRect* changeRect)
     345bool CachedImage::CachedImageObserver::canDestroyDecodedData(const Image& image)
     346{
     347    for (auto cachedImage : m_cachedImages) {
     348        if (&image != cachedImage->image())
     349            continue;
     350        if (!cachedImage->canDestroyDecodedData(image))
     351            return false;
     352    }
     353    return true;
     354}
     355
     356void CachedImage::CachedImageObserver::imageFrameAvailable(const Image& image, ImageAnimatingState animatingState, const IntRect* changeRect)
    346357{
    347358    for (auto cachedImage : m_cachedImages)
    … …  
    349360}
    350361
    351 void CachedImage::CachedImageObserver::changedInRect(const Image* image, const IntRect* rect)
     362void CachedImage::CachedImageObserver::changedInRect(const Image& image, const IntRect* rect)
    352363{
    353364    for (auto cachedImage : m_cachedImages)
    … …  
    477488}
    478489
    479 void CachedImage::decodedSizeChanged(const Image* image, long long delta)
    480 {
    481     if (!image || image != m_image)
     490void CachedImage::decodedSizeChanged(const Image& image, long long delta)
     491{
     492    if (&image != m_image)
    482493        return;
    483494
    … …  
    486497}
    487498
    488 void CachedImage::didDraw(const Image* image)
    489 {
    490     if (!image || image != m_image)
     499void CachedImage::didDraw(const Image& image)
     500{
     501    if (&image != m_image)
    491502        return;
    492503   
    … …  
    498509}
    499510
    500 void CachedImage::imageFrameAvailable(const Image* image, ImageAnimatingState animatingState, const IntRect* changeRect)
    501 {
    502     if (!image || image != m_image)
     511bool CachedImage::canDestroyDecodedData(const Image& image)
     512{
     513    if (&image != m_image)
     514        return false;
     515
     516    CachedResourceClientWalker<CachedImageClient> clientWalker(m_clients);
     517    while (CachedImageClient* client = clientWalker.next()) {
     518        if (!client->canDestroyDecodedData())
     519            return false;
     520    }
     521
     522    return true;
     523}
     524
     525void CachedImage::imageFrameAvailable(const Image& image, ImageAnimatingState animatingState, const IntRect* changeRect)
     526{
     527    if (&image != m_image)
    503528        return;
    504529
    … …  
    515540}
    516541
    517 void CachedImage::changedInRect(const Image* image, const IntRect* rect)
    518 {
    519     if (!image || image != m_image)
     542void CachedImage::changedInRect(const Image& image, const IntRect* rect)
     543{
     544    if (&image != m_image)
    520545        return;
    521546    notifyObservers(rect);

  • trunk/Source/WebCore/loader/cache/CachedImage.h

    r215952 r216901  
    128128
    129129        // ImageObserver API
    130         URL sourceUrl() const override { return m_cachedImages[0]->url(); }
    131         void decodedSizeChanged(const Image*, long long delta) final;
    132         void didDraw(const Image*) final;
     130        URL sourceUrl() const override { return !m_cachedImages.isEmpty() ? m_cachedImages[0]->url() : URL(); }
     131        void decodedSizeChanged(const Image&, long long delta) final;
     132        void didDraw(const Image&) final;
    133133
    134         void imageFrameAvailable(const Image*, ImageAnimatingState, const IntRect* changeRect = nullptr) final;
    135         void changedInRect(const Image*, const IntRect*) final;
     134        bool canDestroyDecodedData(const Image&) final;
     135        void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* changeRect = nullptr) final;
     136        void changedInRect(const Image&, const IntRect*) final;
    136137
    137138        Vector<CachedImage*> m_cachedImages;
    138139    };
    139140
    140     void decodedSizeChanged(const Image*, long long delta);
    141     void didDraw(const Image*);
    142     void imageFrameAvailable(const Image*, ImageAnimatingState, const IntRect* changeRect = nullptr);
    143     void changedInRect(const Image*, const IntRect*);
     141    void decodedSizeChanged(const Image&, long long delta);
     142    void didDraw(const Image&);
     143    bool canDestroyDecodedData(const Image&);
     144    void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* changeRect = nullptr);
     145    void changedInRect(const Image&, const IntRect*);
    144146
    145147    void addIncrementalDataBuffer(SharedBuffer&);

  • trunk/Source/WebCore/loader/cache/CachedImageClient.h

    r215952 r216901  
    4343    virtual void imageChanged(CachedImage*, const IntRect* = nullptr) { }
    4444
    45     // Called when GIF animation progresses.
     45    virtual bool canDestroyDecodedData() { return true; }
     46
     47    // Called when a new decoded frame for a large image is available or when an animated image is ready to advance to the next frame.
    4648    virtual VisibleInViewportState imageFrameAvailable(CachedImage& image, ImageAnimatingState, const IntRect* changeRect) { imageChanged(&image, changeRect); return VisibleInViewportState::No; }
    4749

  • trunk/Source/WebCore/loader/cache/MemoryCache.cpp

    r215160 r216901  
    290290{
    291291    MemoryCache::singleton().forEachResource([](CachedResource& resource) {
    292         if (resource.isImage())
    293             resource.destroyDecodedData();
     292        if (!resource.isImage())
     293            return;
     294
     295        if (auto image = downcast<CachedImage>(resource).image())
     296            image->destroyDecodedData();
    294297    });
    295298}

  • trunk/Source/WebCore/loader/cache/MemoryCache.h

    r215152 r216901  
    105105    void forEachResource(const std::function<void(CachedResource&)>&);
    106106    void forEachSessionResource(SessionID, const std::function<void(CachedResource&)>&);
    107     void destroyDecodedDataForAllImages();
     107    WEBCORE_EXPORT void destroyDecodedDataForAllImages();
    108108
    109109    // Sets the cache's memory capacities, in bytes. These will hold only approximately,

  • trunk/Source/WebCore/platform/graphics/BitmapImage.cpp

    r216882 r216901  
    8080    if (!destroyAll)
    8181        m_source.destroyDecodedDataBeforeFrame(m_currentFrame);
    82     else if (m_source.hasAsyncDecodingQueue())
     82    else if (!canDestroyDecodedData()) {
    8383        m_source.destroyAllDecodedDataExcludeFrame(m_currentFrame);
    84     else {
     84        destroyAll = false;
     85    } else {
    8586        m_source.destroyAllDecodedData();
    8687        m_currentFrameDecodingStatus = ImageFrame::DecodingStatus::Invalid;
    … …  
    8990    // There's no need to throw away the decoder unless we're explicitly asked
    9091    // to destroy all of the frames.
    91     if (!destroyAll || m_source.hasAsyncDecodingQueue())
     92    if (!destroyAll)
    9293        m_source.clearFrameBufferCache(m_currentFrame);
    9394    else
    … …  
    247248
    248249    if (imageObserver())
    249         imageObserver()->didDraw(this);
     250        imageObserver()->didDraw(*this);
    250251}
    251252
    … …  
    314315    m_frameTimer = std::make_unique<Timer>(*this, &BitmapImage::advanceAnimation);
    315316    m_frameTimer->startOneShot(delay);
     317}
     318
     319bool BitmapImage::canDestroyDecodedData()
     320{
     321    // Animated images should preserve the current frame till the next one finishes decoding.
     322    if (m_source.hasAsyncDecodingQueue())
     323        return false;
     324
     325    // Small image should be decoded synchronously. Deleting its decoded frame is fine.
     326    if (!shouldUseAsyncDecodingForLargeImages())
     327        return true;
     328
     329    return !imageObserver() || imageObserver()->canDestroyDecodedData(*this);
    316330}
    317331
    … …  
    409423        // Force repaint if showDebugBackground() is on.
    410424        if (m_showDebugBackground)
    411             imageObserver()->changedInRect(this);
     425            imageObserver()->changedInRect(*this);
    412426        LOG(Images, "BitmapImage::%s - %p - url: %s [lateFrameCount = %ld nextFrame = %ld]", __FUNCTION__, this, sourceURL().string().utf8().data(), ++m_lateFrameCount, nextFrame);
    413427    }
    … …  
    424438        m_currentFrameDecodingStatus = frameDecodingStatusAtIndex(m_currentFrame);
    425439    if (imageObserver())
    426         imageObserver()->imageFrameAvailable(this, ImageAnimatingState::Yes);
     440        imageObserver()->imageFrameAvailable(*this, ImageAnimatingState::Yes);
    427441
    428442    LOG(Images, "BitmapImage::%s - %p - url: %s [m_currentFrame = %ld]", __FUNCTION__, this, sourceURL().string().utf8().data(), m_currentFrame);
    … …  
    475489            m_currentFrameDecodingStatus = frameDecodingStatusAtIndex(m_currentFrame);
    476490        if (imageObserver())
    477             imageObserver()->imageFrameAvailable(this, ImageAnimatingState::No);
     491            imageObserver()->imageFrameAvailable(*this, ImageAnimatingState::No);
    478492    }
    479493}

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

    r216882 r216901  
    192192    void clearTimer();
    193193    void startTimer(Seconds delay);
     194    bool canDestroyDecodedData();
    194195    bool isBitmapImage() const override { return true; }
    195196    void dump(TextStream&) const override;

  • trunk/Source/WebCore/platform/graphics/GraphicsContext3D.cpp

    r212651 r216901  
    369369bool GraphicsContext3D::packImageData(Image* image, const void* pixels, GC3Denum format, GC3Denum type, bool flipY, AlphaOp alphaOp, DataFormat sourceFormat, unsigned width, unsigned height, unsigned sourceUnpackAlignment, Vector<uint8_t>& data)
    370370{
    371     if (!pixels)
     371    if (!image || !pixels)
    372372        return false;
    373373
    … …  
    381381        return false;
    382382    if (ImageObserver* observer = image->imageObserver())
    383         observer->didDraw(image);
     383        observer->didDraw(*image);
    384384    return true;
    385385}

  • trunk/Source/WebCore/platform/graphics/ImageFrameCache.cpp

    r216882 r216901  
    125125        return;
    126126   
    127     m_image->imageObserver()->decodedSizeChanged(m_image, decodedSize);
     127    m_image->imageObserver()->decodedSizeChanged(*m_image, decodedSize);
    128128}
    129129

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

    r215952 r216901  
    4242public:
    4343    virtual URL sourceUrl() const = 0;
    44     virtual void decodedSizeChanged(const Image*, long long delta) = 0;
     44    virtual void decodedSizeChanged(const Image&, long long delta) = 0;
    4545
    46     virtual void didDraw(const Image*) = 0;
     46    virtual void didDraw(const Image&) = 0;
    4747
    48     virtual void imageFrameAvailable(const Image*, ImageAnimatingState, const IntRect* changeRect = nullptr) = 0;
    49     virtual void changedInRect(const Image*, const IntRect* changeRect = nullptr) = 0;
     48    virtual bool canDestroyDecodedData(const Image&) = 0;
     49    virtual void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* changeRect = nullptr) = 0;
     50    virtual void changedInRect(const Image&, const IntRect* changeRect = nullptr) = 0;
    5051};
    5152

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

    r206631 r216901  
    4444
    4545    if (imageObserver())
    46         imageObserver()->didDraw(this);
     46        imageObserver()->didDraw(*this);
    4747}
    4848

  • trunk/Source/WebCore/platform/graphics/cg/ImageCG.cpp

    r206631 r216901  
    4444
    4545    if (imageObserver())
    46         imageObserver()->didDraw(this);
     46        imageObserver()->didDraw(*this);
    4747}
    4848

  • trunk/Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp

    r216450 r216901  
    308308{
    309309    ASSERT(frameCount());
     310    // CGImageSourceGetStatusAtIndex() changes the return status value from kCGImageStatusIncomplete
     311    // to kCGImageStatusComplete only if (index > 1 && index < frameCount() - 1). To get an accurate
     312    // result for the last frame (or the single frame of the static image) use CGImageSourceGetStatus()
     313    // instead for this frame.
     314    if (index == frameCount() - 1)
     315        return CGImageSourceGetStatus(m_nativeDecoder.get()) == kCGImageStatusComplete;
    310316    return CGImageSourceGetStatusAtIndex(m_nativeDecoder.get(), index) == kCGImageStatusComplete;
    311317}

  • trunk/Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp

    r215211 r216901  
    184184
    185185    if (imageObserver())
    186         imageObserver()->decodedSizeChanged(this, -static_cast<long long>(m_cachedBytes) + newCachedBytes);
     186        imageObserver()->decodedSizeChanged(*this, -static_cast<long long>(m_cachedBytes) + newCachedBytes);
    187187
    188188    ASSERT(s_allDecodedDataSize >= m_cachedBytes);
    … …  
    289289
    290290    if (imageObserver())
    291         imageObserver()->didDraw(this);
     291        imageObserver()->didDraw(*this);
    292292}
    293293

  • trunk/Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.cpp

    r198735 r216901  
    3838
    3939    if (m_image->imageObserver())
    40         m_image->imageObserver()->didDraw(m_image.get());
     40        m_image->imageObserver()->didDraw(*m_image);
    4141    m_image = nullptr;
    4242}

  • trunk/Source/WebCore/platform/graphics/win/ImageDirect2D.cpp

    r207357 r216901  
    102102
    103103    if (imageObserver())
    104         imageObserver()->didDraw(this);
     104        imageObserver()->didDraw(*this);
    105105}
    106106

  • trunk/Source/WebCore/rendering/RenderElement.cpp

    r216631 r216901  
    14331433}
    14341434
    1435 bool RenderElement::shouldRepaintInVisibleRect(const IntRect& visibleRect) const
     1435bool RenderElement::isVisibleInDocumentRect(const IntRect& documentRect) const
    14361436{
    14371437    if (document().activeDOMObjectsAreSuspended())
    … …  
    14571457
    14581458    LayoutRect backgroundPaintingRect = backgroundIsPaintedByRoot ? view().backgroundRect() : absoluteClippedOverflowRect();
    1459     if (!visibleRect.intersects(enclosingIntRect(backgroundPaintingRect)))
     1459    if (!documentRect.intersects(enclosingIntRect(backgroundPaintingRect)))
    14601460        return false;
    14611461
    … …  
    14941494}
    14951495
    1496 VisibleInViewportState RenderElement::imageFrameAvailable(CachedImage& image, ImageAnimatingState animatingState, const IntRect* changeRect)
     1496bool RenderElement::isVisibleInViewport() const
    14971497{
    14981498    auto& frameView = view().frameView();
    14991499    auto visibleRect = frameView.windowToContents(frameView.windowClipRect());
    1500     bool shouldRepaint = shouldRepaintInVisibleRect(visibleRect);
    1501 
    1502     if (!shouldRepaint && animatingState == ImageAnimatingState::Yes)
     1500    return isVisibleInDocumentRect(visibleRect);
     1501}
     1502
     1503VisibleInViewportState RenderElement::imageFrameAvailable(CachedImage& image, ImageAnimatingState animatingState, const IntRect* changeRect)
     1504{
     1505    bool isVisible = isVisibleInViewport();
     1506
     1507    if (!isVisible && animatingState == ImageAnimatingState::Yes)
    15031508        view().addRendererWithPausedImageAnimations(*this, image);
    15041509
    15051510    // Static images should repaint even if they are outside the viewport rectangle
    15061511    // because they should be inside the TileCoverageRect.
    1507     if (shouldRepaint || animatingState == ImageAnimatingState::No)
     1512    if (isVisible || animatingState == ImageAnimatingState::No)
    15081513        imageChanged(&image, changeRect);
    15091514
    … …  
    15111516        element()->dispatchWebKitImageReadyEventForTesting();
    15121517
    1513     return shouldRepaint ? VisibleInViewportState::Yes : VisibleInViewportState::No;
     1518    return isVisible ? VisibleInViewportState::Yes : VisibleInViewportState::No;
    15141519}
    15151520
    … …  
    15231528{
    15241529    ASSERT(m_hasPausedImageAnimations);
    1525     if (!shouldRepaintInVisibleRect(visibleRect))
     1530    if (!isVisibleInDocumentRect(visibleRect))
    15261531        return false;
    15271532

  • trunk/Source/WebCore/rendering/RenderElement.h

    r216631 r216901  
    142142    bool borderImageIsLoadedAndCanBeRendered() const;
    143143    bool mayCauseRepaintInsideViewport(const IntRect* visibleRect = nullptr) const;
    144     bool shouldRepaintInVisibleRect(const IntRect& visibleRect) const;
     144    bool isVisibleInDocumentRect(const IntRect& documentRect) const;
    145145
    146146    // Returns true if this renderer requires a new stacking context.
    … …  
    319319    void invalidateCachedFirstLineStyle();
    320320
     321    bool isVisibleInViewport() const;
     322    bool canDestroyDecodedData() final { return !isVisibleInViewport(); }
    321323    VisibleInViewportState imageFrameAvailable(CachedImage&, ImageAnimatingState, const IntRect* changeRect) final;
    322324    void didRemoveCachedImageClient(CachedImage&) final;

  • trunk/Source/WebCore/svg/graphics/SVGImage.cpp

    r215211 r216901  
    322322
    323323    if (imageObserver())
    324         imageObserver()->didDraw(this);
     324        imageObserver()->didDraw(*this);
    325325}
    326326

  • trunk/Source/WebCore/svg/graphics/SVGImageClients.h

    r215952 r216901  
    6161            return;
    6262
    63         imageObserver->imageFrameAvailable(m_image, m_image->isAnimating() ? ImageAnimatingState::Yes : ImageAnimatingState::No, &r);
     63        imageObserver->imageFrameAvailable(*m_image, m_image->isAnimating() ? ImageAnimatingState::Yes : ImageAnimatingState::No, &r);
    6464    }
    6565   

  • trunk/Source/WebCore/testing/Internals.cpp

    r216892 r216901  
    730730    MemoryCache::singleton().pruneLiveResourcesToSize(size, true);
    731731}
     732   
     733void Internals::destroyDecodedDataForAllImages()
     734{
     735    MemoryCache::singleton().destroyDecodedDataForAllImages();
     736}
    732737
    733738unsigned Internals::memoryCacheSize() const

  • trunk/Source/WebCore/testing/Internals.h

    r216696 r216901  
    115115    void clearMemoryCache();
    116116    void pruneMemoryCacheToSize(unsigned size);
     117    void destroyDecodedDataForAllImages();
    117118    unsigned memoryCacheSize() const;
    118119

  • trunk/Source/WebCore/testing/Internals.idl

    r216696 r216901  
    104104    void clearMemoryCache();
    105105    void pruneMemoryCacheToSize(long size);
     106    void destroyDecodedDataForAllImages();
    106107    long memoryCacheSize();
    107108    void setOverrideCachePolicy(CachePolicy policy);

  • trunk/Source/WebKit/mac/ChangeLog

    r216886 r216901  
     12017-05-15  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        Do not delete asynchronously decoded frames for large images if their clients are in the viewport
     4        https://bugs.webkit.org/show_bug.cgi?id=170640
     5
     6        Reviewed by Simon Fraser.
     7
     8        Enable the async decoding for large images.
     9
     10        * WebView/WebView.mm:
     11        (-[WebView _preferencesChanged:]):
     12
    1132017-05-15  Jer Noble  <jer.noble@apple.com>
    214

  • trunk/Source/WebKit/mac/WebView/WebView.mm

    r216704 r216901  
    30863086    settings.setShouldConvertInvalidURLsToBlank(shouldConvertInvalidURLsToBlank());
    30873087
    3088     // FIXME: enable async image decoding after the flickering bug wk170640 is fixed.
    3089     // settings.setLargeImageAsyncDecodingEnabled([preferences largeImageAsyncDecodingEnabled]);
    3090     settings.setLargeImageAsyncDecodingEnabled(false);
     3088    settings.setLargeImageAsyncDecodingEnabled([preferences largeImageAsyncDecodingEnabled]);
    30913089    settings.setAnimatedImageAsyncDecodingEnabled([preferences animatedImageAsyncDecodingEnabled]);
    30923090}

  • trunk/Source/WebKit2/ChangeLog

    r216898 r216901  
     12017-05-15  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        Do not delete asynchronously decoded frames for large images if their clients are in the viewport
     4        https://bugs.webkit.org/show_bug.cgi?id=170640
     5
     6        Reviewed by Simon Fraser.
     7
     8        Enable the async decoding for large images.
     9
     10        * WebProcess/WebPage/WebPage.cpp:
     11        (WebKit::WebPage::updatePreferences):
     12
    1132017-05-15  Youenn Fablet  <youenn@apple.com>
    214

  • trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp

    r216886 r216901  
    33783378    setForceAlwaysUserScalable(m_forceAlwaysUserScalable || store.getBoolValueForKey(WebPreferencesKey::forceAlwaysUserScalableKey()));
    33793379#endif
    3380     // FIXME: enable async image decoding after the flickering bug wk170640 is fixed.
    3381     // settings.setLargeImageAsyncDecodingEnabled(store.getBoolValueForKey(WebPreferencesKey::largeImageAsyncDecodingEnabledKey()));
    3382     settings.setLargeImageAsyncDecodingEnabled(false);
     3380    settings.setLargeImageAsyncDecodingEnabled(store.getBoolValueForKey(WebPreferencesKey::largeImageAsyncDecodingEnabledKey()));
    33833381    settings.setAnimatedImageAsyncDecodingEnabled(store.getBoolValueForKey(WebPreferencesKey::animatedImageAsyncDecodingEnabledKey()));
    33843382    settings.setShouldSuppressKeyboardInputDuringProvisionalNavigation(store.getBoolValueForKey(WebPreferencesKey::shouldSuppressKeyboardInputDuringProvisionalNavigationKey()));
Note: See TracChangeset for help on using the changeset viewer.