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

feat: Wasm32 target custom font loading #217

Merged
merged 7 commits into from
Aug 5, 2023

Conversation

antmelnyk
Copy link
Contributor

@antmelnyk antmelnyk commented Mar 20, 2023

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:

let resvg_wasm = await fetch("./index_bg.wasm");
await initWasm(resvg_wasm);

// Load font as buffer
const req = new XMLHttpRequest();
req.open("GET", "./Roboto-Regular.ttf", true);
req.responseType = "arraybuffer";

// Make sure it's a Uint8Array
const arrayBuffer = req.response;
const font = new Uint8Array(arrayBuffer);

const opts = {
  font: {
    fontsBuffers: [font],
  },
};

const resvgJS = new Resvg(svg, opts);

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.

@vercel
Copy link

vercel bot commented Mar 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
resvg-js ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 5, 2023 10:55am

@zimond
Copy link
Collaborator

zimond commented Mar 20, 2023

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.

@antmelnyk
Copy link
Contributor Author

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 text feature from Resvg. I wonder if something can be done about it...

@yisibl
Copy link
Owner

yisibl commented Mar 20, 2023

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.

@yisibl
Copy link
Owner

yisibl commented Apr 6, 2023

@antmelnyk Can you add some test cases?

@shuding
Copy link

shuding commented Apr 8, 2023

Looking forward to seeing this being landed too, as we will likely to rely on this feature to solve vercel/satori#186 👍

@zimond
Copy link
Collaborator

zimond commented May 10, 2023

@antmelnyk can you add some test cases covering this piece of code? Thanks. Lacking tests is what blocking this PR being merged.

@antmelnyk
Copy link
Contributor Author

antmelnyk commented Jun 6, 2023

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.
Also, don't forget that this increased the wasm size by a lot, so as discussed, you might want to create a separate bundle with a text feature.

@zimond
Copy link
Collaborator

zimond commented Jun 7, 2023

@antmelnyk lint is failing

wasm/index.d.ts Outdated Show resolved Hide resolved
wasm/index.d.ts Outdated Show resolved Hide resolved
wasm/index.d.ts Outdated Show resolved Hide resolved
@yisibl
Copy link
Owner

yisibl commented Jun 13, 2023

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.

You just need to run two sets of commands:

 npm run build && npm run test
 npm run build:wasm && npm run test:wasm

index.d.ts Outdated

export type CustomFontsOptions = {
fontsBuffers: Uint8Array[] // A list of raw font files to load.
} & FontOptions
Copy link
Owner

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.

@yisibl yisibl changed the title Wasm32 target custom font loading feat: Wasm32 target custom font loading Aug 5, 2023
There is no `fontsBuffers` option in `index.d.ts`, which is currently specific to Wasm.

And, let's keep the original manually corrected type.
@yisibl yisibl force-pushed the feature/browserFontLoading branch from 8479b51 to 39c8125 Compare August 5, 2023 10:55
@yisibl yisibl merged commit d94e43d into yisibl:main Aug 5, 2023
34 checks passed
@lasseschou
Copy link

Great work. @yisibl can you please release a new version so we can start using fonts? 👍

@yisibl
Copy link
Owner

yisibl commented Aug 24, 2023

@lasseschou Please wait for me to update the documentation and playground.

@yisibl
Copy link
Owner

yisibl commented Oct 16, 2023

This PR has been released to v2.5.0, please note that the option name has been renamed from fontsBuffers to fontBuffers.

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.

5 participants