RazrFalcon / rustybuzz

A complete harfbuzz's shaping algorithm port to Rust
MIT License
498 stars 34 forks source link

Add features to `Cargo.toml` matching those of `ttf-parser`. #72

Closed eddyb closed 11 months ago

eddyb commented 1 year ago

I mostly wanted to be able to see the impact on compile times of making functionality optional. (I left opentype-layout mandatory as I assume that's the common baseline that most/all fonts will need)

The implementation isn't great, the way I used #[cfg]s gets a bit messy, but I mostly focused on getting it to compile w/ all possible combinations of features (there's still some warnings left tho).

I still need to try get some reliable build timings to compare the impact but I'm not so sure it's worth it, the bulk of the cost may still be tied to opentype-layout which isn't really negotiable.

At the very least, the glyph-names feature could probably be added simply for the reason stated in ttf-parser (IIRC, it's optional specifically because it has a non-trivial size cost).

RazrFalcon commented 1 year ago

I'm very iffy about this. This would only make testing more complicated. And people would assume you can easily disable variable fonts support, which I'm not sure even possible. And definitely not a use case a care to support.

I hate conditional compilation in general, because it complicates testing. And I don't think rustybuzz would benefit from those build features anyway.

PS; why do you even care about rustybuzz compilation times? Just for fan? Afaik it compiles pretty fast already. Sure, not ideal, but not minutes like with syn. 7sec for 30KLOC + deps - it's a pretty good result for Rust.

eddyb commented 11 months ago

After more testing attempts (in a downstream codebase taking 15-20s to build from scratch for me), I was able to see enough variance (due to the 12-way parallelism on this machine, I'm guessing, plus whatever other system-wide effects I'm getting) that I could e.g. get quicker builds with all features enabled than the minimal set (or rather, default-features = false, features = ["std"]).

Apologies for wasting your time with this, should've done more extensive testing between posting (kind of hard and annoying in general, given how much parallel builds differ from serial ones...).

7sec for 30KLOC + deps - it's a pretty good result for Rust.

I suppose it is! Having previously looked through -Z self-profile data (for both ttf-parser and rustybuzz), there wasn't really anything obvious standing out - any function that took too long to e.g. typeck, happened to be a big function, not much getting around that.