<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[181720] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/181720">181720</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2015-03-18 19:15:00 -0700 (Wed, 18 Mar 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>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 <sabouhallawa@apple.com> 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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderImagecpp">trunk/Source/WebCore/rendering/RenderImage.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderImageh">trunk/Source/WebCore/rendering/RenderImage.h</a></li>
<li><a href="#trunkSourceWebCorerenderingsvgRenderSVGForeignObjectcpp">trunk/Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestssvgasimagesvgnointrinsicsizeswitchingexpectedhtml">trunk/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching-expected.html</a></li>
<li><a href="#trunkLayoutTestssvgasimagesvgnointrinsicsizeswitchinghtml">trunk/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (181719 => 181720)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-03-19 01:28:20 UTC (rev 181719)
+++ trunk/LayoutTests/ChangeLog 2015-03-19 02:15:00 UTC (rev 181720)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2015-03-18 Said Abou-Hallawa <sabouhallawa@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2015-03-18 Alexey Proskuryakov <ap@apple.com>
</span><span class="cx">
</span><span class="cx"> webaudio/convolution-mono-mono.html fails on some machines
</span></span></pre></div>
<a id="trunkLayoutTestssvgasimagesvgnointrinsicsizeswitchingexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching-expected.html (0 => 181720)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching-expected.html (rev 0)
+++ trunk/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching-expected.html 2015-03-19 02:15:00 UTC (rev 181720)
</span><span class="lines">@@ -0,0 +1,25 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestssvgasimagesvgnointrinsicsizeswitchinghtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching.html (0 => 181720)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching.html (rev 0)
+++ trunk/LayoutTests/svg/as-image/svg-no-intrinsic-size-switching.html 2015-03-19 02:15:00 UTC (rev 181720)
</span><span class="lines">@@ -0,0 +1,56 @@
</span><ins>+<!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 = "data:image/svg+xml, \
+ <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 = "data:image/svg+xml, \
+ <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>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (181719 => 181720)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-03-19 01:28:20 UTC (rev 181719)
+++ trunk/Source/WebCore/ChangeLog 2015-03-19 02:15:00 UTC (rev 181720)
</span><span class="lines">@@ -1,3 +1,60 @@
</span><ins>+2015-03-18 Said Abou-Hallawa <sabouhallawa@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2015-03-18 Jeremy Jones <jeremyj@apple.com>
</span><span class="cx">
</span><span class="cx"> Fix typo in playerViewControllerWillCancelOptimizedFullscree.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderImagecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderImage.cpp (181719 => 181720)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderImage.cpp 2015-03-19 01:28:20 UTC (rev 181719)
+++ trunk/Source/WebCore/rendering/RenderImage.cpp 2015-03-19 02:15:00 UTC (rev 181720)
</span><span class="lines">@@ -180,7 +180,7 @@
</span><span class="cx">
</span><span class="cx"> // Sets the image height and width to fit the alt text. Returns true if the
</span><span class="cx"> // image size changed.
</span><del>-bool RenderImage::setImageSizeForAltText(CachedImage* newImage /* = 0 */)
</del><ins>+ImageSizeChangeType RenderImage::setImageSizeForAltText(CachedImage* newImage /* = 0 */)
</ins><span class="cx"> {
</span><span class="cx"> IntSize imageSize;
</span><span class="cx"> if (newImage && newImage->imageForRenderer(this))
</span><span class="lines">@@ -198,10 +198,10 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (imageSize == intrinsicSize())
</span><del>- return false;
</del><ins>+ return ImageSizeChangeNone;
</ins><span class="cx">
</span><span class="cx"> setIntrinsicSize(imageSize);
</span><del>- return true;
</del><ins>+ return ImageSizeChangeForAltText;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void RenderImage::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
</span><span class="lines">@@ -209,12 +209,12 @@
</span><span class="cx"> RenderReplaced::styleDidChange(diff, oldStyle);
</span><span class="cx"> if (m_needsToSetSizeForAltText) {
</span><span class="cx"> if (!m_altText.isEmpty() && setImageSizeForAltText(imageResource().cachedImage()))
</span><del>- imageDimensionsChanged(true /* imageSizeChanged */);
</del><ins>+ repaintOrMarkForLayout(ImageSizeChangeForAltText);
</ins><span class="cx"> m_needsToSetSizeForAltText = false;
</span><span class="cx"> }
</span><span class="cx"> #if ENABLE(CSS_IMAGE_ORIENTATION)
</span><span class="cx"> if (diff == StyleDifferenceLayout && oldStyle->imageOrientation() != style().imageOrientation())
</span><del>- return imageDimensionsChanged(true /* imageSizeChanged */);
</del><ins>+ return repaintOrMarkForLayout(ImageSizeChangeForAltText);
</ins><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx"> #if ENABLE(CSS_IMAGE_RESOLUTION)
</span><span class="lines">@@ -222,7 +222,7 @@
</span><span class="cx"> && (oldStyle->imageResolution() != style().imageResolution()
</span><span class="cx"> || oldStyle->imageResolutionSnap() != style().imageResolutionSnap()
</span><span class="cx"> || oldStyle->imageResolutionSource() != style().imageResolutionSource()))
</span><del>- imageDimensionsChanged(true /* imageSizeChanged */);
</del><ins>+ repaintOrMarkForLayout(ImageSizeChangeForAltText;
</ins><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -246,7 +246,7 @@
</span><span class="cx"> m_didIncrementVisuallyNonEmptyPixelCount = true;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- bool imageSizeChanged = false;
</del><ins>+ ImageSizeChangeType imageSizeChange = ImageSizeChangeNone;
</ins><span class="cx">
</span><span class="cx"> // Set image dimensions, taking into account the size of the alt text.
</span><span class="cx"> if (imageResource().errorOccurred()) {
</span><span class="lines">@@ -258,20 +258,17 @@
</span><span class="cx"> }
</span><span class="cx"> return;
</span><span class="cx"> }
</span><del>- imageSizeChanged = setImageSizeForAltText(imageResource().cachedImage());
</del><ins>+ imageSizeChange = setImageSizeForAltText(imageResource().cachedImage());
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- imageDimensionsChanged(imageSizeChanged, rect);
</del><ins>+ repaintOrMarkForLayout(imageSizeChange, rect);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-bool RenderImage::updateIntrinsicSizeIfNeeded(const LayoutSize& newSize, bool imageSizeChanged)
</del><ins>+void RenderImage::updateIntrinsicSizeIfNeeded(const LayoutSize& newSize)
</ins><span class="cx"> {
</span><del>- if (newSize == intrinsicSize() && !imageSizeChanged)
- return false;
- if (imageResource().errorOccurred())
- return imageSizeChanged;
</del><ins>+ if (imageResource().errorOccurred() || !m_imageResource->hasImage())
+ return;
</ins><span class="cx"> setIntrinsicSize(newSize);
</span><del>- return true;
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void RenderImage::updateInnerContentRect()
</span><span class="lines">@@ -282,7 +279,7 @@
</span><span class="cx"> imageResource().setContainerSizeForRenderer(containerSize);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void RenderImage::imageDimensionsChanged(bool imageSizeChanged, const IntRect* rect)
</del><ins>+void RenderImage::repaintOrMarkForLayout(ImageSizeChangeType imageSizeChange, const IntRect* rect)
</ins><span class="cx"> {
</span><span class="cx"> #if ENABLE(CSS_IMAGE_RESOLUTION)
</span><span class="cx"> double scale = style().imageResolution();
</span><span class="lines">@@ -290,11 +287,14 @@
</span><span class="cx"> scale = roundForImpreciseConversion<int>(scale);
</span><span class="cx"> if (scale <= 0)
</span><span class="cx"> scale = 1;
</span><del>- bool intrinsicSizeChanged = updateIntrinsicSizeIfNeeded(imageResource().intrinsicSize(style().effectiveZoom() / scale), imageSizeChanged);
</del><ins>+ LayoutSize newIntrinsicSize = imageResource().intrinsicSize(style().effectiveZoom() / scale);
</ins><span class="cx"> #else
</span><del>- bool intrinsicSizeChanged = updateIntrinsicSizeIfNeeded(imageResource().intrinsicSize(style().effectiveZoom()), imageSizeChanged);
</del><ins>+ LayoutSize newIntrinsicSize = imageResource().intrinsicSize(style().effectiveZoom());
</ins><span class="cx"> #endif
</span><ins>+ LayoutSize oldIntrinsicSize = intrinsicSize();
</ins><span class="cx">
</span><ins>+ updateIntrinsicSizeIfNeeded(newIntrinsicSize);
+
</ins><span class="cx"> // In the case of generated image content using :before/:after/content, we might not be
</span><span class="cx"> // in the render tree yet. In that case, we just need to update our intrinsic size.
</span><span class="cx"> // layout() will be called after we are inserted in the tree which will take care of
</span><span class="lines">@@ -302,66 +302,52 @@
</span><span class="cx"> if (!containingBlock())
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- bool shouldRepaint = true;
- if (intrinsicSizeChanged) {
- if (!preferredLogicalWidthsDirty())
- setPreferredLogicalWidthsDirty(true);
</del><ins>+ bool imageSourceHasChangedSize = oldIntrinsicSize != newIntrinsicSize || imageSizeChange != ImageSizeChangeNone;
</ins><span class="cx">
</span><del>- 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;
</del><ins>+ if (imageSourceHasChangedSize)
+ setPreferredLogicalWidthsDirty(true);
</ins><span class="cx">
</span><del>- imageSizeChanged = width() != newWidth || height() != newHeight;
- }
</del><ins>+ // 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;
</ins><span class="cx">
</span><del>- // 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();
</del><ins>+ // 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();
</ins><span class="cx">
</span><del>- bool layoutSizeDependsOnIntrinsicSize = style().aspectRatioType() == AspectRatioFromIntrinsic;
</del><ins>+ bool layoutSizeDependsOnIntrinsicSize = style().aspectRatioType() == AspectRatioFromIntrinsic;
</ins><span class="cx">
</span><del>- if (imageSizeChanged || hasOverrideSize || containingBlockNeedsToRecomputePreferredSize || layoutSizeDependsOnIntrinsicSize) {
- shouldRepaint = false;
- if (!selfNeedsLayout())
- setNeedsLayout();
- }
</del><ins>+ if (needsLayout || containingBlockNeedsToRecomputePreferredSize || layoutSizeDependsOnIntrinsicSize) {
+ setNeedsLayout();
+ return;
+ }
</ins><span class="cx">
</span><del>- 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.
</del><ins>+ 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.
</ins><span class="cx">
</span><del>- // FIXME: we should not have to trigger another call to setContainerSizeForRenderer()
- // from here, since it's already being done during layout.
- updateInnerContentRect();
- }
</del><ins>+ // FIXME: we should not have to trigger another call to setContainerSizeForRenderer()
+ // from here, since it's already being done during layout.
+ updateInnerContentRect();
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- 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();
</del><ins>+ 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)));
+ }
</ins><span class="cx">
</span><del>- repaintRectangle(repaintRect);
</del><ins>+ repaintRectangle(repaintRect);
</ins><span class="cx">
</span><del>- // Tell any potential compositing layers that the image needs updating.
- contentChanged(ImageChanged);
- }
</del><ins>+ // Tell any potential compositing layers that the image needs updating.
+ contentChanged(ImageChanged);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void RenderImage::notifyFinished(CachedResource* newImage)
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderImageh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderImage.h (181719 => 181720)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderImage.h 2015-03-19 01:28:20 UTC (rev 181719)
+++ trunk/Source/WebCore/rendering/RenderImage.h 2015-03-19 02:15:00 UTC (rev 181720)
</span><span class="lines">@@ -33,6 +33,11 @@
</span><span class="cx"> class HTMLAreaElement;
</span><span class="cx"> class HTMLMapElement;
</span><span class="cx">
</span><ins>+enum ImageSizeChangeType {
+ ImageSizeChangeNone,
+ ImageSizeChangeForAltText
+};
+
</ins><span class="cx"> class RenderImage : public RenderReplaced {
</span><span class="cx"> public:
</span><span class="cx"> RenderImage(Element&, Ref<RenderStyle>&&, StyleImage* = nullptr, const float = 1.0f);
</span><span class="lines">@@ -43,7 +48,7 @@
</span><span class="cx"> const RenderImageResource& imageResource() const { return *m_imageResource; }
</span><span class="cx"> CachedImage* cachedImage() const { return imageResource().cachedImage(); }
</span><span class="cx">
</span><del>- bool setImageSizeForAltText(CachedImage* newImage = 0);
</del><ins>+ ImageSizeChangeType setImageSizeForAltText(CachedImage* newImage = nullptr);
</ins><span class="cx">
</span><span class="cx"> void updateAltText();
</span><span class="cx">
</span><span class="lines">@@ -74,7 +79,7 @@
</span><span class="cx">
</span><span class="cx"> virtual void styleDidChange(StyleDifference, const RenderStyle*) override final;
</span><span class="cx">
</span><del>- virtual void imageChanged(WrappedImagePtr, const IntRect* = 0) override;
</del><ins>+ virtual void imageChanged(WrappedImagePtr, const IntRect* = nullptr) override;
</ins><span class="cx">
</span><span class="cx"> void paintIntoRect(GraphicsContext*, const FloatRect&);
</span><span class="cx"> virtual void paint(PaintInfo&, const LayoutPoint&) override final;
</span><span class="lines">@@ -107,8 +112,8 @@
</span><span class="cx"> virtual bool shadowControlsNeedCustomLayoutMetrics() const { return false; }
</span><span class="cx">
</span><span class="cx"> IntSize imageSizeForError(CachedImage*) const;
</span><del>- void imageDimensionsChanged(bool imageSizeChanged, const IntRect* = 0);
- bool updateIntrinsicSizeIfNeeded(const LayoutSize&, bool imageSizeChanged);
</del><ins>+ void repaintOrMarkForLayout(ImageSizeChangeType, const IntRect* = nullptr);
+ void updateIntrinsicSizeIfNeeded(const LayoutSize&);
</ins><span class="cx"> // Update the size of the image to be rendered. Object-fit may cause this to be different from the CSS box's content rect.
</span><span class="cx"> void updateInnerContentRect();
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingsvgRenderSVGForeignObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp (181719 => 181720)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp 2015-03-19 01:28:20 UTC (rev 181719)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp 2015-03-19 02:15:00 UTC (rev 181720)
</span><span class="lines">@@ -53,10 +53,12 @@
</span><span class="cx">
</span><span class="cx"> void RenderSVGForeignObject::paint(PaintInfo& paintInfo, const LayoutPoint&)
</span><span class="cx"> {
</span><del>- if (paintInfo.context->paintingDisabled()
- || (paintInfo.phase != PaintPhaseForeground && paintInfo.phase != PaintPhaseSelection))
</del><ins>+ if (paintInfo.context->paintingDisabled())
</ins><span class="cx"> return;
</span><span class="cx">
</span><ins>+ if (paintInfo.phase != PaintPhaseForeground && paintInfo.phase != PaintPhaseSelection)
+ return;
+
</ins><span class="cx"> PaintInfo childPaintInfo(paintInfo);
</span><span class="cx"> GraphicsContextStateSaver stateSaver(*childPaintInfo.context);
</span><span class="cx"> childPaintInfo.applyTransform(localTransform());
</span><span class="lines">@@ -65,30 +67,30 @@
</span><span class="cx"> childPaintInfo.context->clip(m_viewport);
</span><span class="cx">
</span><span class="cx"> SVGRenderingContext renderingContext;
</span><del>- bool continueRendering = true;
</del><span class="cx"> if (paintInfo.phase == PaintPhaseForeground) {
</span><span class="cx"> renderingContext.prepareToRenderSVGContent(*this, childPaintInfo);
</span><del>- continueRendering = renderingContext.isRenderingPrepared();
</del><ins>+ if (!renderingContext.isRenderingPrepared())
+ return;
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- 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);
- }
</del><ins>+ LayoutPoint childPoint = IntPoint();
+ if (paintInfo.phase == PaintPhaseSelection) {
+ RenderBlock::paint(childPaintInfo, childPoint);
+ return;
</ins><span class="cx"> }
</span><ins>+
+ // 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);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> LayoutRect RenderSVGForeignObject::clippedOverflowRectForRepaint(const RenderLayerModelObject* repaintContainer) const
</span></span></pre>
</div>
</div>
</body>
</html>