-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support absolute TeX units #732
Conversation
src/buildHTML.js
Outdated
// Thus, multiplying a length by this number converts the length from units | ||
// into pts. Dividing the result by ptPerEm gives the number of ems | ||
// *assuming* a font size of ptPerEm (normal size, normal style). | ||
const ptPerUnit = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should include em
and mu
in this list as well.
src/Parser.js
Outdated
@@ -787,11 +787,8 @@ Parser.prototype.parseSizeGroup = function(optional) { | |||
number: +(match[1] + match[2]), // sign + magnitude, cast to number | |||
unit: match[3], | |||
}; | |||
if (data.unit !== "em" && data.unit !== "ex" && data.unit !== "mu") { | |||
throw new ParseError("Invalid unit: '" + data.unit + "'", res); | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should continue to throw here, but check if data.unit
is one of the keys in ptPerUnit
(assuming we add em
and mu
to it).
I think we'll want to eventually have a |
@edemaine thanks for this PR. I love how simple the changes are for this. |
Thanks @kevinbarabash . The decision to make
I'll save from squashing for easier review, but these commits should be squashed. |
Hey Erik, I think you forgot to |
I have a request. I ask that these proposed units be exposed to authors with names that are differentiated in some small way from the name of any pre-existing truly absolute unit. For example, expose the unit name |
@ronkok I understand your concern, but I for one would not be happy with nonstandard units. I want to be able to copy/paste LaTeX code between LaTeX and KaTeX and have them render roughly the same, up to scaling. I doubt I am alone... In my opinion, it's reasonable for units in KaTeX to mean LaTeX units, not CSS units. Another example: the following render with the same horizontal space in KaTeX and in LaTeX, because of the styles-don't-scale-ems rule I just learned about in LaTeX. (To test in LaTeX, you need to remove the braces on the \begin{matrix}
x_{\kern{1em}y} \\
x\kern{1em}_y
\end{matrix} By contrast, the following render with different horizontal spaces in HTML: x<sub><span style="padding-left:1em"></span>y</sub><br>
x<span style="padding-left:1em"></span><sub>y</sub> So even for the existing primary unit, I could see an argument that absolute units should render as absolute units. That is not the decision made by KaTeX so far (which already uses But at least in my personal applications, the relative treatment of units as in this PR would work great. |
I am all for relative units and I am very much in favor of a migration path from LaTeX. I just think that much confusion can be saved if we avoid using the same word to mean two different things. Poor @kevinbarabash will be doing user support for the rest of his life. Of course, this PR may really be just a fiendish plot to make it impossible for KaTeX to ever support truly absolute units. In which case you have my support. |
Hahaha, that made me laugh! I think I see what you're asking. There are a few possible long-term plans:
I think you had in mind Plan 4, which explains the idea of naming the units differently. I think I had in mind Plan 1, and maybe later extension to Plan 3. In this case, we don't need to name the units differently. Perhaps we should reach consensus on which plan(s) make sense in the long term. Personally I'm happy with Plan 1, even though it could lead to relevant support queries (kinda like the queries related to |
My vote would be for 2. It doesn't preclude doing any of the other options later. |
@kevinbarabash Ah. You do realize that's the opposite of what's implemented in this PR? |
@edemaine I mis-parsed the choices. Is there an option that does what's in this PR? I agree with @ronkok that some people might be confused that |
Sorry for the confusion. Option 1 corresponds to the PR (once I correct it to handle font sizes). It could later be extended to Option 3. |
Rebased, and separated out the handling of absolute and relative units using the new infrastructure from #719. All seems working, except I find that \begin{array}{l}
\mathrm H\kern 1em\mathrm H\\
\mathrm H{\large \kern 1em}\mathrm H\\
\end{array} now produces two identical spaces in KaTeX, but not in LaTeX. I think this is a bug, not from this PR, but introduced by #719. It's maybe not truly a bug because LaTeX gives |
Yes, I think I found it. Basically, as part of #719, I went through and changed mentions of
But there's more. TeX's notion of a quad (em) is different from CSS's. In CSS “em” just measures the current font size. In TeX, 1em is relatively larger for nominally smaller font sizes. So This branch has the necessary changes but I have not run screenshotter tests or added new ones so no PR at the mo
|
Cool, thanks for investigating! I don't think this is quite right yet, though. As far as I've found in LaTeX, I didn't know about the |
You're partially right, but it turns out it depends. (1)
Result: 1em always in textsize. (2)
Result: 1em of current style (scaled). (3)
Result: 1em of current size (scaled). 🤷♂️ |
src/units.js
Outdated
const calculateSize = function(sizeValue, options) { | ||
let x = sizeValue.number; | ||
if (sizeValue.unit in emPerUnit) { | ||
x *= emPerUnit[sizeValue.unit]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend making changes based on #755. em
and mu
should scale by the quad
font metric; em
should be additionally un-scaled if the current style is script/scriptscript.
src/units.js
Outdated
if (sizeValue.unit in emPerUnit) { | ||
x *= emPerUnit[sizeValue.unit]; | ||
} else if (sizeValue.unit === "ex") { | ||
x *= options.fontMetrics().emPerEx; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be xHeight
, and should be un-scaled in script/scriptscript.
src/units.js
Outdated
} else if (sizeValue.unit === "ex") { | ||
x *= options.fontMetrics().emPerEx; | ||
} else if (sizeValue.unit in ptPerUnit) { | ||
x *= ptPerUnit[sizeValue.unit] / options.fontMetrics().ptPerEm; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think this should be divided by options.sizeMultiplier
, so that \large
/script/scriptscript doesn't change the meaning of absolute units.
Finally got to rebasing this to include @kohler's em/ex/mu size fixes. I think this is ready to go now. Here's a texcmp output for a new screenshot, which unfortunately has a shift caused by the parentheticals being in |
P.S. @ronkok Your documentation looks good, but your (*) footnote is no longer accurate, as @kohler discovered above. In fact, |
It's hard to tell what's going on with the spacing for |
@edemaine what font size would this need to be rendered at to verify that |
src/units.js
Outdated
"dd": 1238 / 1157, // didot | ||
"cc": 14856 / 1157, // cicero (12 didot) | ||
"nd": 685 / 642, // new didot | ||
"nc": 1370 / 107, // new cîcero (12 new didot) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no î please ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, missed one! Thanks.
README.md
Outdated
Similarly, if you specify a length using LaTeX absolute units, | ||
such as <code>\rule{1cm}{1pt}</code>, it gets converted into the | ||
equivalent number of ems assuming a 10pt font, which will end up getting | ||
scaled according to your font size. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this isn't quite right? KaTeX might assume that the “base font size” of the .katex
span is 10pt, but the reader is likely to misunderstand what that means. In katex.less, the default .katex
rule raises the font size to 1.21em—that is, 1.21x the surrounding font size. Which means that KaTeX assumes that the base font size (1em in the surrounding context) is 8.264 TeX pt or something.
In a typical modern layout with 16px base font size, 1pt in KaTeX equals (16 * 1.21 / 10) = 1.936px (CSS px). Whereas 1 CSS pt always equals 1.333px. Weird!
This kind of makes me question this approach of mapping LaTeX units to relative CSS units. It would be better IMO for non-font-relative units—especially px
!—to match CSS's concepts. It feels strange and even maybe misguided to document extremely precise units (“1 TeX inch is exactly 72.27 TeX point!”) when the precision is relative to an imprecisely specified, and often even unknown, base.
But I don't feel that strongly and actually implementing non-font-relative units would greatly complicate KaTeX height & depth calculations.
Perhaps best would be to document the current implementation but describe it as provisional. For example, “KaTeX might change in the future to make absolute TeX units, such as cm
, in
, and px
, equivalent to the corresponding CSS units. We recommend using relative units.” This agrees with @ronkok's comment that absolute units kinda suck. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wondered if this documentation needed updating -- thanks for the comments! I also wasn't aware of the 1.21x scaling. I assume there's a reason for that, like KaTeX fonts integrate better with typical web fonts at that scale?
Anyway, I took a stab at rewriting, and justifying why this makes sense. Namely, "The end result is that any rendered KaTeX should be a scaled version of what LaTeX would do with a 10pt base font (e.g., \documentclass{article}
), where the scale depends on the CSS font-size
." This is the only definition that will satisfy this property. Personally, I think it's what I'd want -- I want to be able to copy/paste some KaTeX code into LaTeX (or vice versa), even code that uses absolute units, and have everything look relatively the same, just up to a global scale factor.
To me, mapping KaTeX pt
(say) to CSS pt
would not make sense, precisely because an author wouldn't have enough context when writing the LaTeX to know how that should be scaled to "look good". For example, maybe you make an equation look good, but then globally change the site's font-size. Now your formulas will look relatively bad.
On the other hand, I'd be fine with later adding a unit like css pt
that maps to CSS pt. Except that, as you say, this would make height computation difficult if not impossible to do on the server side.
src/units.js
Outdated
"nc": 1370 / 107, // new cîcero (12 new didot) | ||
"sp": 1 / 65536, // scaled point (TeX's internal smallest unit) | ||
// https://tex.stackexchange.com/a/41371 | ||
"px": 803 / 800, // \pdfpxdimen defaults to 1 bp in pdfTeX and LuaTeX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
px
is not a default TeX unit, and it might be more useful, and less surprising, to map px
one-to-one onto CSS px
than to this. 1 CSS px
equals 1/96in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
px
is built into pdflatex
. Maybe I'm biased because that's the only TeX format I use, but that seems pretty standard/default to me. I'd be OK with omitting px
(I frankly didn't realize it was valid in pdflatex
until working on this PR), if we think it's too confusing -- but I don't think it's a good idea for some TeX units to map to CSS units when some TeX units to map to TeX units (in some sense).
src/units.js
Outdated
if (sizeValue.unit in ptPerUnit) { | ||
// Absolute units | ||
scale = ptPerUnit[sizeValue.unit] // Convert unit to pt | ||
/ options.fontMetrics().ptPerEm // Convert pt to em |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the comment to say CSS em
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
@kohler Implemented or responded to your review comments. Thanks for the quick review! @kevinbarabash You should set the
|
Please accept the following alternate text for the “font size and lengths” section. I feel pretty strongly about this. The current text overpromises and is more detailed than the README warrants. Font size and lengthsBy default, KaTeX math is rendered in a 1.21× larger font than the surrounding context, which makes super- and subscripts easier to read. You can control this using CSS; for example: .katex { font-size: 1.1em; } KaTeX supports all TeX units, including absolute units like FWIW, this image shows the difference between a 1cm rule in KaTeX (black) and a 1cm rule using browser units (red), with the default browser font size. |
@kohler I like that wording, and will change it once I'm at a computer again. (Small planned change: the use of em to specify percentages seems like it might be counterintuitive, so I'll add a mention that the CSS example achieves 1.1x.) I'm OK with removing the "KaTeX aims to render a scaled version of what LaTeX would do", but I'm curious what is wrong with that. Do you disagree with the goal or the claim or both? |
Perhaps an edit?
I'm trying to be more precise with the word absolute. TeX has absolute units. KaTeX really doesn't. |
@edemaine: I hate to respond because at some level we do agree but
I think KaTeX does, and should, aim for LaTeX compatibility, but “KaTeX aims to produce the visual equivalent of LaTeX output at 10pt body font size scaled up to, by default, 1.21x the font size of the context” isn't a good goal or a useful claim. KaTeX aims to be a fast, useful library for rendering TeX-like math in the browser. Scaled cut-and-paste compatibility with arbitrary LaTeX documents is a different goal and a less good one. |
But it's your PR and if you feel strongly about the “KaTeX aims to render…” I approve of it. |
Some thoughts:
|
Sorry for the delay. I took a stab at revising the documentation. In particular, I left a statement about being a scaled version of LaTeX, but now it's just in the context of unit/kern rendering. Let me know if you see any other changes/comments! |
LGTM! |
The new screenshot is correct as per #732 and compared to Firefox.
This is a proposed fix to #706, as discussed in that thread.
Rationale: Absolute distances are used all over KaTeX already, such as
\arraycolsep
being defined to 5pt. But it's not actually 5pt -- it renders as half of the font size. As a math author, this is actually what I want -- I don't care about "real-world distances", I care about how I'd write it in LaTeX to get the relative spacing that I want. I have an intuitive sense of how much space I add when I write\\[0.5in]
(although I admittedly usually use em and ex units), and want roughly that much space, independent of what CSS font size or browser scaling they happen to get rendered in.Issues: One issue I ran into is where to put the table of units. Currently it is in
buildHTML
, which means the parser can't access it and so can't directly tell whether a unit is valid, so I moved validity testing tobuildHTML
. (This causes some current tests to fail.) Perhaps it would be better to move this somewhere else. My feeling is that there should be abuildGeom
that would be useful for outputting HTML and other "geometric" formats we support in the future (SVG, Canvas) but not MathML (sobuildCommon
doesn't make sense). Thoughts?I'm also missing any additional tests. Open to suggestions. I think the obvious thing would be a screenshot test, as it's essentially a visual effect. An example I found useful was
which taught me that the LaTeX em unit really doesn't change when switching style, only when switching font size. Not what I thought!