<!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>[182876] 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/182876">182876</a></dd>
<dt>Author</dt> <dd>said@apple.com</dd>
<dt>Date</dt> <dd>2015-04-15 19:50:29 -0700 (Wed, 15 Apr 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Minimum font size pref breaks SVG text very badly.
https://bugs.webkit.org/show_bug.cgi?id=143590.
Reviewed by Simon Fraser.
Source/WebCore:
When enabling the minimum font size perf, the computed font size is set
to the minimum font size if the computed value is smaller than the minimum.
The bug happens because the SVG text element applies its scaling on the
computed value after applying the minimum fort size rule. This means the
final computed value for the font size will be the scaling of the minimum
font size and not minimum font size itself. What we need is to postpone
applying the minimum font size rules, till the SVG scaling is applied.
Tests: svg/text/font-small-enlarged-minimum-larger.svg
svg/text/font-small-enlarged-minimum-smaller.svg
* rendering/svg/RenderSVGInlineText.cpp:
(WebCore::RenderSVGInlineText::computeNewScaledFontForStyle): Call
computedFontSizeFromSpecifiedSizeForSVGInlineText() even if scalingFactor
is 1. We need to make sure the minimum font size rules are applied. This
function was assuming the mininum font size rule was applied when resolving
the style. This is not true anymore for the SVG text.
* style/StyleFontSizeFunctions.cpp:
(WebCore::Style::computedFontSizeFromSpecifiedSize): Do not apply the
minimum size rules for the SVG element until it applies its scaling to
the font size.
LayoutTests:
When enabling the minimum font size perf, the SVG text element should
apply the minimum font size rules on the scaled font.
* svg/text/font-small-enlarged-minimum-larger-expected.svg: Added.
* svg/text/font-small-enlarged-minimum-larger.svg: Added.
Minimum font size is larger than the scaled font size. Also the expected
file makes sure the minimum font size rules are still applied if no scaling
is applied.
* svg/text/font-small-enlarged-minimum-smaller-expected.svg: Added.
* svg/text/font-small-enlarged-minimum-smaller.svg: Added.
Minimum font size is smaller than the scaled font size. So the minimim font
size rule should not have any effect on the final computed font size.</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="#trunkSourceWebCorerenderingsvgRenderSVGInlineTextcpp">trunk/Source/WebCore/rendering/svg/RenderSVGInlineText.cpp</a></li>
<li><a href="#trunkSourceWebCorestyleStyleFontSizeFunctionscpp">trunk/Source/WebCore/style/StyleFontSizeFunctions.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestssvgtextfontsmallenlargedminimumlargerexpectedsvg">trunk/LayoutTests/svg/text/font-small-enlarged-minimum-larger-expected.svg</a></li>
<li><a href="#trunkLayoutTestssvgtextfontsmallenlargedminimumlargersvg">trunk/LayoutTests/svg/text/font-small-enlarged-minimum-larger.svg</a></li>
<li><a href="#trunkLayoutTestssvgtextfontsmallenlargedminimumsmallerexpectedsvg">trunk/LayoutTests/svg/text/font-small-enlarged-minimum-smaller-expected.svg</a></li>
<li><a href="#trunkLayoutTestssvgtextfontsmallenlargedminimumsmallersvg">trunk/LayoutTests/svg/text/font-small-enlarged-minimum-smaller.svg</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (182875 => 182876)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-04-16 02:25:28 UTC (rev 182875)
+++ trunk/LayoutTests/ChangeLog 2015-04-16 02:50:29 UTC (rev 182876)
</span><span class="lines">@@ -1,3 +1,24 @@
</span><ins>+2015-04-15 Said Abou-Hallawa <said@apple.com>
+
+ Minimum font size pref breaks SVG text very badly.
+ https://bugs.webkit.org/show_bug.cgi?id=143590.
+
+ Reviewed by Simon Fraser.
+
+ When enabling the minimum font size perf, the SVG text element should
+ apply the minimum font size rules on the scaled font.
+
+ * svg/text/font-small-enlarged-minimum-larger-expected.svg: Added.
+ * svg/text/font-small-enlarged-minimum-larger.svg: Added.
+ Minimum font size is larger than the scaled font size. Also the expected
+ file makes sure the minimum font size rules are still applied if no scaling
+ is applied.
+
+ * svg/text/font-small-enlarged-minimum-smaller-expected.svg: Added.
+ * svg/text/font-small-enlarged-minimum-smaller.svg: Added.
+ Minimum font size is smaller than the scaled font size. So the minimim font
+ size rule should not have any effect on the final computed font size.
+
</ins><span class="cx"> 2015-04-15 Jordan Harband <ljharb@gmail.com>
</span><span class="cx">
</span><span class="cx"> String.prototype.startsWith/endsWith/includes have wrong length in r182673
</span></span></pre></div>
<a id="trunkLayoutTestssvgtextfontsmallenlargedminimumlargerexpectedsvg"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/text/font-small-enlarged-minimum-larger-expected.svg (0 => 182876)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/text/font-small-enlarged-minimum-larger-expected.svg (rev 0)
+++ trunk/LayoutTests/svg/text/font-small-enlarged-minimum-larger-expected.svg 2015-04-16 02:50:29 UTC (rev 182876)
</span><span class="lines">@@ -0,0 +1,7 @@
</span><ins>+<svg xmlns="http://www.w3.org/2000/svg" style="width: 400px; height: 400px;">
+ <script>
+ internals.settings.setMinimumFontSize(80.0);
+ </script>
+ <rect x="40" y="40" width="320" height="320" stroke="green" fill="none" stroke-width="2"/>
+ <text x="200" y="200" font-size="40.0" fill="black">1</text>
+</svg>
</ins></span></pre></div>
<a id="trunkLayoutTestssvgtextfontsmallenlargedminimumlargersvg"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/text/font-small-enlarged-minimum-larger.svg (0 => 182876)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/text/font-small-enlarged-minimum-larger.svg (rev 0)
+++ trunk/LayoutTests/svg/text/font-small-enlarged-minimum-larger.svg 2015-04-16 02:50:29 UTC (rev 182876)
</span><span class="lines">@@ -0,0 +1,8 @@
</span><ins>+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10" style="width: 400px; height: 400px;">
+ <script>
+ internals.settings.setMinimumFontSize(80.0);
+ </script>
+ <!-- scaling factor = 40, so the scaled font-size = 40.0 -->
+ <rect x="1" y="1" width="8" height="8" stroke="green" fill="none" stroke-width="0.05"/>
+ <text x="5" y="5" font-size="1.0" fill="black">1</text>
+</svg>
</ins></span></pre></div>
<a id="trunkLayoutTestssvgtextfontsmallenlargedminimumsmallerexpectedsvg"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/text/font-small-enlarged-minimum-smaller-expected.svg (0 => 182876)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/text/font-small-enlarged-minimum-smaller-expected.svg (rev 0)
+++ trunk/LayoutTests/svg/text/font-small-enlarged-minimum-smaller-expected.svg 2015-04-16 02:50:29 UTC (rev 182876)
</span><span class="lines">@@ -0,0 +1,4 @@
</span><ins>+<svg xmlns="http://www.w3.org/2000/svg" style="width: 400px; height: 400px;">
+ <rect x="40" y="40" width="320" height="320" stroke="green" fill="none" stroke-width="2"/>
+ <text x="200" y="200" font-size="40.0" fill="black">1</text>
+</svg>
</ins></span></pre></div>
<a id="trunkLayoutTestssvgtextfontsmallenlargedminimumsmallersvg"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/text/font-small-enlarged-minimum-smaller.svg (0 => 182876)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/text/font-small-enlarged-minimum-smaller.svg (rev 0)
+++ trunk/LayoutTests/svg/text/font-small-enlarged-minimum-smaller.svg 2015-04-16 02:50:29 UTC (rev 182876)
</span><span class="lines">@@ -0,0 +1,8 @@
</span><ins>+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10" style="width: 400px; height: 400px;">
+ <script>
+ internals.settings.setMinimumFontSize(30.0);
+ </script>
+ <!-- scaling factor = 40, so the scaled font-size = 40.0 -->
+ <rect x="1" y="1" width="8" height="8" stroke="green" fill="none" stroke-width="0.05"/>
+ <text x="5" y="5" font-size="1.0" fill="black">1</text>
+</svg>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (182875 => 182876)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-04-16 02:25:28 UTC (rev 182875)
+++ trunk/Source/WebCore/ChangeLog 2015-04-16 02:50:29 UTC (rev 182876)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2015-04-15 Said Abou-Hallawa <said@apple.com>
+
+ Minimum font size pref breaks SVG text very badly.
+ https://bugs.webkit.org/show_bug.cgi?id=143590.
+
+ Reviewed by Simon Fraser.
+
+ When enabling the minimum font size perf, the computed font size is set
+ to the minimum font size if the computed value is smaller than the minimum.
+ The bug happens because the SVG text element applies its scaling on the
+ computed value after applying the minimum fort size rule. This means the
+ final computed value for the font size will be the scaling of the minimum
+ font size and not minimum font size itself. What we need is to postpone
+ applying the minimum font size rules, till the SVG scaling is applied.
+
+ Tests: svg/text/font-small-enlarged-minimum-larger.svg
+ svg/text/font-small-enlarged-minimum-smaller.svg
+
+ * rendering/svg/RenderSVGInlineText.cpp:
+ (WebCore::RenderSVGInlineText::computeNewScaledFontForStyle): Call
+ computedFontSizeFromSpecifiedSizeForSVGInlineText() even if scalingFactor
+ is 1. We need to make sure the minimum font size rules are applied. This
+ function was assuming the mininum font size rule was applied when resolving
+ the style. This is not true anymore for the SVG text.
+
+ * style/StyleFontSizeFunctions.cpp:
+ (WebCore::Style::computedFontSizeFromSpecifiedSize): Do not apply the
+ minimum size rules for the SVG element until it applies its scaling to
+ the font size.
+
</ins><span class="cx"> 2015-04-15 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><span class="cx"> Remove obsolete VMInspector debugging tool.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingsvgRenderSVGInlineTextcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/svg/RenderSVGInlineText.cpp (182875 => 182876)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/svg/RenderSVGInlineText.cpp 2015-04-16 02:25:28 UTC (rev 182875)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGInlineText.cpp 2015-04-16 02:50:29 UTC (rev 182876)
</span><span class="lines">@@ -229,7 +229,7 @@
</span><span class="cx"> {
</span><span class="cx"> // Alter font-size to the right on-screen value to avoid scaling the glyphs themselves, except when GeometricPrecision is specified
</span><span class="cx"> scalingFactor = SVGRenderingContext::calculateScreenFontSizeScalingFactor(renderer);
</span><del>- if (scalingFactor == 1 || !scalingFactor || style.fontDescription().textRenderingMode() == GeometricPrecision) {
</del><ins>+ if (!scalingFactor || style.fontDescription().textRenderingMode() == GeometricPrecision) {
</ins><span class="cx"> scalingFactor = 1;
</span><span class="cx"> scaledFont = style.fontCascade();
</span><span class="cx"> return;
</span></span></pre></div>
<a id="trunkSourceWebCorestyleStyleFontSizeFunctionscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/style/StyleFontSizeFunctions.cpp (182875 => 182876)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/style/StyleFontSizeFunctions.cpp 2015-04-16 02:25:28 UTC (rev 182875)
+++ trunk/Source/WebCore/style/StyleFontSizeFunctions.cpp 2015-04-16 02:50:29 UTC (rev 182876)
</span><span class="lines">@@ -39,9 +39,13 @@
</span><span class="cx">
</span><span class="cx"> namespace Style {
</span><span class="cx">
</span><del>-enum ESmartMinimumForFontSize { DoNotUseSmartMinimumForFontSize, UseSmartMinimumForFontFize };
</del><ins>+enum MinimumFontSizeRule {
+ DonNotApplyMinimumFontSize,
+ DoNotUseSmartMinimumForFontSize,
+ UseSmartMinimumForFontFize
+};
</ins><span class="cx">
</span><del>-static float computedFontSizeFromSpecifiedSize(float specifiedSize, bool isAbsoluteSize, float zoomFactor, ESmartMinimumForFontSize useSmartMinimumForFontSize, const Settings* settings)
</del><ins>+static float computedFontSizeFromSpecifiedSize(float specifiedSize, bool isAbsoluteSize, float zoomFactor, MinimumFontSizeRule minimumForFontSizeRule, const Settings* settings)
</ins><span class="cx"> {
</span><span class="cx"> // Text with a 0px font size should not be visible and therefore needs to be
</span><span class="cx"> // exempt from minimum font size rules. Acid3 relies on this for pixel-perfect
</span><span class="lines">@@ -63,6 +67,9 @@
</span><span class="cx"> if (!settings)
</span><span class="cx"> return 1.0f;
</span><span class="cx">
</span><ins>+ if (minimumForFontSizeRule == DonNotApplyMinimumFontSize)
+ return specifiedSize;
+
</ins><span class="cx"> int minSize = settings->minimumFontSize();
</span><span class="cx"> int minLogicalSize = settings->minimumLogicalFontSize();
</span><span class="cx"> float zoomedSize = specifiedSize * zoomFactor;
</span><span class="lines">@@ -75,7 +82,7 @@
</span><span class="cx"> // after zooming. The font size must either be relative to the user default or the original size
</span><span class="cx"> // must have been acceptable. In other words, we only apply the smart minimum whenever we're positive
</span><span class="cx"> // doing so won't disrupt the layout.
</span><del>- if (useSmartMinimumForFontSize && zoomedSize < minLogicalSize && (specifiedSize >= minLogicalSize || !isAbsoluteSize))
</del><ins>+ if (minimumForFontSizeRule == UseSmartMinimumForFontFize && zoomedSize < minLogicalSize && (specifiedSize >= minLogicalSize || !isAbsoluteSize))
</ins><span class="cx"> zoomedSize = minLogicalSize;
</span><span class="cx">
</span><span class="cx"> // Also clamp to a reasonable maximum to prevent insane font sizes from causing crashes on various
</span><span class="lines">@@ -91,7 +98,7 @@
</span><span class="cx"> if (Frame* frame = document.frame())
</span><span class="cx"> zoomFactor *= frame->textZoomFactor();
</span><span class="cx"> }
</span><del>- return computedFontSizeFromSpecifiedSize(specifiedSize, isAbsoluteSize, zoomFactor, UseSmartMinimumForFontFize, document.settings());
</del><ins>+ return computedFontSizeFromSpecifiedSize(specifiedSize, isAbsoluteSize, zoomFactor, useSVGZoomRules ? DonNotApplyMinimumFontSize : UseSmartMinimumForFontFize, document.settings());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> float computedFontSizeFromSpecifiedSizeForSVGInlineText(float specifiedSize, bool isAbsoluteSize, float zoomFactor, const Document& document)
</span></span></pre>
</div>
</div>
</body>
</html>