Skip to content
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

Fix rendering of '/' #66

Merged
merged 1 commit into from
Jul 3, 2022
Merged

Fix rendering of '/' #66

merged 1 commit into from
Jul 3, 2022

Conversation

jipolanco
Copy link
Contributor

This PR is an attempt to fix MakieOrg/Makie.jl#1967, by which the / character is incorrectly rendered in math.

Here is a small example showcasing the issue:

using CairoMakie
fig = Figure()
Label(fig[1, 1], L"k^{-5/3}")

In the current version, the text is incorrectly rendered as follows:

test_old

This PR changes the result to:

test_new

The issue seems to come from the / character being duplicated in the _latex_to_computer_modern dict:

_latex_to_computer_modern = Dict(
    # ...
    raw"/"                         => ("cmmi10", 0x3d),
    # ...
    raw"/"                         => ("cmsy10", 0x36),
    # ...
)

I don't know if this was intentional, but the fact is that Julia ends up keeping the second entry, which seems to have some issues. Concretely, the width of this character seems to be 0 in the font file (if I understand things correctly...) which leads precisely to the wrong rendering above. More explicitly:

using MathTeXEngine: tex_layout, FontFamily, unravel, LayoutState,
                     horizontal_layout, get_extent, texparse, hadvance

ex = texparse("5/3")
state = LayoutState(FontFamily())
tl = tex_layout(ex, state)

tl.positions
# 3-element Vector{GeometryBasics.Point{2, Float32}}:
#  [0.0, 0.0]  # '5'
#  [0.5, 0.0]  # '/' -> has zero width!
#  [0.5, 0.0]  # '3'

# Alternatively:
char = tl.elements[2]  # '/'
hadvance(char)  # = 0.0f0 (has zero width)

By removing this entry, the issue is fixed since the other entry (in the cmmi10 font) doesn't have this problem.

@codecov-commenter
Copy link

codecov-commenter commented Jul 3, 2022

Codecov Report

Merging #66 (ed42742) into master (e6bbaad) will increase coverage by 3.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #66      +/-   ##
==========================================
+ Coverage   78.71%   81.88%   +3.17%     
==========================================
  Files           7        9       +2     
  Lines         451      508      +57     
==========================================
+ Hits          355      416      +61     
+ Misses         96       92       -4     
Impacted Files Coverage Δ
src/engine/computer_modern_data.jl 100.00% <ø> (ø)
src/MathTeXEngine.jl 100.00% <0.00%> (ø)
src/parser/parser.jl 72.78% <0.00%> (+0.37%) ⬆️
src/parser/commands_registration.jl 84.50% <0.00%> (+49.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6bbaad...ed42742. Read the comment docs.

@Kolaru
Copy link
Owner

Kolaru commented Jul 3, 2022

That's very nice. Thanks a lot for finding the cause of the bug and fixing it !

Concretely, the width of this character seems to be 0 in the font file (if I understand things correctly...)

It is a bit more complicated than that. Character horizontal alignment is dictated by 3 values (see scheme below)

  • The bearing X (distance between the origin and the start of the ink)
  • The width (the width of the ink, inkwidth in MathTeXEngine)
  • The advance (distance between the origin and the origin of the next character, hadvance in MathTexEngine)

image

You can visualize them using what is provided in the prototype/prototype.jl (poorly documented currently)

  • Yellow: bearing X
  • Red: width
  • Green: additional space after the character (i.e. advance - (bearing + width)
  • Blue and lines: vertical information

image

That being said I am considering again using a different font that has more consistent glyph information, if I manage to fix the other problem it is causing.

@Kolaru Kolaru merged commit 11e9db5 into Kolaru:master Jul 3, 2022
@jipolanco
Copy link
Contributor Author

Thanks for the detailed explanation!

@jipolanco jipolanco deleted the jip/slash branch July 4, 2022 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

latex rendering of "/" is incorrect, which affects figure axis labels
3 participants