Fix math spacing, delimiters, radicals, and script fractions#156
Fix math spacing, delimiters, radicals, and script fractions#156AshtonSBradley wants to merge 13 commits into
Conversation
AshtonSBradley
changed the title
|
Hi @Kolaru This tests quite a lot of corner cases (coverage inspired by other font packages) and took quite a lot of manual iteration loops to converge on the current visual regression state where all fonts look pretty consistent and not obviously broken. Quite a few things were just broken for some fonts, or used weird glyphs. I also went to a fair bit of effort to make this a compact change to code. I hope it is useful to close a bunch of issues all at once. |
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #156 +/- ##
==========================================
+ Coverage 79.93% 84.63% +4.69%
==========================================
Files 10 10
Lines 658 898 +240
==========================================
+ Hits 526 760 +234
- Misses 132 138 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Looks like the main change in the ReferenceTests is the integral symbol become smaller. A minor shift in space between top of characters and bottom of square root vinculum. The rest look either unchanged or a little better to my eye. Expansion of the package ReferenceTest to improve coverage could also be a useful direction, but I didn't go there in this PR yet. |
Kolaru
left a comment
Kolaru
left a comment
There was a problem hiding this comment.
Thanks a lot @AshtonSBradley, this is fantastic!
Overall, things looks good to me, and I could even follow you change to the monster parser.
I have to admit that for the layouting part, I haven't necessarily looked deep into the formula, but I trust your reference sheet and the reference tests regarding position tweaks :)
I still don't fully understand why the script level needs to be tracked, I feel like that shrinking everything down should do the trick, but well... it doesn't.
Regarding the reference tests, it is expected to fail since you are fixing some of them.
I inspected them, and I think that there are only few regressions:
- Subscripts
Was it a conscious choice to align the sub and superscript for cases like V^1_2 and U_{ij}? LaTeX is definitely closer to the current version.
Also x_{y \leftarrow 0} subscript is aligned too low. I assume that it may be related to the fact that you changed both subscripts and spaced symbol logic.
- Square root
The \sqrt{\frac{1}{2}} looks really cramped. However, I think that this one can be fixed later, comparing with MathJax, it seems like already on master the alignment is off. So I'm fine with having both fixed at a later point.
| @@ -1,5 +1,5 @@ | |||
| _latex_to_new_computer_modern = Dict( | |||
| raw"\int" => 5930, | |||
| raw"\int" => 878, | |||
There was a problem hiding this comment.
I hope nobody will complain about this change.
I agree with the change, considering that the proper way to handle it would be to differentiate inline mode (math part of a line) and equation mode (where the full string is a single equation). However, this is outside the scope of the current PR.
| export @L_str | ||
|
|
||
| const _italic_correction_enabled = Ref(true) | ||
| const _unspace_binary_operators_heuristic_enabled = Ref(true) |
There was a problem hiding this comment.
I would remove the leading _: as far as I understand, it only makes sense to have them have Ref if they are intended to be modified by user, so I wouldn't mark them as private.
Still we should not export them, as they are advanced settings.
|
Thanks again for the detailed pass. I pushed an update addressing each point:
Updated visual artifacts:
Local checks pass:
|
|
Pushed a follow-up that makes subscript placement more generally slant-aware. The subscript rule is now based on the core glyph, not the subscript character class:
This removes the previous special case that pushed lower-case Greek subscripts outward just because the subscript itself was Greek. The tests now cover Updated artifacts: Local checks pass:
|
|
@Kolaru i think I have addressed all of your comments, I hope this is converging. |
|
That looks great! I have integrated you visual spacing sheet to the reference test, I will generate the new reference once this is merged. Everything looks good, I only have few small remarks left: 
Do you why the new spacing logic doesn't apply to Then, regarding |
|
Thanks @Kolaru, that was a very helpful catch. I pushed a small follow-up addressing both points:
I also regenerated the reference sheets after the update: Local checks pass: full test suite with |
3 participants
Fix math spacing, delimiters, radicals, and script fractions
Summary
This PR improves several related math layout regressions that show up in labels, scripts, and multi-font rendering:
f(t),g(x),(f)x, andη(t).\inf_x\tan(x)and\sup_x\tan(x).LayoutState.script_level, allowing script-style fractions and operator spacing to be tuned separately from top-level math.x^{\frac{1}{1+2}}andx_{\frac{1}{1+2}}.\langle,\rangle, vertical bars, and integrals.Closes #9.
Closes #95.
Closes #129.
Closes #142.
Explanation
The main layout change is to carry script depth in
LayoutState. Decorated expressions now lay out their lower and upper scripts in an incremented script state, which lets nested fractions and spaced operators use script-style rules without changing top-level layout.Subscript and superscript anchors are deliberately asymmetric. Subscripts are driven by the core glyph: slanted cores use the advance-width anchor so subscripts can tuck under italic lean, while upright/non-slanted cores use
max(hadvance(core), rightinkbound(core))to avoid ink collisions. Superscripts continue to usemax(hadvance(core), rightinkbound(core)), preserving the desiredV^1_2stagger where the subscript tucks inward but the superscript stays farther out.Italic/upright spacing still uses glyph ink bounds rather than only advance widths. Lower-case Greek symbols are marked as slanted for layout purposes, including symbols loaded through a font family's special-character path. Adjacent slanted glyphs also get a small ink-gap correction, covering cramped cases such as
k\xiwithout adding a broad operator-space rule.Named math-operator spacing now looks through
:decoratedand:underoverwrappers when the core is a:function. This applies the same thin-space rule to cases such as\inf_x\tan(x)and\sup_x\tan(x)while leaving symbolic operators such as\sum_x yout of that path. The existing no-space-before-opening-delimiter rule is preserved, so\inf_x(\tan(x))does not receive an extra gap before(.Delimiter and radical rendering falls back to the default math font when the selected font lacks a usable math glyph or has text-shaped delimiter glyphs. Delimiter sizing uses the visible contents to avoid over-scaling brackets and bars around a lone display operator, which allows the NewComputerModern long integral glyph to be restored without bringing back the oversized-delimiter artifact. Delimiters also compute a visual axis from non-operator, non-delimiter content so simple and nested brackets sit on the apparent expression midline; rule-containing delimiters are scaled around the chosen axis, with a small brace-specific cap to avoid over-growing
{α/β}solely because of the beta descender.Fraction rules are positioned from combined numerator/denominator ink bounds. This fixes the offset vinculum in reference fractions and keeps shortened script-fraction rules centered over the actual script contents. Square-root vincula keep a small right padding so
\sqrt{2}and\sqrt{\frac{1}{2}}do not look clipped or left-centered; the fractional square-root case remains on the larger radical variant, which looked better in the reference sheet.The parser/test helper change is small:
manual_texexprnow treats delimiter and punctuation tuple heads as leaves, matchingisleaf. That keeps layout code from needing to normalize parser/test helper artifacts.Reference Sheets
The extensive visual sheet was useful as a debugging workflow: math layout bugs are coupled, and looking across fonts/cases made it easier to catch second-order regressions quickly. Following review, the standalone local before/after helper was removed from this PR because it did not fit the package's reference-test workflow. The spacing cases are now integrated through the generated reference sheets instead.
Updated generated reference artifacts are available here:
basics/functions.pngspacing/Issue #129 math operator spacing.pngbasics/delimiters.pngbasics/square_roots.pngbasics/subsuper.pngTests
julia --project=/Users/braas09p/Dropbox/Julia/Dev/MathTeXEngine.jl test/runtests.jljulia --project=@runic -m Runic --check src/engine/layout.jl test/layout.jl test/runtests.jlpassed.julia --project=/Users/braas09p/Dropbox/Julia/Dev/MathTeXEngine.jl/reference reference/generate.jl.Notes
This does not attempt a full OpenType MATH-table implementation. The reference sheets include related cases from #93, #105, #110, #126, and PR #151 so they can be inspected, but this PR should not claim to fully close all of those issues.
This partially overlaps #61 by improving unary/operator spacing, but #61 also discusses superscript vertical placement, so I would not close it from this PR alone.
No new dependencies are added.
No public exports are added.