Improve rule coding, including for `\sqrt`. by kohler · Pull Request #776 · KaTeX/KaTeX

kohler · 2017-07-31T03:19:12Z

Rule widths (overline, underline, fraction, sqrt) actually scale
with the current font size in AMS-TeX. Implement that. (Sqrt is
a special case: the rule width depends on the font size of the
surd.)
Change the CSS coding for rules. The old, complex coding prevented
variable-width lines and may have contributed to issues like poor spacing in fraction inside square root (depends on rendering environment) #696.
Its purpose, according to 0a3a227,
was IE8 support; but KaTeX no longer supports IE8.

kohler · 2017-07-31T14:55:24Z

Example difference, using \frac towers (an example from @ronkok). Before:

After:

Note that the rules are less unruly.

k4b7 · 2017-07-31T16:01:18Z

@kohler I'll review this evening. The screenshots look good.

k4b7 · 2017-08-03T18:43:21Z

I'll review this evening

I was overly optimistic. I'll try to get to it this weekend.

k4b7 · 2017-08-05T02:39:12Z

The \sqrt rule looks a lot better. I've found the smaller font sizes to be more troublesome. Here are some renderings using 12px sized font.

Safari

Safari @ 400%

Chrome

Chrome @ 400%

I tried out some other font sizes. 18px was the worst.

Safari @ 400%

Chrome @ 400%

My guess is that the \sqrt rule is a bit to high on Chrome for 18px and is getting clipped. This also happens at 11px and might happen for other font sizes. I zoomed to 100% just in case the zoom was causing an issue and the rule was still missing. This should be fixed before merging.

* Rule widths (overline, underline, fraction, sqrt) actually scale with the current font size in AMS-TeX. Implement that. (Sqrt is a special case: the rule width depends on the font size *of the surd*.) * Change the CSS coding for rules. The old, complex coding prevented variable-width lines and may have contributed to issues like KaTeX#696. Its purpose, according to 0a3a227, was IE8 support; but KaTeX no longer supports IE8.

Smaller sizes seem more important.

kohler · 2017-08-05T12:56:24Z

Hi @kevinbarabash, I rebased on top of the current master & pushed. I cannot replicate a missing \sqrt rule on Chrome, with either 11px or 18px, at any mag. Here's the diff I made to main.css. Version 59.0.3071.115 (Official Build) (64-bit) on Mac. Can you help me replicate? There's no clipping in the CSS coding, so your proposed explanation doesn't make sense to me.

--- a/static/main.css
+++ b/static/main.css
@@ -1,7 +1,7 @@
 body {
     margin: 0px;
     padding: 0px;
-    font-size: 72px;
+    font-size: 11px;
 }
 
 #input {

k4b7 · 2017-08-05T22:29:32Z

I'm running a dev version of Chrome. It said it was ready to upgrade so I upgraded and the problem went away. This also resulted in better positioning at 12px. The sort-line is no longer floating a little bit above the square root symbol.

k4b7

Looks good, just a few questions in the comments. I'm surprised that the position of the some of the subscripts changed in the screenshots. Any idea of why that happened?

k4b7 · 2017-08-05T23:12:36Z

src/buildHTML.js

-                         options);
+    const delimChar = delimiter.customSizedDelim("\\surd", minDelimiterHeight,
+            false, options, group.mode);
+    const delim = makeSpan(["sqrt-sign"], [delimChar], options);


thanks for breaking this up

k4b7 · 2017-08-05T23:13:03Z

src/buildHTML.js

-    const ruleWidth = line.height;
+    // Calculate the minimum size for the \surd delimiter
+    const metrics = options.fontMetrics();
+    const theta = metrics.defaultRuleThickness;


Is theta the official name for this metric in the TeXBook?

k4b7 · 2017-08-05T23:26:25Z

src/buildHTML.js

+    let ruleWidth;
    if (group.value.hasBarLine) {
        rule = makeLineSpan("frac-line", options);
+        ruleWidth = ruleSpacing = rule.height;


Can you change this to:

ruleWidth = rule.height; ruleSpacing = ruleWidth;

I misread this and thought that ruleSpacing wasn't being defined in this block.

k4b7 · 2017-08-05T23:28:49Z

src/buildHTML.js

-const makeLineSpan = function(className, options) {
-    const baseOptions = options.havingBaseStyle();
-    const line = makeSpan(
-        [className].concat(baseOptions.sizingClasses(options)),


Are the sizing classes not necessary b/c we're manually figuring setting the borderBottomWidth?

Yes. The sizing classes were necessary because previously we wanted all rules to have the same thickness, and that thickness was specified in katex.less in base-sized em units. Now that we're specifying the thickness in local em units, no sizing classes.

kohler · 2017-08-06T10:59:45Z

When a nucleus has both sub & sub, the defaultRuleThickness is part of the calculation involved in separating the subscript from the superscript. The screenshotter subscript positions changed in scriptscript mode because now defaultRuleThickness is relatively larger for script and scriptscript fonts—e.g., the script defaultRuleThickness is 0.049em, not 0.04em.

The previous KaTeX implementation differed. It used 0.04em as the thickness for purposes of separating sup+sub, but it should have used 0.04em base-size units for consistency with the TeXbook (since previously all rules were 0.04em base-size units thick). Instead it used 0.04em script-size units (in scriptscript mode).

k4b7 · 2017-08-06T19:11:15Z

@kohler thanks for the PR. The updated horizontal rules look great.

kohler force-pushed the rule-css branch from d230734 to 5b337e6 Compare July 31, 2017 03:30

kohler mentioned this pull request Jul 31, 2017

Improve \sqrt #775

Closed

kohler force-pushed the rule-css branch from 007ff5f to ce044e2 Compare July 31, 2017 14:47

k4b7 self-requested a review July 31, 2017 16:01

ronkok mentioned this pull request Aug 3, 2017

Improve \sqrt #781

Closed

kohler added 2 commits August 5, 2017 08:41

The 0.5px offset makes larger sizes better, smaller sizes worse.

04e6302

Smaller sizes seem more important.

kohler force-pushed the rule-css branch from ce044e2 to 04e6302 Compare August 5, 2017 12:44

k4b7 reviewed Aug 5, 2017

View reviewed changes

Cleanup (intended to be squashed).

98f63e6

k4b7 approved these changes Aug 6, 2017

View reviewed changes

k4b7 merged commit dcdca73 into KaTeX:master Aug 6, 2017

kohler deleted the rule-css branch August 8, 2017 11:14

Uh oh!

Conversation

kohler commented Jul 31, 2017

Uh oh!

kohler commented Jul 31, 2017

Uh oh!

k4b7 commented Jul 31, 2017

Uh oh!

k4b7 commented Aug 3, 2017

Uh oh!

k4b7 commented Aug 5, 2017

Uh oh!

kohler commented Aug 5, 2017

Uh oh!

k4b7 commented Aug 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k4b7 left a comment

Choose a reason for hiding this comment

Uh oh!

k4b7 Aug 5, 2017

Choose a reason for hiding this comment

Uh oh!

k4b7 Aug 5, 2017

Choose a reason for hiding this comment

Uh oh!

kohler Aug 6, 2017

Choose a reason for hiding this comment

Uh oh!

k4b7 Aug 5, 2017

Choose a reason for hiding this comment

Uh oh!

kohler Aug 6, 2017

Choose a reason for hiding this comment

Uh oh!

k4b7 Aug 5, 2017

Choose a reason for hiding this comment

Uh oh!

kohler Aug 6, 2017

Choose a reason for hiding this comment

Uh oh!

kohler commented Aug 6, 2017

Uh oh!

k4b7 commented Aug 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

k4b7 commented Aug 5, 2017 •

edited

Loading