Bug 1370646 - Honor the maxTextRunSize if it's within reasonable range r=heycam
authorviolet <violet.bugreport@gmail.com>
Wed, 03 Apr 2019 01:00:38 +0000
changeset 467739 bc2d8bd355abb9e103212bca4e9641961c52a9a9
parent 467738 d124ebd3b9ebe144ef421dacc432e06733e495f4
child 467740 d1789703bc525e8eb87446a2f0f9d8587988fe29
push id112658
push useraciure@mozilla.com
push dateThu, 04 Apr 2019 04:41:45 +0000
treeherdermozilla-inbound@a36718c8163e [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
@@ -5217,19 +5217,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>
+