Support for \raisebox by edemaine · Pull Request #685 · KaTeX/KaTeX

edemaine · 2017-04-27T21:35:39Z

I started working on \raisebox support which will help for #657 and #224. Currently \raisebox{...ex}{...text...} "works", except that the bounding box of the resulting typeset text isn't set correctly, so raised/lowered objects cut into the lines before/after. I guess I'm still not fully up to speed on how HTML generation works here -- based on other code in buildHTML.js, I tried manually updating .height or .depth according to whether the box moved up or down, but that didn't work. If anyone has a chance to look at this, I'd appreciate help fixing it.

Here's a sample render with the test server:

Here's the same example rendered in TeX, so seems good except for the bounding box issue:

Also fixed a typo where parsed sizes are accidentally labeled as "color" nodes instead of "size" nodes.

k4b7 · 2017-04-29T20:29:08Z

src/buildHTML.js

+    const span = groupTypes.text(group, options);
+    if (group.value.dy) {
+        const dy = group.value.dy.value;
+        span.style.top = "" + (-dy.number) + dy.unit;


this should handle mu units right?

Good point -- indeed, it currently does not, but would be easy to tweak to. (A bigger issue is that calculateSize only supports mu and em.)

It looks like calculateSize supports ex.

const calculateSize = function(sizeValue, style) { let x = sizeValue.number; if (sizeValue.unit === "ex") { x *= style.metrics.emPerEx; } else if (sizeValue.unit === "mu") { x /= 18; } return x; };

k4b7 · 2017-04-29T20:30:08Z

src/buildHTML.js

+            span.height += calculateSize(dy.number, options.style);
+        } else {
+            span.depth -= calculateSize(dy.number, options.style);
+        }


shouldn't height and depth both be updated regardless of the sign of dy.number?

The idea is that the height increases if the box is moved up, and the depth increases if the box is moved down. At least, my mental model is that height is the extent above the baseline (y-max) while depth is the extent below the baseline (y-min) -- please correct me if I'm wrong. However, this code doesn't seem to be working, and I'm not really sure how height and depth end up getting used (or not).

At least, my mental model is that height is the extent above the baseline (y-max) while depth is the extent below the baseline (y-min) -- please correct me if I'm wrong.

That's right, but if a glyph is shifted vertically, the the extent below the baseline will be different.

What happens if you put the test expression inside a \sqrt or a \frac? It may be that just our wrapper container isn't doing the right thing with the height and depth you're setting on the span.

Inside a \frac or \sqrt, the bounding box looks good:

This screenshot, by the way, was done with code that was slightly tweaked:

groupTypes.transform = function(group, options) { const span = groupTypes.text(group, options); const dy = calculateSize(group.value.dy.value, options.style); if (dy > 0) { span.height += dy; } else if (dy < 0) { span.depth -= dy; } span.style.top = -dy + "em"; span.style.position = "relative"; return span; };

@ronkok did your tweaks have any effect on the top level bounding box?

Now that you ask me, yes they did. Odd. I was only trying to clean it up, not change how it worked. Here's a screenshot from the original code:

Now that I need to take this more seriously, here’s a version that restores the outer if statement.

groupTypes.transform = function(group, options) { const span = groupTypes.text(group, options); if (group.value.dy) { const dy = calculateSize(group.value.dy.value, options.style); if (dy > 0) { span.height += dy; } else if (dy < 0) { span.depth -= dy; } span.style.top = -dy + "em"; span.style.position = "relative"; } return span; };

That code gives me the good results.

edemaine · 2017-05-15T23:52:08Z

Awesome, thanks @kevinbarabash and @ronkok for finding the solution here! Looks like it was close indeed, and switching to calculateSize at the top level was the missing link.

Here is what my original example A\raisebox{-0.5em}{B}\raisebox{0.5em}{C}D looks like with the new code (now pushed):

It's better, though the top margin is still a lot smaller (zero or even slightly less) than a straight \text{C}:

ronkok · 2017-05-15T23:56:23Z

You mentioned a smaller margin that a straight \text{C}. I wonder if that is why buildCommon.makeVList spends the effort to call makeFontSizer.

edemaine · 2017-05-16T00:02:51Z

Hey, I got it! I was looking at the definition of the bottom strut in buildHTML.js, and it uses a vertical-align to put the bottom strut below the top strut. I didn't know you could put numbers into vertical-align. So I tried that instead of a top style (see this trivial diff), and it now has exactly the right bounding box!

k4b7 · 2017-05-16T00:03:45Z

I didn't know you could put numbers into vertical-align

Neither did I. Cool!

k4b7 · 2017-05-16T00:06:44Z

@edemaine now that everything's working correctly, can you add a screenshot test?

edemaine · 2017-05-16T01:55:32Z

In adding a screenshot test similar to @ronkok's, I realize that we're not quite there yet: vertical-align behaves well at the top level (and in \fracs) but doesn't seem to interact properly with \sqrt...

It seems that the positive vertical-align in the b is causing the ab to move down... weird... Hmm, I think this is related to makeFontSizer that @ronkok mentioned, as the contents of the sqrt are put in a vlist (although so are the \fracs...).

ronkok · 2017-05-16T02:10:28Z

I don't think the problem is yours. If I understand matters correctly, KaTeX does not change the radical height in a continuous manner. It's a step function. You happened to land on a bad height.

edemaine · 2017-05-16T02:14:24Z

@ronkok Here's what it looks like, switching back to top setting:

The only difference is the vertical shift in the a\raisebox{0.5em}{b}.

I believe the issue is Movement Of the Line Box’s Baseline (quirks of vertical-align). Still grokking...

edemaine · 2017-05-16T02:47:10Z

Upon further investigation, I think the top version (essentially @ronkok's version) was actually correct. There is a larger space at the top of \text{a} simply because of the overall line height. This margin disappears even in a simple example like a^{a^{a^a}}:

So I've reverted to that version, and added a screenshot test. I think we're back to good-to-go.

k4b7 · 2017-05-16T02:54:16Z

@edemaine you may want to use the texcmp docker to compare the output against LaTeX's output.

edemaine · 2017-05-16T03:18:03Z

One more question: I currently call the parse node "transform", with the thought that it could be used for rotatebox and such in the future. But I wonder if just calling it "raisebox" and keeping it simple would be better. Let me know your opinion.

k4b7 · 2017-05-16T03:37:39Z

Good question. I think having separate raisebox and rotatebox methods probably makes since we'll need to do something pretty different to implement rotatebox.

edemaine · 2017-05-16T13:27:10Z

@kevinbarabash Good call on using texcmp. I see now that LaTeX resets font size to full size when switching to text mode via \raisebox (which \text probably is careful to avoid), and I need to do the same:

I've changed the parser node name from "transform" to "raisebox".

edemaine · 2017-05-16T13:58:58Z

In principle I fixed the switch-to-normalsize issue, but I've run into a bug: #709

k4b7 · 2017-06-28T01:00:04Z

@edemaine I've merged the fix for #709. Do you know if there will be additional work after rebasing?

edemaine · 2017-06-30T17:52:22Z

I rebased. The switch to \normalsize is working now, but the bounding box computation seems completely broken (unphased by the \raisebox). In this texcmp, the red (KaTeX) raised b's intersect whatever is above them (\frac line or \sqrt line).

I get the sense that this is the result of some other changes... and presumably #746?

* Use calculateSize to support all currently supported units (ex, em, mu). * Shift height and depth in all cases.

edemaine · 2017-09-04T22:10:14Z

I got another chance to look this over and revise substantially. I believe this is now working almost perfectly. I use makeVList to do the vertical shift, so it should be well-behaved (and it appears at least reasonably so). Internal to buildHTML, I use sizing and text nodes to switch to the equivalent of \textrm{\normalsize...}. texcmp is almost perfect (slight horizontal spacing difference?):

I also slightly cleaned up the parse tree: text nodes were using a style attribute to specify a font, but styles mean something else in KaTeX; switched the attribute to font.

Probably best to look at this as a complete diff but if you prefer you can look at just the last commit for the changes relative to previous versions.

Out of date

k4b7

LGTM. I'm going to rename textFunctionStyles to fontFunctionStyles before merging.

k4b7 · 2017-09-05T00:32:06Z

src/buildHTML.js


 groupTypes.text = function(group, options) {
-    const newOptions = options.withFont(group.value.style);
+    const newOptions = options.withFont(group.value.font);


Calling withFont with a font makes more sense.

k4b7 · 2017-09-05T00:35:11Z

src/buildMathML.js


+// Transforms (translation/rotation) don't seem to have a representation
+// in MathML, so just treat them like \text{...}
+groupTypes.raisebox = groupTypes.text;


k4b7 · 2017-09-05T00:36:18Z

src/functions.js

        type: "text",
        body: ordargument(body),
-        style: textFunctionStyles[context.funcName],
+        font: textFunctionStyles[context.funcName],


Since we're calling these fonts now, should probably rename this to textFunctionFonts.

k4b7 · 2017-09-05T00:48:33Z

@edemaine thanks for picking this one up again and finishing it off. The results look great!

edemaine · 2017-09-05T02:44:38Z

Thanks @kevinbarabash for all the reviews today!!

ronkok · 2017-09-11T15:51:02Z

I think that one can get effective presentation MathML for \raisebox with code something like:

const node = new mathMLTree.MathNode(
        "mpadded", [buildGroup(group.value.body, options)]);
node.setAttribute("voffset", group.value.dy.value);

I have not tested that code, but I have put the HTML below into Firefox

<math xmlns="http://www.w3.org/198/Math/MathML">
    <mi>a</mi><mpadded voffset="0.5em">b</mpadded><mi>c</mi>
</math>

... and it comes out looking like this:

This comment is coming very late, I know. Sorry.

k4b7 previously requested changes Apr 29, 2017

View reviewed changes

k4b7 added the GH Review: needs-revision label May 13, 2017

edemaine force-pushed the raisebox branch from da32222 to 32ca75b Compare May 16, 2017 02:46

edemaine changed the title ~~Rough support for \raisebox~~ Support for \raisebox May 16, 2017

k4b7 added the blocked label Jun 5, 2017

This was referenced Jun 10, 2017

\mathop{x} should center single-character x #726

Closed

Feature request: support \colonequals or similar #657

Closed

Implement \coloneqq, \colonequals, etc. based on mathtools and colonequals #727

Merged

k4b7 removed the blocked label Jun 28, 2017

edemaine force-pushed the raisebox branch from 6f59009 to 3fb3cef Compare June 30, 2017 17:00

edemaine mentioned this pull request Jul 10, 2017

Add \LaTeX function #131

Closed

k4b7 mentioned this pull request Jul 30, 2017

Shrinkwrap vlists in table-like CSS. #768

Merged

edemaine force-pushed the raisebox branch from 3fb3cef to 345b766 Compare August 1, 2017 13:07

Rough support for \raisebox

ee857ae

edemaine added 5 commits September 4, 2017 17:18

Fix bounding box thanks to @kevinbarash and @ronkok

4b11831

* Use calculateSize to support all currently supported units (ex, em, mu). * Shift height and depth in all cases.

Add screenshot test

0b7b9fb

Rename transform -> raisebox

25cce36

Switch to normalsize in raisebox

bf901d4

Attempt at using makeVList

cd61165

edemaine force-pushed the raisebox branch from 345b766 to cd61165 Compare September 4, 2017 22:05

Significant rewrite, now working well

3e97ec3

edemaine added GH Review: review-needed and removed GH Review: needs-revision labels Sep 4, 2017

k4b7 approved these changes Sep 5, 2017

View reviewed changes

k4b7 added 2 commits September 4, 2017 20:38

Merge branch 'master' into raisebox

389e0e4

rename textFunctionStyles to textFunctionFonts

0a1c7f5

k4b7 merged commit ca224ed into KaTeX:master Sep 5, 2017

edemaine mentioned this pull request Sep 11, 2017

\raisebox in MathML #874

Closed

Uh oh!

Conversation

edemaine commented Apr 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edemaine commented May 15, 2017

Uh oh!

ronkok commented May 15, 2017

Uh oh!

edemaine commented May 16, 2017

Uh oh!

k4b7 commented May 16, 2017

Uh oh!

k4b7 commented May 16, 2017

Uh oh!

edemaine commented May 16, 2017

Uh oh!

ronkok commented May 16, 2017

Uh oh!

edemaine commented May 16, 2017

Uh oh!

edemaine commented May 16, 2017

Uh oh!

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

Uh oh!

Uh oh!

edemaine commented May 16, 2017

Uh oh!

k4b7 commented May 16, 2017

Uh oh!

edemaine commented May 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edemaine commented May 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

edemaine commented Jun 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edemaine commented Sep 4, 2017

Uh oh!

k4b7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k4b7 commented Sep 5, 2017

Uh oh!

edemaine commented Sep 5, 2017

Uh oh!

ronkok commented Sep 11, 2017

Uh oh!

Reviewers

Assignees

Labels

edemaine commented Apr 27, 2017 •

edited

Loading

k4b7 commented May 16, 2017 •

edited

Loading

edemaine commented May 16, 2017 •

edited

Loading

edemaine commented May 16, 2017 •

edited

Loading

k4b7 commented Jun 28, 2017 •

edited

Loading

edemaine commented Jun 30, 2017 •

edited

Loading