Title: [182876] trunk
Revision
182876
Author
s...@apple.com
Date
2015-04-15 19:50:29 -0700 (Wed, 15 Apr 2015)

Log Message

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (182875 => 182876)


--- trunk/LayoutTests/ChangeLog	2015-04-16 02:25:28 UTC (rev 182875)
+++ trunk/LayoutTests/ChangeLog	2015-04-16 02:50:29 UTC (rev 182876)
@@ -1,3 +1,24 @@
+2015-04-15  Said Abou-Hallawa  <s...@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.
+
 2015-04-15  Jordan Harband  <ljh...@gmail.com>
 
         String.prototype.startsWith/endsWith/includes have wrong length in r182673

Added: trunk/LayoutTests/svg/text/font-small-enlarged-minimum-larger-expected.svg (0 => 182876)


--- 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)
@@ -0,0 +1,7 @@
+<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>

Added: trunk/LayoutTests/svg/text/font-small-enlarged-minimum-larger.svg (0 => 182876)


--- 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)
@@ -0,0 +1,8 @@
+<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>

Added: trunk/LayoutTests/svg/text/font-small-enlarged-minimum-smaller-expected.svg (0 => 182876)


--- 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)
@@ -0,0 +1,4 @@
+<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>

Added: trunk/LayoutTests/svg/text/font-small-enlarged-minimum-smaller.svg (0 => 182876)


--- 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)
@@ -0,0 +1,8 @@
+<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>

Modified: trunk/Source/WebCore/ChangeLog (182875 => 182876)


--- 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)
@@ -1,3 +1,33 @@
+2015-04-15  Said Abou-Hallawa  <s...@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.
+
 2015-04-15  Mark Lam  <mark....@apple.com>
 
         Remove obsolete VMInspector debugging tool.

Modified: trunk/Source/WebCore/rendering/svg/RenderSVGInlineText.cpp (182875 => 182876)


--- 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)
@@ -229,7 +229,7 @@
 {
     // Alter font-size to the right on-screen value to avoid scaling the glyphs themselves, except when GeometricPrecision is specified
     scalingFactor = SVGRenderingContext::calculateScreenFontSizeScalingFactor(renderer);
-    if (scalingFactor == 1 || !scalingFactor || style.fontDescription().textRenderingMode() == GeometricPrecision) {
+    if (!scalingFactor || style.fontDescription().textRenderingMode() == GeometricPrecision) {
         scalingFactor = 1;
         scaledFont = style.fontCascade();
         return;

Modified: trunk/Source/WebCore/style/StyleFontSizeFunctions.cpp (182875 => 182876)


--- 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)
@@ -39,9 +39,13 @@
 
 namespace Style {
 
-enum ESmartMinimumForFontSize { DoNotUseSmartMinimumForFontSize, UseSmartMinimumForFontFize };
+enum MinimumFontSizeRule {
+    DonNotApplyMinimumFontSize,
+    DoNotUseSmartMinimumForFontSize,
+    UseSmartMinimumForFontFize
+};
 
-static float computedFontSizeFromSpecifiedSize(float specifiedSize, bool isAbsoluteSize, float zoomFactor, ESmartMinimumForFontSize useSmartMinimumForFontSize, const Settings* settings)
+static float computedFontSizeFromSpecifiedSize(float specifiedSize, bool isAbsoluteSize, float zoomFactor, MinimumFontSizeRule minimumForFontSizeRule, const Settings* settings)
 {
     // Text with a 0px font size should not be visible and therefore needs to be
     // exempt from minimum font size rules. Acid3 relies on this for pixel-perfect
@@ -63,6 +67,9 @@
     if (!settings)
         return 1.0f;
 
+    if (minimumForFontSizeRule == DonNotApplyMinimumFontSize)
+        return specifiedSize;
+
     int minSize = settings->minimumFontSize();
     int minLogicalSize = settings->minimumLogicalFontSize();
     float zoomedSize = specifiedSize * zoomFactor;
@@ -75,7 +82,7 @@
     // after zooming. The font size must either be relative to the user default or the original size
     // must have been acceptable. In other words, we only apply the smart minimum whenever we're positive
     // doing so won't disrupt the layout.
-    if (useSmartMinimumForFontSize && zoomedSize < minLogicalSize && (specifiedSize >= minLogicalSize || !isAbsoluteSize))
+    if (minimumForFontSizeRule ==  UseSmartMinimumForFontFize && zoomedSize < minLogicalSize && (specifiedSize >= minLogicalSize || !isAbsoluteSize))
         zoomedSize = minLogicalSize;
 
     // Also clamp to a reasonable maximum to prevent insane font sizes from causing crashes on various
@@ -91,7 +98,7 @@
         if (Frame* frame = document.frame())
             zoomFactor *= frame->textZoomFactor();
     }
-    return computedFontSizeFromSpecifiedSize(specifiedSize, isAbsoluteSize, zoomFactor, UseSmartMinimumForFontFize, document.settings());
+    return computedFontSizeFromSpecifiedSize(specifiedSize, isAbsoluteSize, zoomFactor, useSVGZoomRules ? DonNotApplyMinimumFontSize : UseSmartMinimumForFontFize, document.settings());
 }
 
 float computedFontSizeFromSpecifiedSizeForSVGInlineText(float specifiedSize, bool isAbsoluteSize, float zoomFactor, const Document& document)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to