Minimum font size pref breaks SVG text very badly.
authorsaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Apr 2015 02:50:29 +0000 (02:50 +0000)
committersaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Apr 2015 02:50:29 +0000 (02:50 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@182876 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/svg/text/font-small-enlarged-minimum-larger-expected.svg [new file with mode: 0644]
LayoutTests/svg/text/font-small-enlarged-minimum-larger.svg [new file with mode: 0644]
LayoutTests/svg/text/font-small-enlarged-minimum-smaller-expected.svg [new file with mode: 0644]
LayoutTests/svg/text/font-small-enlarged-minimum-smaller.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGInlineText.cpp
Source/WebCore/style/StyleFontSizeFunctions.cpp

index 52873bd..7f8553a 100644 (file)
@@ -1,3 +1,24 @@
+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.
+
 2015-04-15  Jordan Harband  <ljharb@gmail.com>
 
         String.prototype.startsWith/endsWith/includes have wrong length in r182673
diff --git a/LayoutTests/svg/text/font-small-enlarged-minimum-larger-expected.svg b/LayoutTests/svg/text/font-small-enlarged-minimum-larger-expected.svg
new file mode 100644 (file)
index 0000000..6a03457
--- /dev/null
@@ -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>
diff --git a/LayoutTests/svg/text/font-small-enlarged-minimum-larger.svg b/LayoutTests/svg/text/font-small-enlarged-minimum-larger.svg
new file mode 100644 (file)
index 0000000..579b475
--- /dev/null
@@ -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>
diff --git a/LayoutTests/svg/text/font-small-enlarged-minimum-smaller-expected.svg b/LayoutTests/svg/text/font-small-enlarged-minimum-smaller-expected.svg
new file mode 100644 (file)
index 0000000..e5b4ae3
--- /dev/null
@@ -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>
diff --git a/LayoutTests/svg/text/font-small-enlarged-minimum-smaller.svg b/LayoutTests/svg/text/font-small-enlarged-minimum-smaller.svg
new file mode 100644 (file)
index 0000000..aeacb47
--- /dev/null
@@ -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>
index d0873c6..a1303c7 100644 (file)
@@ -1,3 +1,33 @@
+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.
+
 2015-04-15  Mark Lam  <mark.lam@apple.com>
 
         Remove obsolete VMInspector debugging tool.
index 472bb2b..c3c471b 100644 (file)
@@ -229,7 +229,7 @@ void RenderSVGInlineText::computeNewScaledFontForStyle(const RenderObject& rende
 {
     // 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;
index 0ab3e16..0142d18 100644 (file)
@@ -39,9 +39,13 @@ namespace WebCore {
 
 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 @@ static float computedFontSizeFromSpecifiedSize(float specifiedSize, bool isAbsol
     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 @@ static float computedFontSizeFromSpecifiedSize(float specifiedSize, bool isAbsol
     // 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 @@ float computedFontSizeFromSpecifiedSize(float specifiedSize, bool isAbsoluteSize
         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)