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

Graphite font engine doesn't work #1183

Closed
CraftSpider opened this issue Apr 9, 2024 · 4 comments
Closed

Graphite font engine doesn't work #1183

CraftSpider opened this issue Apr 9, 2024 · 4 comments

Comments

@CraftSpider
Copy link
Contributor

As listed in the title. If one uses a font and requests the graphite engine, like \font\g="[LinLibertine_R_G]/GR", then multiple operations either don't work correctly, or cause errors.

  • Attempting to enable features using the :ss05 syntax leads to 'feature not found' even if the feature exists - this is due to values returned by gr_fref_label not being the 4-character font selector, but instead more like a human-readable tagline. Instead of ss05 they'll be 'ss05' Some description of the feature.
  • use \XeTeXisdefaultselector can segfault due to tag_from_lang returning a nullptr (expected, the null/invalid language has no string representation).
  • Other small issues - there's definitely at least one memory leak due to allocating memory then just dropping the pointer, and several other graphite methods are used in potentially odd ways.

I intend to fix these issues in #1138, but I also wanted to ask basically - how much do we care? I discovered these issues writing tests for my rewrite, but apparently literally no one has uses the graphite engine enough to find these incredibly easy to run-into bugs. fontspec just doesn't ever use the graphite engine, and some quick googling didn't find anyone using the /GR selector.

@CraftSpider
Copy link
Contributor Author

CraftSpider commented Apr 9, 2024

Updated note - the feature name thing is sort-of mentioned in the reference - https://mirror.mwt.me/ctan/info/xetexref/xetex-reference.pdf - it shows enabling small caps by doing Small Caps=True. So it's not exactly a bug, but it definitely doesn't match how other things tell you to do it - other programs say to do :scps=True or :scps=1, so this behavior is at least non-standard. And some fonts will, using this format, require :'scps' Small Caps=True which is just silly.

@pkgw
Copy link
Collaborator

pkgw commented Apr 10, 2024

I think that it would be good to try to fix these things up — they sound like pretty manageable issues? Also, I believe that Graphite was created to help with the rendering of rare scripts used by small language communities, which seems like a noble goal to me ... and one that makes me reluctant to put as much weight as I normally would on the question of how frequently the capability seems to actually be used. (You might read about SIL International, who I believe also helped fund general XeTeX development; part of their reason for supporting this stuff is to enable proselytization, though ...) Also, if I understand correctly, I think that the engine will automatically choose to use Graphite as the engine for certain fonts, so I'm not sure if Googling will easily diagnose how often it's used? Although the big bugs that you're encountering certainly imply that it doesn't get much usage.

Granting all the above, if those rare scripts can now be handled by mainstream font engines, then maybe Graphite has simply outlived its usefulness, in which case I would worry a lot less about removing support for it. It certainly does not seem to be very actively developed.

If feasible, I think that it would be great to put any Graphite fixes in their own PR(s), since I think it will be pretty easy to lose them amidst everything else going on in #1138. I haven't been following the details of #1138 though and I trust your judgment about whether that would be a reasonable thing to attempt.

@CraftSpider
Copy link
Contributor Author

It should be pretty feasible to fix these without #1138, it's not terribly hard to at least improve them to not crash or do obviously wrong stuff. I'll try to get a PR up at some point with just the C code fixes for Graphite.

@CraftSpider
Copy link
Contributor Author

Fixed by #1179

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

No branches or pull requests

2 participants