Changeset 213404 in webkit


Ignore:
Timestamp:
Mar 3, 2017, 5:45:39 PM (9 years ago)
Author:
Simon Fraser
Message:

Clean up RenderImage and a RenderImageResource function
​https://bugs.webkit.org/show_bug.cgi?id=169153

Reviewed by Zalan Bujtas.

Change all calls to imageResource().cachedImage() in RenderImage to use the inline
cachedImage() function.

In RenderImage::paintReplaced(), early return after the broken image block (and no need
to test imageResource().hasImage() again in the second condition). Convert height/width to size,
which also forces us to be explicit about using flooredIntSize() when fetching the image
(perhaps this should be a roundedIntSize, but I didn't want to change behavior).

Change RenderImageResource::image() to take an IntSize, rather than int height and width.

No behavior change.

  • rendering/RenderImage.cpp:

(WebCore::RenderImage::styleDidChange):
(WebCore::RenderImage::imageChanged):
(WebCore::RenderImage::notifyFinished):
(WebCore::RenderImage::paintReplaced):
(WebCore::RenderImage::paintIntoRect):
(WebCore::RenderImage::foregroundIsKnownToBeOpaqueInRect):
(WebCore::RenderImage::embeddedContentBox):

  • rendering/RenderImageResource.cpp:

(WebCore::RenderImageResource::image):

  • rendering/RenderImageResource.h:

(WebCore::RenderImageResource::image):

  • rendering/RenderImageResourceStyleImage.cpp:

(WebCore::RenderImageResourceStyleImage::image):

  • rendering/RenderImageResourceStyleImage.h:
Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r213400 r213404  
     12017-03-03  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Clean up RenderImage and a RenderImageResource function
     4        https://bugs.webkit.org/show_bug.cgi?id=169153
     5
     6        Reviewed by Zalan Bujtas.
     7       
     8        Change all calls to imageResource().cachedImage() in RenderImage to use the inline
     9        cachedImage() function.
     10
     11        In RenderImage::paintReplaced(), early return after the broken image block (and no need
     12        to test imageResource().hasImage() again in the second condition). Convert height/width to size,
     13        which also forces us to be explicit about using flooredIntSize() when fetching the image
     14        (perhaps this should be a roundedIntSize, but I didn't want to change behavior).
     15
     16        Change RenderImageResource::image() to take an IntSize, rather than int height and width.
     17
     18        No behavior change.
     19
     20        * rendering/RenderImage.cpp:
     21        (WebCore::RenderImage::styleDidChange):
     22        (WebCore::RenderImage::imageChanged):
     23        (WebCore::RenderImage::notifyFinished):
     24        (WebCore::RenderImage::paintReplaced):
     25        (WebCore::RenderImage::paintIntoRect):
     26        (WebCore::RenderImage::foregroundIsKnownToBeOpaqueInRect):
     27        (WebCore::RenderImage::embeddedContentBox):
     28        * rendering/RenderImageResource.cpp:
     29        (WebCore::RenderImageResource::image):
     30        * rendering/RenderImageResource.h:
     31        (WebCore::RenderImageResource::image):
     32        * rendering/RenderImageResourceStyleImage.cpp:
     33        (WebCore::RenderImageResourceStyleImage::image):
     34        * rendering/RenderImageResourceStyleImage.h:
     35
    1362017-03-03  Antoine Quint  <graouts@apple.com>
    237

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

    r210768 r213404  
    211211    RenderReplaced::styleDidChange(diff, oldStyle);
    212212    if (m_needsToSetSizeForAltText) {
    213         if (!m_altText.isEmpty() && setImageSizeForAltText(imageResource().cachedImage()))
     213        if (!m_altText.isEmpty() && setImageSizeForAltText(cachedImage()))
    214214            repaintOrMarkForLayout(ImageSizeChangeForAltText);
    215215        m_needsToSetSizeForAltText = false;
    … …  
    258258            return;
    259259        }
    260         imageSizeChange = setImageSizeForAltText(imageResource().cachedImage());
     260        imageSizeChange = setImageSizeForAltText(cachedImage());
    261261    }
    262262
    … …  
    343343    invalidateBackgroundObscurationStatus();
    344344
    345     if (&newImage == imageResource().cachedImage()) {
     345    if (&newImage == cachedImage()) {
    346346        // tell any potential compositing layers
    347347        // that the image is done and they can reference it directly.
    … …  
    352352void RenderImage::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
    353353{
    354     LayoutUnit cWidth = contentWidth();
    355     LayoutUnit cHeight = contentHeight();
    356     LayoutUnit leftBorder = borderLeft();
    357     LayoutUnit topBorder = borderTop();
    358     LayoutUnit leftPad = paddingLeft();
    359     LayoutUnit topPad = paddingTop();
     354    LayoutSize contentSize = this->contentSize();
    360355
    361356    GraphicsContext& context = paintInfo.context();
    … …  
    369364            page().addRelevantUnpaintedObject(this, visualOverflowRect());
    370365
    371         if (cWidth > 2 && cHeight > 2) {
     366        if (contentSize.width() > 2 && contentSize.height() > 2) {
    372367            LayoutUnit borderWidth = LayoutUnit(1 / deviceScaleFactor);
     368
     369            LayoutUnit leftBorder = borderLeft();
     370            LayoutUnit topBorder = borderTop();
     371            LayoutUnit leftPad = paddingLeft();
     372            LayoutUnit topPad = paddingTop();
    373373
    374374            // Draw an outline rect where the image should be.
    … …  
    376376            context.setStrokeColor(Color::lightGray);
    377377            context.setFillColor(Color::transparent);
    378             context.drawRect(snapRectToDevicePixels(LayoutRect(paintOffset.x() + leftBorder + leftPad, paintOffset.y() + topBorder + topPad, cWidth, cHeight), deviceScaleFactor), borderWidth);
     378            context.drawRect(snapRectToDevicePixels(LayoutRect({ paintOffset.x() + leftBorder + leftPad, paintOffset.y() + topBorder + topPad }, contentSize), deviceScaleFactor), borderWidth);
    379379
    380380            bool errorPictureDrawn = false;
    … …  
    382382            // When calculating the usable dimensions, exclude the pixels of
    383383            // the ouline rect so the error image/alt text doesn't draw on it.
    384             LayoutUnit usableWidth = cWidth - 2 * borderWidth;
    385             LayoutUnit usableHeight = cHeight - 2 * borderWidth;
     384            LayoutSize usableSize = contentSize - LayoutSize(2 * borderWidth, 2 * borderWidth);
    386385
    387386            RefPtr<Image> image = imageResource().image();
    388387
    389             if (imageResource().errorOccurred() && !image->isNull() && usableWidth >= image->width() && usableHeight >= image->height()) {
     388            if (imageResource().errorOccurred() && !image->isNull() && usableSize.width() >= image->width() && usableSize.height() >= image->height()) {
    390389                // Call brokenImage() explicitly to ensure we get the broken image icon at the appropriate resolution.
    391                 std::pair<Image*, float> brokenImageAndImageScaleFactor = imageResource().cachedImage()->brokenImage(document().deviceScaleFactor());
     390                std::pair<Image*, float> brokenImageAndImageScaleFactor = cachedImage()->brokenImage(document().deviceScaleFactor());
    392391                image = brokenImageAndImageScaleFactor.first;
    393392                FloatSize imageSize = image->size();
    394393                imageSize.scale(1 / brokenImageAndImageScaleFactor.second);
    395394                // Center the error image, accounting for border and padding.
    396                 LayoutUnit centerX = (usableWidth - imageSize.width()) / 2;
     395                LayoutUnit centerX = (usableSize.width() - imageSize.width()) / 2;
    397396                if (centerX < 0)
    398397                    centerX = 0;
    399                 LayoutUnit centerY = (usableHeight - imageSize.height()) / 2;
     398                LayoutUnit centerY = (usableSize.height() - imageSize.height()) / 2;
    400399                if (centerY < 0)
    401400                    centerY = 0;
    … …  
    424423                LayoutUnit textWidth = font.width(textRun);
    425424                if (errorPictureDrawn) {
    426                     if (usableWidth >= textWidth && fontMetrics.height() <= imageOffset.height())
     425                    if (usableSize.width() >= textWidth && fontMetrics.height() <= imageOffset.height())
    427426                        context.drawText(font, textRun, altTextOffset);
    428                 } else if (usableWidth >= textWidth && usableHeight >= fontMetrics.height())
     427                } else if (usableSize.width() >= textWidth && usableSize.height() >= fontMetrics.height())
    429428                    context.drawText(font, textRun, altTextOffset);
    430429            }
    431430        }
    432     } else if (imageResource().hasImage() && cWidth > 0 && cHeight > 0) {
    433         RefPtr<Image> img = imageResource().image(cWidth, cHeight);
    434         if (!img || img->isNull()) {
    435             if (paintInfo.phase == PaintPhaseForeground)
    436                 page().addRelevantUnpaintedObject(this, visualOverflowRect());
    437             return;
    438         }
    439 
    440         LayoutRect contentBoxRect = this->contentBoxRect();
    441         contentBoxRect.moveBy(paintOffset);
    442         LayoutRect replacedContentRect = this->replacedContentRect(intrinsicSize());
    443         replacedContentRect.moveBy(paintOffset);
    444         bool clip = !contentBoxRect.contains(replacedContentRect);
    445         GraphicsContextStateSaver stateSaver(context, clip);
    446         if (clip)
    447             context.clip(contentBoxRect);
    448 
    449         paintIntoRect(context, snapRectToDevicePixels(replacedContentRect, deviceScaleFactor));
    450        
    451         if (cachedImage() && paintInfo.phase == PaintPhaseForeground) {
    452             // For now, count images as unpainted if they are still progressively loading. We may want
    453             // to refine this in the future to account for the portion of the image that has painted.
    454             LayoutRect visibleRect = intersection(replacedContentRect, contentBoxRect);
    455             if (cachedImage()->isLoading())
    456                 page().addRelevantUnpaintedObject(this, visibleRect);
    457             else
    458                 page().addRelevantRepaintedObject(this, visibleRect);
    459         }
     431        return;
     432    }
     433   
     434    if (contentSize.isEmpty())
     435        return;
     436
     437    RefPtr<Image> img = imageResource().image(flooredIntSize(contentSize));
     438    if (!img || img->isNull()) {
     439        if (paintInfo.phase == PaintPhaseForeground)
     440            page().addRelevantUnpaintedObject(this, visualOverflowRect());
     441        return;
     442    }
     443
     444    LayoutRect contentBoxRect = this->contentBoxRect();
     445    contentBoxRect.moveBy(paintOffset);
     446    LayoutRect replacedContentRect = this->replacedContentRect(intrinsicSize());
     447    replacedContentRect.moveBy(paintOffset);
     448    bool clip = !contentBoxRect.contains(replacedContentRect);
     449    GraphicsContextStateSaver stateSaver(context, clip);
     450    if (clip)
     451        context.clip(contentBoxRect);
     452
     453    paintIntoRect(context, snapRectToDevicePixels(replacedContentRect, deviceScaleFactor));
     454   
     455    if (cachedImage() && paintInfo.phase == PaintPhaseForeground) {
     456        // For now, count images as unpainted if they are still progressively loading. We may want
     457        // to refine this in the future to account for the portion of the image that has painted.
     458        LayoutRect visibleRect = intersection(replacedContentRect, contentBoxRect);
     459        if (cachedImage()->isLoading())
     460            page().addRelevantUnpaintedObject(this, visibleRect);
     461        else
     462            page().addRelevantRepaintedObject(this, visibleRect);
    460463    }
    461464}
    … …  
    537540        return;
    538541
    539     RefPtr<Image> img = imageResource().image(rect.width(), rect.height());
     542    RefPtr<Image> img = imageResource().image(flooredIntSize(rect.size()));
    540543    if (!img || img->isNull())
    541544        return;
    … …  
    570573    if (!imageResource().hasImage() || imageResource().errorOccurred())
    571574        return false;
    572     if (imageResource().cachedImage() && !imageResource().cachedImage()->isLoaded())
     575    if (cachedImage() && !cachedImage()->isLoaded())
    573576        return false;
    574577    if (!contentBoxRect().contains(localRect))
    … …  
    591594
    592595    // Check for image with alpha.
    593     return imageResource().cachedImage() && imageResource().cachedImage()->currentFrameKnownToBeOpaque(this);
     596    return cachedImage() && cachedImage()->currentFrameKnownToBeOpaque(this);
    594597}
    595598
    … …  
    742745RenderBox* RenderImage::embeddedContentBox() const
    743746{
    744     CachedImage* cachedImage = imageResource().cachedImage();
     747    CachedImage* cachedImage = this->cachedImage();
    745748    if (cachedImage && is<SVGImage>(cachedImage->image()))
    746749        return downcast<SVGImage>(*cachedImage->image()).embeddedContentBox();

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

    r208511 r213404  
    9393}
    9494
    95 RefPtr<Image> RenderImageResource::image(int, int) const
     95RefPtr<Image> RenderImageResource::image(const IntSize&) const
    9696{
    9797    return m_cachedImage ? m_cachedImage->imageForRenderer(m_renderer) : Image::nullImage();

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

    r208668 r213404  
    5050    void resetAnimation();
    5151
    52     virtual RefPtr<Image> image(int width = 0, int height = 0) const;
     52    virtual RefPtr<Image> image(const IntSize& size = { }) const;
    5353    virtual bool errorOccurred() const;
    5454

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

    r208511 r213404  
    6464}
    6565
    66 RefPtr<Image> RenderImageResourceStyleImage::image(int width, int height) const
     66RefPtr<Image> RenderImageResourceStyleImage::image(const IntSize& size) const
    6767{
    6868    // Generated content may trigger calls to image() while we're still pending, don't assert but gracefully exit.
    6969    if (m_styleImage->isPending())
    7070        return nullptr;
    71     return m_styleImage->image(m_renderer, IntSize(width, height));
     71    return m_styleImage->image(m_renderer, size);
    7272}
    7373

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

    r208668 r213404  
    4444
    4545    bool hasImage() const override { return true; }
    46     RefPtr<Image> image(int width = 0, int height = 0) const override;
     46    RefPtr<Image> image(const IntSize& = { }) const override;
    4747    bool errorOccurred() const override { return m_styleImage->errorOccurred(); }
    4848
Note: See TracChangeset for help on using the changeset viewer.