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

Make allsorts run on no_std #42

Closed
wants to merge 8 commits into from
Closed

Conversation

fschutt
Copy link
Contributor

@fschutt fschutt commented Feb 3, 2021

This is necessary to have a font shaper for embedded environments.

NOTE: WIP PR, some dependencies do not yet have no_std support, also compiling fails currently

TODO LIST:

  • enable no_std on brotli decoder (possible, but the documentation is poor)
  • enable no_std on zlib decoder (possible using miniz_oxide?)
  • get rid of std::io::Cursor traits, use either Vec<u8> or &[u8] where possible
  • wait for encoding-rs to merge no_std support (in progress)
  • remove rental crate (rental is not maintained anymore, also no no_std support!)

- most uses of HashMap were replaced by BTreeMap because HashMap
  does not exist on no_std (TODO: perf regression?)
- use miniz_oxide instead of libz-sys for zlib implementation
- use custom allocator for brotli decoding
- make "std" an optional feature, enabled by default
- put blanket std::error::Error impls behind std feature flag
@fschutt
Copy link
Contributor Author

fschutt commented Feb 5, 2021

Okay, now everything should at least compile on no_std - I am still not 100% sure about the brotli decoder and the zlib decoder - they compile, but they probably don't work yet (i.e. they'll just emit a ParseError). As far as I can see, the brotli decoder needs a custom memory allocator and the API for that is very cumbersome.

Since rental is not maintained anymore, I've published a fork with a custom feature that allows rental to run on no_std. I've not removed it because it's good for what it does.

@Aaron1011
Copy link

Aaron1011 commented Apr 22, 2022

EDIT - I mistakenly thought that @fschutt was part of the yeslogic organization. cc @adrianwong @brendanzab @wezm - the comment below concerns the allsorts-rental crate.

@fschutt Would it be possible for you to make a new release of allsorts-rental that include this commit from rental: jpernst/rental@61188eb. Doing so will allow allsorts-rental to continue to compile in future versions of Rust (after a backwards-compatibility hack is removed: rust-lang/rust#83125).

Background: It looks like this PR (or something based off of it) was published to crates.io as allsorts_no_std. It was then added as a dependency by https://github.com/fschutt/rust-fontconfig, which is used by https://github.com/LtPeriwinkle/mist.

Unfortunately, once the backwards-compatibility hack rust-lang/rust#83125 is removed from rustc, these crates will stop compiling. The simplest way to keep everything compiling would be to make a new point release of allsorts-rental that includes jpernst/rental@61188eb. Specifically, all of the commas added by that commit need to present (they were previously 'generated' by rustc itself, and the rental proc-macro expects them to exist).

Feel free to ask me if you have any questions about this. Thank you for your time.

@wezm
Copy link
Contributor

wezm commented Apr 23, 2022

Hi @Aaron1011 we (YesLogic) don't own the allsorts-rental and allsorts_no_std crates. It would be great if @fschutt could edit the READMEs and crate metadata to make this clearer but they seem to be busy.

@fschutt
Copy link
Contributor Author

fschutt commented Apr 23, 2022

Yeah, this PR is pretty old now, I think this can be closed for now. My original effort to make allsorts run on no_std was relatively pointless anyway, I wanted to reduce the binary size of azul, but in the end it's easier to use the upstream allsorts in order to not miss out on bugfixes. I could maybe rebase + re-publish (most of the PR is just replacing std::String with alloc::String), but I'm extremely busy atm.

The reason I like no_std crates is not just because of size, but also because of security. I can be relatively certain that allsorts doesn't contain malicious code if it runs on no_std and it just does font parsing. The more no_std, the better.

Yes, I did publish it as allsorts-no-std, since I needed an intermediate solution, but right now I am back to using the regular allsorts crate since I didn't manage to make azul run on no-std anyway (it needs access to the file system to load fonts + access to the windowing system).

The allsorts-rental is just a fork of the rental crate, since that crate was (is?) unmaintained.

@wezm
Copy link
Contributor

wezm commented Apr 24, 2022

Yes, I did publish it as allsorts-no-std, since I needed an intermediate solution

Any chance you could publish a new point release that changes the repo links etc. in the Cargo.toml and README to not point at our repo? A note about maintenance status and that it's fork would be super helpful for avoiding confusion too.

@wezm wezm closed this Apr 24, 2022
@Aaron1011
Copy link

@fschutt Would you be able to publish that point release? It would be a big help in removing the back-compat hack from rustc.

@fschutt
Copy link
Contributor Author

fschutt commented Jul 17, 2022

What back-compat hack? And yeah, sorry about not publishing a release, I forgot, sorry.

@wezm
Copy link
Contributor

wezm commented Jul 18, 2022

What back-compat hack?

I think it was this one from Aaron1011's comment:

the backwards-compatibility hack rust-lang/rust#83125

@Aaron1011
Copy link

@fschutt Sorry to keep bothering you - would you be able to publish that release? It would be a huge help in removing the backward-compatibility hack from rustc.

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

Successfully merging this pull request may close these issues.

3 participants