-
Notifications
You must be signed in to change notification settings - Fork 59
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
feat: Wasm32 target custom font loading #217
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I like this PR and in fact it does not conflict with the original font resolving PR, which focuses on resolving font related attributes in the SVG so that the users could get a hint about which font buffer to load. And this PR simply provides a way to load the buffer. |
I agree. Those are orthogonal changes! The only problem I see now is that the WASM file size increased by 600 KBs, probably because of the addition of the |
Thank you for your contribution, this is a great addition. I expect this wasm file to be generated and distributed independently, given the file size limitations for certain scenarios. I have been very busy lately and will look at this PR in April. |
@antmelnyk Can you add some test cases? |
Looking forward to seeing this being landed too, as we will likely to rely on this feature to solve vercel/satori#186 👍 |
@antmelnyk can you add some test cases covering this piece of code? Thanks. Lacking tests is what blocking this PR being merged. |
Hi. I'm sorry for the delay. I added the test. I hope it's OK. Please rerun all the necessary builds for release with the latest dependencies because I don't know precisely what must be run. There are a lot of build and bundle commands in package.json. |
@antmelnyk lint is failing |
You just need to run two sets of commands:
|
0c1e9f0
to
4263491
Compare
4263491
to
d72fe20
Compare
index.d.ts
Outdated
|
||
export type CustomFontsOptions = { | ||
fontsBuffers: Uint8Array[] // A list of raw font files to load. | ||
} & FontOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no fontsBuffers option in index.d.ts, which is currently specific to Wasm.
d72fe20
to
8479b51
Compare
There is no `fontsBuffers` option in `index.d.ts`, which is currently specific to Wasm. And, let's keep the original manually corrected type.
8479b51
to
39c8125
Compare
Great work. @yisibl can you please release a new version so we can start using fonts? 👍 |
@lasseschou Please wait for me to update the documentation and playground. |
This PR has been released to v2.5.0, please note that the option name has been renamed from |
Heyo!
I know there is an opened but a bit stale PR that improves font handling, but I feel like ResvgJS lacks a direct, essential font-loading support on the wasm32 target before going into some more complicated APIs.
What do you think about the solution presented in this PR? Dead simple, with no API breaking change, just new optional font options and the possibility to pass an array of font data.
Works like this:
In the future, more complex API can be built on top of it or just, well, replaced if there is a better solution at one point.