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

Adding support for more math characters #1832

Closed
oddhack opened this issue Nov 23, 2020 · 11 comments
Closed

Adding support for more math characters #1832

oddhack opened this issue Nov 23, 2020 · 11 comments
Assignees
Milestone

Comments

@oddhack
Copy link
Contributor

oddhack commented Nov 23, 2020

Looking at some closed issues, I think there is some willingness/ability to add additional characters to the default font for e.g. math support. If so, we'd find all of the following very useful for the Vulkan docs:

:harr: ↔
:cdot: ⋅
:downarrow: ↓
:elem: ∈
:land: ∧
:lor: ∨
:oplus: ⊕
:lceil: ⌈
:rceil: ⌉
:lfloor: ⌊
:rfloor: ⌋
:forall: ∀
:check: ✓

@mojavelinux
Copy link
Member

mojavelinux commented Nov 23, 2020

Are you expecting these in the primary font, or in the fallback font?

asciidoctor-pdf -a pdf-theme=default-with-fallback-font glyphs.adoc

The challenge with adding glyphs to the primary font is that we can only add characters that the font itself contains (in this case Noto Serif). (When I update the font, I'm not adding glyphs. I'm only not taking them away). So we have to check if it even has them.

The fallback font is missing glyphs for the *floor characters, but seems to have the rest.

@oddhack
Copy link
Contributor Author

oddhack commented Nov 23, 2020

Symbols in an inconsistent font is far better than empty boxes, so having them in the fallback font sounds like a solution that would accomplish what is needed without requiring you to act as a font designer. The lack of ceiling/floor symbols is significant though. ISTM they're present in M+ 1mn (per https://www.fontspace.com/m-1mn-font-f13602) and M+ 1mn is included in the default-theme.yml font: for 1.5.3 along with Noto Serif, so I'm not clear on why they're not rendering (unless your M+ 1mn also does not include these symbols?)

@mojavelinux
Copy link
Member

Right, I'm saying that M+ includes them in the full font, but we're losing the *floor glyphs when preparing the font for Asciidoctor PDF. So I can restore them. (We don't bundle the whole font because its enormous and makes the gem size explode).

I can definitely add these missing glyphs to M+ 1p (the fallback font), but I could also consider adding them in the monospaced font too (since it comes from the same family and may have them).

Do you want to use them in monospaced format or in regular format?

p.s. Keep in mind that you always have the option to use your own fallback font. The theming system allows you to extend the default theme to add an extra font with just a few lines of configuration.

@mojavelinux
Copy link
Member

FYI here's the script that subsets the fonts for inclusion in the gem to give you can get a sense of how this process works: https://github.com/asciidoctor/asciidoctor-pdf/blob/master/scripts/subset-fonts.pe

@oddhack
Copy link
Contributor Author

oddhack commented Nov 23, 2020

For our purposes the regular format is superior. I can see how to setup my own fallback font but then I'd be doing basically what you're talking about, except with far less knowledge of how to do it. Hopefully these symbols will be more generally useful than just the docs I edit (several other Khronos specs are also going to need them, at least).

@oddhack
Copy link
Contributor Author

oddhack commented Nov 23, 2020

Oh, I think both floor and ceil are missing from the fallback - not just floor.

@mojavelinux
Copy link
Member

Oh, I think both floor and ceil are missing from the fallback - not just floor.

I can confirm that. I meant to say floor and ceil.

@oddhack
Copy link
Contributor Author

oddhack commented Nov 30, 2020

FYI here's the script that subsets the fonts for inclusion in the gem to give you can get a sense of how this process works: https://github.com/asciidoctor/asciidoctor-pdf/blob/master/scripts/subset-fonts.pe

So trying to parse this without actually understanding the scripting language, to add characters to the M+ fallback font ("code_font"?), would the right thing to do be add SelectMore() statements for the relevant glyphs inside the "if script == "subset")" block?.

@mojavelinux
Copy link
Member

That is absolutely correct. The way the script works is that it starts with no selections, then it selects glyphs to include. Then it saves the font.

@oddhack
Copy link
Contributor Author

oddhack commented Dec 8, 2020

That is absolutely correct. The way the script works is that it starts with no selections, then it selects glyphs to include. Then it saves the font.

Thanks. Should I be working off the latest tag in the repo for purposes of generating our own font? I get a fatal error from the subset script when running from HEAD due to a missing (or more likely, misnamed) font.

oddhack added a commit to KhronosGroup/asciidoctor-pdf that referenced this issue Jan 11, 2021
This follows advice in

asciidoctor#1832

on how to add some missing characters in the M+ fallback font. I think
the floor/ceiling characters are the only ones not already present, but
my branch explicitly tags all the glyphs the Vulkan spec requires that
aren't in the bundled (non-fallback) fonts, for possible future
reference.

The change to the bundled font is very small - about 220 bytes - and I
hope it can be included with asciidoctor-pdf going forward.

I had problems building from the repository scripts, as I don't have
much of the infrastructure the entire repository expects. In addition
there was some sort of issue with a mis-named font in subset-fonts.sh
relative to HEAD at the time I did the change. Eventually I cobbled
together a shell script which modified subset-fonts.sh and let me build
the one font I needed (the M+ fallback) locally using fontforge, without
using podman, and have verified that works for us.
@mojavelinux mojavelinux self-assigned this Jan 22, 2021
@mojavelinux mojavelinux added this to the v2.0.0 milestone Jan 22, 2021
@mojavelinux
Copy link
Member

Resolved by commit 9432cb0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants