-
Notifications
You must be signed in to change notification settings - Fork 62
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
Use core maths #162
Use core maths #162
Conversation
Yep, looks good to me. |
This means ttf-parser now depends on core_maths & libm always. Part of the point of having a "libm" feature (or equivalent) is to opt into it when you need it for no-std compilation and to avoid additional dependencies otherwise for the (quite common) "std" case. Previously ttf-parser had no dependencies.
This is a true, but odd, point. This hasn't been solved here, 0.23 just does the equivalent of this for everyone unconditionally. What was solved was no-std users don't need to enable a feature at the cost of std users getting extra dependencies. |
Yeah, I know, unfortunately there doesn't seem to be a perfect solution for this, it's a trade-off. But as I mentioned above, I don't have a strong opinion on it, for me it's fine to undo the feature part. |
Yep it's a tradeoff, just seems worth considering for this crate as it is so lean on dependencies. Having the extra dependencies using "std" doesn't seem to affect the size of binaries afaics. But there is a minor regression in clean compile times. 0.22
master (0.23)
|
To my understanding The problem is simple: we need trigonometry functions for And if you have two crates, one of which depends on std ttf-parser and one depends on no_std ttf-parser - then you will get core_maths/libm anyway. If you have a better solution I will gladly accept it. |
I think the previous approach, having a feature, was mildly better.
Yes but as I said before this is an edge case where the downside is the same downside now applied to everyone. Using a feature, a crate downstream of ttf-parser can still also support no-std with a "libm" feature and use that to activate ttf-parser's feature. E.g. [features]
libm = ["dep:libm", "ttf-parser/libm"] So there isn't a good reason that the dependency tree will inevitably have no-std ttf-parser in it anyway as there is a pattern to activating it when necessary. I guess the argument for the ergonomics is that no-std users already have to set So imo having the feature is generally the best approach because it minimises dependencies. I've used this approach myself too (e.g. ab_glyph_rasterizer) and I don't think it causes a problem for no-std usage. But overall, I can live with either way. So I've probably banged on about this enough now! 😅 |
Would making |
So what I did here is that I always include the
core_maths
dependency and I removedno-std-float
, so that in order to enableno_std
mode, you can simply disable all default features and don't need to additionally enable a separate feature. The reason for this is that since features are additive, it's apparently bad practice to have an explicityno_std
feature.. Because if one crate depends onstd
ofttf-parser
and another onno-std-float
, then both features would be activated if a crates uses both of them as a dependency.If you want I can undo the last part, though.