tectonic-typesetting / tectonic

A modernized, complete, self-contained TeX/LaTeX engine, powered by XeTeX and TeXLive.
https://tectonic-typesetting.github.io/
Other
3.99k stars 162 forks source link

Port xetex_layout to Rust #1138

Open CraftSpider opened 9 months ago

CraftSpider commented 9 months ago

Been poking this on and off for a while, finally reached the point where it's... not done, but should pass most tests on Windows and hopefully Linux. Mac will require a bit more love before it works. Opening to see what CI has to say so far.

pkgw commented 9 months ago

Ooh, very exciting!

CraftSpider commented 9 months ago

I am so baffled by the linking errors on everything - they aren't happening locally, which makes them pretty hard to diagnose.

Mrmaxmeier commented 9 months ago

Yep, I saw undefined reference to symbol 'BrotliDecoderDecompress' in my CI logs and was also wondering what that's about. Probably some strange interaction with dependencies.. (?) I'll try poking at the CI container a bit.

CraftSpider commented 9 months ago

The icu/unicode stuff is probably because the library does symbol versioning, which you only disable normally on say system installs. I probably, somehow, am locally getting some system installed version pulled in which is providing the symbols. This is... kind of annoying, but I can fix it.

CraftSpider commented 9 months ago

Found a solution but it will require updates to vcpkg-rs - that PR will go up tonight or tomorrow, probably.

codecov[bot] commented 9 months ago

Codecov Report

Attention: Patch coverage is 64.92109% with 978 lines in your changes missing coverage. Please review.

Project coverage is 47.46%. Comparing base (82484db) to head (aeb720d). Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
crates/xetex_layout/src/manager.rs 45.19% 234 Missing :warning:
crates/xetex_layout/src/font.rs 63.00% 111 Missing :warning:
crates/xetex_layout/src/c_api/engine.rs 60.86% 108 Missing :warning:
crates/bridge_graphite2/src/lib.rs 39.53% 104 Missing :warning:
crates/bridge_freetype2/src/lib.rs 69.02% 70 Missing :warning:
crates/xetex_layout/src/manager/fc.rs 66.66% 58 Missing :warning:
crates/xetex_layout/src/c_api/font.rs 53.44% 54 Missing :warning:
crates/bridge_harfbuzz/src/font_funcs.rs 73.36% 49 Missing :warning:
crates/bridge_harfbuzz/src/lib.rs 85.71% 37 Missing :warning:
crates/bridge_freetype2/src/sys.rs 14.70% 29 Missing :warning:
... and 14 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1138 +/- ## ========================================== + Coverage 46.92% 47.46% +0.54% ========================================== Files 177 194 +17 Lines 65160 66713 +1553 ========================================== + Hits 30575 31668 +1093 - Misses 34585 35045 +460 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

CraftSpider commented 9 months ago

I should look into whether there's an existing CoreFoundation/CoreText library for mac. Since they're, well, core, it should be possible to use a standard sys crate for them without jumping through the hoops other dependencies need due to supporting so many ways of handling them.

pkgw commented 9 months ago

There's https://github.com/servo/core-foundation-rs ... the crates don't seem as popular as I'd have thought, but a Servo project is probably going to be trustworthy?

CraftSpider commented 8 months ago

@pkgw What's your take on dropping support for MacOS < 10.7, OSX Lion? It's more than 10 years old, and some quick googling says the number of mac users on it should be basically negligible. It would allow some minor simplification.

CraftSpider commented 8 months ago

Related note - the core-foundation-rs crate and similar from servo target 10.7 and above, so it would allow using them.

pkgw commented 8 months ago

@CraftSpider Dropping support for the older macOS seems fine to me. It generally becomes intractable to support the older OSes without paying for your own custom CI setup, anyway. It looks like conda-forge is currently targeting >=10.9, and I'd generally lean towards copying them.

CraftSpider commented 7 months ago

This is a big change - if desired, I can split it out into the primary change to Rust, then the individual library wrappers as follow-ups to the big change, as I made sure to get tests passing between each major change.

CraftSpider commented 3 months ago

Obviously I'm biased, but I'm tempted to pull in my crate enrede to replace icu at some point. It means both one fewer allocation for recoding strings, but also would allow replacing most usages of the std CString with enrede's CString<Utf8>, an encoding-strict type that is null terminated, but safe to turn to a &str for easy use in Rust code.