Title: [181942] releases/WebKitGTK/webkit-2.8
Revision
181942
Author
[email protected]
Date
2015-03-25 03:05:12 -0700 (Wed, 25 Mar 2015)

Log Message

Merge r181720 - Switching between two SVG images with no intrinsic sizes causes them to get the default SVG size instead of the container size.
https://bugs.webkit.org/show_bug.cgi?id=142805.

Patch by Said Abou-Hallawa <[email protected]> on 2015-03-18
Reviewed by Darin Adler.
Source/WebCore:

The bug happens due to wrong logic in RenderImage::imageDimensionsChanged().
This function decides to setNeedsLayout() if the intrinsic size of the image
changes. If the size does not change, it only repaints the image rectangle.
When switching the src of the an image between two SVG images and both of
them have no intrinsic size, we do not updateInnerContentRect() and this
means an SVGImageForContainer is not going to be created for this image.
When the image is drawn, it is drawn directly from the SVGImage. And this
means the drawing has to be scaled by container_size / SVG_default_intrinsic_size

After figuring out that I need to updateInnerContentRect() to fix this bug,
I found out Blink has already changed this code to do the same thing. But
they also did more clean-up in this function. Here is the link
https://codereview.chromium.org/114323004. I think their change seems correct
although they did not say what exactly they were trying to fix.

The plan for repaintOrMarkForLayout(), which is the new name of this function,
is the following:
    -- setNeedLayout() if the intrinsic size changes and it affects the size
       of the image.
    -- updateInnerContentRect() if the intrinsic size did not change but the
       image has exiting layout.
    -- repaint the image rectangle if layout is not needed.

This change also removes the call to computeLogicalWidthInRegion(), which is
almost running a layout for the image. This call figures out whether the image
needs to setNeedsLayout(). This call is unnecessary; the image needs to run a
layout if the intrinsic size has changed and it affects the size of the image.

Test: svg/as-image/svg-no-intrinsic-size-switching.html

* rendering/RenderImage.cpp:
(WebCore::RenderImage::styleDidChange): Change the function call.
(WebCore::RenderImage::imageChanged): Rename local variable and change the
function call.

(WebCore::RenderImage::updateIntrinsicSizeIfNeeded): Simplify this function.
Call setIntrinsicSize() with the new size unless the image is in error state.

(WebCore::RenderImage::repaintOrMarkForLayout): This a better name for this
function since it is called even if the intrinsic size was not changed.
(WebCore::RenderImage::imageDimensionsChanged): Deleted.

* rendering/RenderImage.h: Rename imageDimensionsChanged() and change the
updateIntrinsicSizeIfNeeded() to return void.

* rendering/svg/RenderSVGForeignObject.cpp:
(WebCore::RenderSVGForeignObject::paint): Code cleanup. This function can
only handle the paint phases PaintPhaseForeground and PaintPhaseSelection.
Use this information to simplify the logic and order of painting there.

LayoutTests:

* svg/as-image/svg-no-intrinsic-size-switching-expected.html: Added.
* svg/as-image/svg-no-intrinsic-size-switching.html: Added.
Ensure that switching the source of an <img> element between two SVG images,
which have no intrinsic sizes, gets the image the size of the container and
not the default SVG intrinsic size which is 300x150 pixels.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog (181941 => 181942)


--- releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog	2015-03-25 10:02:37 UTC (rev 181941)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog	2015-03-25 10:05:12 UTC (rev 181942)
@@ -1,3 +1,16 @@
+2015-03-18  Said Abou-Hallawa  <[email protected]>
+
+        Switching between two SVG images with no intrinsic sizes causes them to get the default SVG size instead of the container size.
+        https://bugs.webkit.org/show_bug.cgi?id=142805.
+
+        Reviewed by Darin Adler.
+
+        * svg/as-image/svg-no-intrinsic-size-switching-expected.html: Added.
+        * svg/as-image/svg-no-intrinsic-size-switching.html: Added.
+        Ensure that switching the source of an <img> element between two SVG images,
+        which have no intrinsic sizes, gets the image the size of the container and
+        not the default SVG intrinsic size which is 300x150 pixels.
+
 2015-03-18  Simon Fraser  <[email protected]>
 
         Avoid repaints when changing transform on an element with multiple background images

Added: releases/WebKitGTK/webkit-2.8/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching-expected.html (0 => 181942)


--- releases/WebKitGTK/webkit-2.8/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching-expected.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching-expected.html	2015-03-25 10:05:12 UTC (rev 181942)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+    div.outer {
+      width: 300px;
+      height: 100px;
+      background-color: green;
+    }
+    div.inner {
+      position: relative;
+      left: 75px;
+      top: 25px;
+      width: 150px;
+      height: 50px;
+      background-color: lime;
+    }
+  </style>
+</head>
+<body>
+  <div class="outer">
+    <div class="inner"></div>
+  </div>
+</body>
+</html>

Added: releases/WebKitGTK/webkit-2.8/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching.html (0 => 181942)


--- releases/WebKitGTK/webkit-2.8/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching.html	2015-03-25 10:05:12 UTC (rev 181942)
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+    div {
+      width: 300px;
+      height: 100px;
+      background-color: red;
+    }
+  </style>
+</head>
+<body _onload_="changeImage('orange')">
+  <div>
+    <img height="100" id="image_1" _onload_="onloadImage()">
+  </div>
+  <script>
+    if (window.testRunner)
+      testRunner.waitUntilDone();
+    
+    function onloadImage() {
+      var colors = ["orange", "green", "salmon", "yellow", "green"];
+
+      if (typeof onloadImage.counter == 'undefined')
+        onloadImage.counter = 0;
+
+      if (++onloadImage.counter == colors.length) {
+        if (window.testRunner)
+          testRunner.notifyDone();
+        return;
+      }
+
+      changeImage(colors[onloadImage.counter]);
+    }    
+    function changeImage(name) {
+      var image = document.getElementById('image_1');
+      switch (name) {
+      case 'orange':
+        image.src = "" \
+          <svg version='1.1' xmlns='http://www.w3.org/2000/svg' viewBox='0 0 200 200' width='100' height='100'> \
+            <rect width='100%' height='100%' fill='orange'/> \
+            <rect x='25%' y='25%' width='50%' height='50%' fill='lime'/> \
+          </svg>";
+        break;
+
+      default:
+        image.src = "" \
+          <svg version='1.1' xmlns='http://www.w3.org/2000/svg'> \
+            <rect width='100%' height='100%' fill='" + name + "'/> \
+            <rect x='25%' y='25%' width='50%' height='50%' fill='lime'/> \
+          </svg>";
+        break;
+      }
+    }
+  </script>
+</body>
+</html>

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog (181941 => 181942)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog	2015-03-25 10:02:37 UTC (rev 181941)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog	2015-03-25 10:05:12 UTC (rev 181942)
@@ -1,3 +1,67 @@
+2015-03-18  Manuel Rego Casasnovas  <[email protected]>
+
+        Unreviewed. GTK build fix after r181720.
+
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::styleDidChange):
+
+2015-03-18  Said Abou-Hallawa  <[email protected]>
+
+        Switching between two SVG images with no intrinsic sizes causes them to get the default SVG size instead of the container size.
+        https://bugs.webkit.org/show_bug.cgi?id=142805.
+
+        Reviewed by Darin Adler.
+        
+        The bug happens due to wrong logic in RenderImage::imageDimensionsChanged().
+        This function decides to setNeedsLayout() if the intrinsic size of the image
+        changes. If the size does not change, it only repaints the image rectangle.
+        When switching the src of the an image between two SVG images and both of
+        them have no intrinsic size, we do not updateInnerContentRect() and this
+        means an SVGImageForContainer is not going to be created for this image.
+        When the image is drawn, it is drawn directly from the SVGImage. And this
+        means the drawing has to be scaled by container_size / SVG_default_intrinsic_size
+        
+        After figuring out that I need to updateInnerContentRect() to fix this bug,
+        I found out Blink has already changed this code to do the same thing. But 
+        they also did more clean-up in this function. Here is the link
+        https://codereview.chromium.org/114323004. I think their change seems correct
+        although they did not say what exactly they were trying to fix.
+        
+        The plan for repaintOrMarkForLayout(), which is the new name of this function,
+        is the following:
+            -- setNeedLayout() if the intrinsic size changes and it affects the size
+               of the image.
+            -- updateInnerContentRect() if the intrinsic size did not change but the
+               image has exiting layout.
+            -- repaint the image rectangle if layout is not needed.
+            
+        This change also removes the call to computeLogicalWidthInRegion(), which is
+        almost running a layout for the image. This call figures out whether the image
+        needs to setNeedsLayout(). This call is unnecessary; the image needs to run a
+        layout if the intrinsic size has changed and it affects the size of the image.
+                    
+        Test: svg/as-image/svg-no-intrinsic-size-switching.html
+
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::styleDidChange): Change the function call.
+        (WebCore::RenderImage::imageChanged): Rename local variable and change the
+        function call.
+        
+        (WebCore::RenderImage::updateIntrinsicSizeIfNeeded): Simplify this function.
+        Call setIntrinsicSize() with the new size unless the image is in error state.
+        
+        (WebCore::RenderImage::repaintOrMarkForLayout): This a better name for this
+        function since it is called even if the intrinsic size was not changed.
+        (WebCore::RenderImage::imageDimensionsChanged): Deleted.
+        
+        * rendering/RenderImage.h: Rename imageDimensionsChanged() and change the 
+        updateIntrinsicSizeIfNeeded() to return void.
+        
+        * rendering/svg/RenderSVGForeignObject.cpp:
+        (WebCore::RenderSVGForeignObject::paint): Code cleanup. This function can
+        only handle the paint phases PaintPhaseForeground and PaintPhaseSelection.
+        Use this information to simplify the logic and order of painting there.
+
 2015-03-18  Simon Fraser  <[email protected]>
 
         Avoid repaints when changing transform on an element with multiple background images

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/RenderImage.cpp (181941 => 181942)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/RenderImage.cpp	2015-03-25 10:02:37 UTC (rev 181941)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/RenderImage.cpp	2015-03-25 10:05:12 UTC (rev 181942)
@@ -180,7 +180,7 @@
 
 // Sets the image height and width to fit the alt text.  Returns true if the
 // image size changed.
-bool RenderImage::setImageSizeForAltText(CachedImage* newImage /* = 0 */)
+ImageSizeChangeType RenderImage::setImageSizeForAltText(CachedImage* newImage /* = 0 */)
 {
     IntSize imageSize;
     if (newImage && newImage->imageForRenderer(this))
@@ -198,10 +198,10 @@
     }
 
     if (imageSize == intrinsicSize())
-        return false;
+        return ImageSizeChangeNone;
 
     setIntrinsicSize(imageSize);
-    return true;
+    return ImageSizeChangeForAltText;
 }
 
 void RenderImage::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
@@ -209,12 +209,12 @@
     RenderReplaced::styleDidChange(diff, oldStyle);
     if (m_needsToSetSizeForAltText) {
         if (!m_altText.isEmpty() && setImageSizeForAltText(imageResource().cachedImage()))
-            imageDimensionsChanged(true /* imageSizeChanged */);
+            repaintOrMarkForLayout(ImageSizeChangeForAltText);
         m_needsToSetSizeForAltText = false;
     }
 #if ENABLE(CSS_IMAGE_ORIENTATION)
     if (diff == StyleDifferenceLayout && oldStyle->imageOrientation() != style().imageOrientation())
-        return imageDimensionsChanged(true /* imageSizeChanged */);
+        return repaintOrMarkForLayout(ImageSizeChangeForAltText);
 #endif
 
 #if ENABLE(CSS_IMAGE_RESOLUTION)
@@ -222,7 +222,7 @@
         && (oldStyle->imageResolution() != style().imageResolution()
             || oldStyle->imageResolutionSnap() != style().imageResolutionSnap()
             || oldStyle->imageResolutionSource() != style().imageResolutionSource()))
-        imageDimensionsChanged(true /* imageSizeChanged */);
+        repaintOrMarkForLayout(ImageSizeChangeForAltText);
 #endif
 }
 
@@ -246,7 +246,7 @@
         m_didIncrementVisuallyNonEmptyPixelCount = true;
     }
 
-    bool imageSizeChanged = false;
+    ImageSizeChangeType imageSizeChange = ImageSizeChangeNone;
 
     // Set image dimensions, taking into account the size of the alt text.
     if (imageResource().errorOccurred()) {
@@ -258,20 +258,17 @@
             }
             return;
         }
-        imageSizeChanged = setImageSizeForAltText(imageResource().cachedImage());
+        imageSizeChange = setImageSizeForAltText(imageResource().cachedImage());
     }
 
-    imageDimensionsChanged(imageSizeChanged, rect);
+    repaintOrMarkForLayout(imageSizeChange, rect);
 }
 
-bool RenderImage::updateIntrinsicSizeIfNeeded(const LayoutSize& newSize, bool imageSizeChanged)
+void RenderImage::updateIntrinsicSizeIfNeeded(const LayoutSize& newSize)
 {
-    if (newSize == intrinsicSize() && !imageSizeChanged)
-        return false;
-    if (imageResource().errorOccurred())
-        return imageSizeChanged;
+    if (imageResource().errorOccurred() || !m_imageResource->hasImage())
+        return;
     setIntrinsicSize(newSize);
-    return true;
 }
 
 void RenderImage::updateInnerContentRect()
@@ -282,7 +279,7 @@
         imageResource().setContainerSizeForRenderer(containerSize);
 }
 
-void RenderImage::imageDimensionsChanged(bool imageSizeChanged, const IntRect* rect)
+void RenderImage::repaintOrMarkForLayout(ImageSizeChangeType imageSizeChange, const IntRect* rect)
 {
 #if ENABLE(CSS_IMAGE_RESOLUTION)
     double scale = style().imageResolution();
@@ -290,11 +287,14 @@
         scale = roundForImpreciseConversion<int>(scale);
     if (scale <= 0)
         scale = 1;
-    bool intrinsicSizeChanged = updateIntrinsicSizeIfNeeded(imageResource().intrinsicSize(style().effectiveZoom() / scale), imageSizeChanged);
+    LayoutSize newIntrinsicSize = imageResource().intrinsicSize(style().effectiveZoom() / scale);
 #else
-    bool intrinsicSizeChanged = updateIntrinsicSizeIfNeeded(imageResource().intrinsicSize(style().effectiveZoom()), imageSizeChanged);
+    LayoutSize newIntrinsicSize = imageResource().intrinsicSize(style().effectiveZoom());
 #endif
+    LayoutSize oldIntrinsicSize = intrinsicSize();
 
+    updateIntrinsicSizeIfNeeded(newIntrinsicSize);
+
     // In the case of generated image content using :before/:after/content, we might not be
     // in the render tree yet. In that case, we just need to update our intrinsic size.
     // layout() will be called after we are inserted in the tree which will take care of
@@ -302,66 +302,52 @@
     if (!containingBlock())
         return;
 
-    bool shouldRepaint = true;
-    if (intrinsicSizeChanged) {
-        if (!preferredLogicalWidthsDirty())
-            setPreferredLogicalWidthsDirty(true);
+    bool imageSourceHasChangedSize = oldIntrinsicSize != newIntrinsicSize || imageSizeChange != ImageSizeChangeNone;
 
-        bool hasOverrideSize = hasOverrideHeight() || hasOverrideWidth();
-        if (!hasOverrideSize && !imageSizeChanged) {
-            LogicalExtentComputedValues computedValues;
-            computeLogicalWidthInRegion(computedValues);
-            LayoutUnit newWidth = computedValues.m_extent;
-            computeLogicalHeight(height(), 0, computedValues);
-            LayoutUnit newHeight = computedValues.m_extent;
+    if (imageSourceHasChangedSize)
+        setPreferredLogicalWidthsDirty(true);
 
-            imageSizeChanged = width() != newWidth || height() != newHeight;
-        }
+    // If the actual area occupied by the image has changed and it is not constrained by style then a layout is required.
+    bool imageSizeIsConstrained = style().logicalWidth().isSpecified() && style().logicalHeight().isSpecified();
+    bool needsLayout = !imageSizeIsConstrained && imageSourceHasChangedSize;
 
-        // FIXME: We only need to recompute the containing block's preferred size
-        // if the containing block's size depends on the image's size (i.e., the container uses shrink-to-fit sizing).
-        // There's no easy way to detect that shrink-to-fit is needed, always force a layout.
-        bool containingBlockNeedsToRecomputePreferredSize =
-            style().logicalWidth().isPercent()
-            || style().logicalMaxWidth().isPercent()
-            || style().logicalMinWidth().isPercent();
+    // FIXME: We only need to recompute the containing block's preferred size
+    // if the containing block's size depends on the image's size (i.e., the container uses shrink-to-fit sizing).
+    // There's no easy way to detect that shrink-to-fit is needed, always force a layout.
+    bool containingBlockNeedsToRecomputePreferredSize =
+        style().logicalWidth().isPercent()
+        || style().logicalMaxWidth().isPercent()
+        || style().logicalMinWidth().isPercent();
 
-        bool layoutSizeDependsOnIntrinsicSize = style().aspectRatioType() == AspectRatioFromIntrinsic;
+    bool layoutSizeDependsOnIntrinsicSize = style().aspectRatioType() == AspectRatioFromIntrinsic;
 
-        if (imageSizeChanged || hasOverrideSize || containingBlockNeedsToRecomputePreferredSize || layoutSizeDependsOnIntrinsicSize) {
-            shouldRepaint = false;
-            if (!selfNeedsLayout())
-                setNeedsLayout();
-        }
+    if (needsLayout || containingBlockNeedsToRecomputePreferredSize || layoutSizeDependsOnIntrinsicSize) {
+        setNeedsLayout();
+        return;
+    }
 
-        if (everHadLayout() && !selfNeedsLayout()) {
-            // The inner content rectangle is calculated during layout, but may need an update now
-            // (unless the box has already been scheduled for layout). In order to calculate it, we
-            // may need values from the containing block, though, so make sure that we're not too
-            // early. It may be that layout hasn't even taken place once yet.
+    if (everHadLayout() && !selfNeedsLayout()) {
+        // The inner content rectangle is calculated during layout, but may need an update now
+        // (unless the box has already been scheduled for layout). In order to calculate it, we
+        // may need values from the containing block, though, so make sure that we're not too
+        // early. It may be that layout hasn't even taken place once yet.
 
-            // FIXME: we should not have to trigger another call to setContainerSizeForRenderer()
-            // from here, since it's already being done during layout.
-            updateInnerContentRect();
-        }
+        // FIXME: we should not have to trigger another call to setContainerSizeForRenderer()
+        // from here, since it's already being done during layout.
+        updateInnerContentRect();
     }
 
-    if (shouldRepaint) {
-        LayoutRect repaintRect;
-        if (rect) {
-            // The image changed rect is in source image coordinates (pre-zooming),
-            // so map from the bounds of the image to the contentsBox.
-            repaintRect = enclosingIntRect(mapRect(*rect, FloatRect(FloatPoint(), imageResource().imageSize(1.0f)), contentBoxRect()));
-            // Guard against too-large changed rects.
-            repaintRect.intersect(contentBoxRect());
-        } else
-            repaintRect = contentBoxRect();
+    LayoutRect repaintRect = contentBoxRect();
+    if (rect) {
+        // The image changed rect is in source image coordinates (pre-zooming),
+        // so map from the bounds of the image to the contentsBox.
+        repaintRect.intersect(enclosingIntRect(mapRect(*rect, FloatRect(FloatPoint(), imageResource().imageSize(1.0f)), repaintRect)));
+    }
         
-        repaintRectangle(repaintRect);
+    repaintRectangle(repaintRect);
 
-        // Tell any potential compositing layers that the image needs updating.
-        contentChanged(ImageChanged);
-    }
+    // Tell any potential compositing layers that the image needs updating.
+    contentChanged(ImageChanged);
 }
 
 void RenderImage::notifyFinished(CachedResource* newImage)

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/RenderImage.h (181941 => 181942)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/RenderImage.h	2015-03-25 10:02:37 UTC (rev 181941)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/RenderImage.h	2015-03-25 10:05:12 UTC (rev 181942)
@@ -33,6 +33,11 @@
 class HTMLAreaElement;
 class HTMLMapElement;
 
+enum ImageSizeChangeType {
+    ImageSizeChangeNone,
+    ImageSizeChangeForAltText
+};
+
 class RenderImage : public RenderReplaced {
 public:
     RenderImage(Element&, Ref<RenderStyle>&&, StyleImage* = nullptr, const float = 1.0f);
@@ -43,7 +48,7 @@
     const RenderImageResource& imageResource() const { return *m_imageResource; }
     CachedImage* cachedImage() const { return imageResource().cachedImage(); }
 
-    bool setImageSizeForAltText(CachedImage* newImage = 0);
+    ImageSizeChangeType setImageSizeForAltText(CachedImage* newImage = nullptr);
 
     void updateAltText();
 
@@ -74,7 +79,7 @@
 
     virtual void styleDidChange(StyleDifference, const RenderStyle*) override final;
 
-    virtual void imageChanged(WrappedImagePtr, const IntRect* = 0) override;
+    virtual void imageChanged(WrappedImagePtr, const IntRect* = nullptr) override;
 
     void paintIntoRect(GraphicsContext*, const FloatRect&);
     virtual void paint(PaintInfo&, const LayoutPoint&) override final;
@@ -107,8 +112,8 @@
     virtual bool shadowControlsNeedCustomLayoutMetrics() const { return false; }
 
     IntSize imageSizeForError(CachedImage*) const;
-    void imageDimensionsChanged(bool imageSizeChanged, const IntRect* = 0);
-    bool updateIntrinsicSizeIfNeeded(const LayoutSize&, bool imageSizeChanged);
+    void repaintOrMarkForLayout(ImageSizeChangeType, const IntRect* = nullptr);
+    void updateIntrinsicSizeIfNeeded(const LayoutSize&);
     // Update the size of the image to be rendered. Object-fit may cause this to be different from the CSS box's content rect.
     void updateInnerContentRect();
 

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp (181941 => 181942)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp	2015-03-25 10:02:37 UTC (rev 181941)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp	2015-03-25 10:05:12 UTC (rev 181942)
@@ -53,10 +53,12 @@
 
 void RenderSVGForeignObject::paint(PaintInfo& paintInfo, const LayoutPoint&)
 {
-    if (paintInfo.context->paintingDisabled()
-        || (paintInfo.phase != PaintPhaseForeground && paintInfo.phase != PaintPhaseSelection))
+    if (paintInfo.context->paintingDisabled())
         return;
 
+    if (paintInfo.phase != PaintPhaseForeground && paintInfo.phase != PaintPhaseSelection)
+        return;
+
     PaintInfo childPaintInfo(paintInfo);
     GraphicsContextStateSaver stateSaver(*childPaintInfo.context);
     childPaintInfo.applyTransform(localTransform());
@@ -65,30 +67,30 @@
         childPaintInfo.context->clip(m_viewport);
 
     SVGRenderingContext renderingContext;
-    bool continueRendering = true;
     if (paintInfo.phase == PaintPhaseForeground) {
         renderingContext.prepareToRenderSVGContent(*this, childPaintInfo);
-        continueRendering = renderingContext.isRenderingPrepared();
+        if (!renderingContext.isRenderingPrepared())
+            return;
     }
 
-    if (continueRendering) {
-        // Paint all phases of FO elements atomically, as though the FO element established its
-        // own stacking context.
-        bool preservePhase = paintInfo.phase == PaintPhaseSelection || paintInfo.phase == PaintPhaseTextClip;
-        LayoutPoint childPoint = IntPoint();
-        childPaintInfo.phase = preservePhase ? paintInfo.phase : PaintPhaseBlockBackground;
-        RenderBlock::paint(childPaintInfo, IntPoint());
-        if (!preservePhase) {
-            childPaintInfo.phase = PaintPhaseChildBlockBackgrounds;
-            RenderBlock::paint(childPaintInfo, childPoint);
-            childPaintInfo.phase = PaintPhaseFloat;
-            RenderBlock::paint(childPaintInfo, childPoint);
-            childPaintInfo.phase = PaintPhaseForeground;
-            RenderBlock::paint(childPaintInfo, childPoint);
-            childPaintInfo.phase = PaintPhaseOutline;
-            RenderBlock::paint(childPaintInfo, childPoint);
-        }
+    LayoutPoint childPoint = IntPoint();
+    if (paintInfo.phase == PaintPhaseSelection) {
+        RenderBlock::paint(childPaintInfo, childPoint);
+        return;
     }
+
+    // Paint all phases of FO elements atomically, as though the FO element established its
+    // own stacking context.
+    childPaintInfo.phase = PaintPhaseBlockBackground;
+    RenderBlock::paint(childPaintInfo, childPoint);
+    childPaintInfo.phase = PaintPhaseChildBlockBackgrounds;
+    RenderBlock::paint(childPaintInfo, childPoint);
+    childPaintInfo.phase = PaintPhaseFloat;
+    RenderBlock::paint(childPaintInfo, childPoint);
+    childPaintInfo.phase = PaintPhaseForeground;
+    RenderBlock::paint(childPaintInfo, childPoint);
+    childPaintInfo.phase = PaintPhaseOutline;
+    RenderBlock::paint(childPaintInfo, childPoint);
 }
 
 LayoutRect RenderSVGForeignObject::clippedOverflowRectForRepaint(const RenderLayerModelObject* repaintContainer) const
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to