Bug 1370646 - Honor the maxTextRunSize if it's within reasonable range r=heycam
☠☠ backed out by beed1b214c4f ☠ ☠
authorviolet <violet.bugreport@gmail.com>
Fri, 29 Mar 2019 07:01:10 +0000
changeset 466733 005d447749ecce7321818b2aa04a296cccfbb1ac
parent 466732 c45308b644b0bf71af22215a8e581e7ef9ddc10d
child 466734 29c306545aa5c14a1f2bbe911770b39d09ab5938
push id112600
push useropoprus@mozilla.com
push dateFri, 29 Mar 2019 22:13:12 +0000
treeherdermozilla-inbound@2841d42bdf5d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1370646
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1370646 - Honor the maxTextRunSize if it's within reasonable range r=heycam If the maximal and minimal font-size in a SVGTextFrame have a huge difference, previously we chose mFontSizeScaleFactor to satisfy the minimal one. That's problematic, because the maximal one might be a reasonable size, while the minimal one is extremely small. We should honor the maximal one if this is the case. Differential Revision: https://phabricator.services.mozilla.com/D24494
layout/svg/SVGTextFrame.cpp
layout/svg/tests/mochitest.ini
layout/svg/tests/test_multiple_font_size.html
--- a/layout/svg/SVGTextFrame.cpp
+++ b/layout/svg/SVGTextFrame.cpp
@@ -5218,19 +5218,30 @@ bool SVGTextFrame::UpdateFontSizeScaleFa
   double maxTextRunSize = maxSize * contextScale;
 
   if (minTextRunSize >= CLAMP_MIN_SIZE && maxTextRunSize <= CLAMP_MAX_SIZE) {
     // We are already in the ideal font size range for all text frames,
     // so we only have to take into account the contextScale.
     mFontSizeScaleFactor = contextScale;
   } else if (maxSize / minSize > CLAMP_MAX_SIZE / CLAMP_MIN_SIZE) {
     // We can't scale the font sizes so that all of the text frames lie
-    // within our ideal font size range, so we treat the minimum as more
-    // important and just scale so that minSize = CLAMP_MIN_SIZE.
-    mFontSizeScaleFactor = CLAMP_MIN_SIZE / minSize;
+    // within our ideal font size range.
+    // Heuristically, if the maxTextRunSize is within the CLAMP_MAX_SIZE
+    // as a reasonable value, it's likely to be the user's intent to
+    // get a valid font for the maxTextRunSize one, we should honor it.
+    // The same for minTextRunSize.
+    if (maxTextRunSize <= CLAMP_MAX_SIZE) {
+      mFontSizeScaleFactor = CLAMP_MAX_SIZE / maxSize;
+    } else if (minTextRunSize >= CLAMP_MIN_SIZE) {
+      mFontSizeScaleFactor = CLAMP_MIN_SIZE / minSize;
+    } else {
+      // So maxTextRunSize is too big, minTextRunSize is too small,
+      // we can't really do anything for this case, just leave it as is.
+      mFontSizeScaleFactor = contextScale;
+    }
   } else if (minTextRunSize < CLAMP_MIN_SIZE) {
     mFontSizeScaleFactor = CLAMP_MIN_SIZE / minSize;
   } else {
     mFontSizeScaleFactor = CLAMP_MAX_SIZE / maxSize;
   }
 
   return mFontSizeScaleFactor != oldFontSizeScaleFactor;
 }
--- a/layout/svg/tests/mochitest.ini
+++ b/layout/svg/tests/mochitest.ini
@@ -6,9 +6,10 @@ support-files =
 [test_filter_crossorigin.html]
 support-files =
   filters.svg
   file_filter_crossorigin.svg
   file_black_yellow.svg
   file_yellow_black.svg
 
 [test_hover_near_text.html]
+[test_multiple_font_size.html]
 [test_use_tree_cycle.html]
new file mode 100644
--- /dev/null
+++ b/layout/svg/tests/test_multiple_font_size.html
@@ -0,0 +1,26 @@
+<!DOCTYPE HTML>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<link rel="stylesheet" href="/tests/SimpleTest/test.css">
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1370646">Mozilla Bug 1370646</a>
+
+<svg xmlns="http://www.w3.org/2000/svg" width="440" height="100" viewBox="0 0 440 100">
+  <text>
+    <tspan id="a" style="font-size:100px">3</tspan>
+  </text>
+  <text>
+    <tspan id="b" style="font-size:100px">3</tspan>
+    <tspan style="font-size:0.1px">0</tspan>
+  </text>
+</svg>
+
+<script type="application/javascript">
+  SimpleTest.waitForExplicitFinish();
+
+  let alen = document.getElementById("a").getComputedTextLength(),
+      blen = document.getElementById("b").getComputedTextLength();
+
+  SimpleTest.isfuzzy(alen, blen, 5, "lengths should be close");
+
+  SimpleTest.finish();
+</script>
+