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 fontc functional for wasm users #1215

Open
simoncozens opened this issue Jan 24, 2025 · 6 comments
Open

Make fontc functional for wasm users #1215

simoncozens opened this issue Jan 24, 2025 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@simoncozens
Copy link
Contributor

fontc uses fontbe which uses fea-rs which has a structure Source which stores feature file paths as OsStrings. fea-rs uses the serde feature to serialize these paths for fontbe to do persistable job orchestration.

However, serde does not implement Serialize and Deserialise for OsStrings on the WASM platform, only Unix and Windows, so the whole thing falls down. We could replace it all with Strings to work everywhere.

Additionally, fontbe/src/feature.rs has the concept of an InMemoryResolver which sounds perfect for things which don't have filesystems! Except that it also stores content_path: OsString, which makes it dependent on filesystems again.

@simoncozens simoncozens changed the title Pervasive of OsString causes problems for wasm Pervasive use of OsString causes problems for wasm Jan 24, 2025
@davelab6
Copy link
Member

This may be a blocker for using fontc within fontraweb; @rsheeter how important does this sound to you

@simoncozens
Copy link
Contributor Author

In general: attempting to build fontc and its dependent libraries with --target wasm32-unknown-unknown from time to time is a great way to shake out unintended assumptions.

@cmyr
Copy link
Member

cmyr commented Jan 31, 2025

Yea, we could definitely get rid of OsStr; the basic idea is that in general we're dealing with paths, but we wanted something that would also work for arbitrary identifiers for the in-memory case, and so I went with OsString. This is easy enough to fix though, if it's a problem!

@rsheeter
Copy link
Contributor

rsheeter commented Jan 31, 2025

This may be a blocker for using fontc within fontraweb; @rsheeter how important does this sound to you

"may be" or definitely is? - I'd have thought definitely is.

I think it's worth doing but lower priority than making the brand fonts identical on https://googlefonts.github.io/fontc_crater/. If that sounds wrong please lmk.

attempting to build fontc and its dependent libraries with --target wasm32-unknown-unknown from time to time is a great way to shake out unintended assumptions

+1 to this, lets systematically work our way through making sure that works and adding it to CI. I imagine we'd want to start with the fontations libraries. I will rename this issue to reflect the more general problem.

EDIT: apparently we had this idea already for fontations, https://github.com/googlefonts/fontations/blob/main/.github/workflows/rust.yml#L125

@rsheeter rsheeter changed the title Pervasive use of OsString causes problems for wasm Make fontc functional for wasm users Jan 31, 2025
@rsheeter rsheeter added the enhancement New feature or request label Jan 31, 2025
@simoncozens
Copy link
Contributor Author

Adding CI to fontations is a good idea, but I don't anticipate any problems there as we are using read-fonts/write-fonts/skrifa heavily already in Fontspector web (yes it still has Fontbakery branding but it's all Rust underneath).

@rsheeter
Copy link
Contributor

rsheeter commented Feb 4, 2025

As of #1239 cargo build -p fontc --target wasm32-unknown-unknown succeeds and has been added to CI to ensure it remains so. I have not verified anything actually works in wasm. Baby steps :)

@rsheeter rsheeter self-assigned this Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants