Draft implementation of wasm shaper #122

asibahi · 2024-07-03T14:43:41Z

Hello.

working on this

So I started a basic structure of the wasm runner and how to export functions to the wasm module. I am using the wasmtime crate.

The general workflow is this :

look for Wasm table. If it is there : run it.
the shape_with exported function should just always direct to shape_with_plan
if Wasm table is not there or is malformed , use shape_with_plan. instead like normal.

I already hit a couple of things I do not understand:

the API has a concept of scale, which rustybuzz (afaik) does not have. Some of the harfbuzz examples depend on scale. What would be the correct strategy to deal with this?
I have absolutely no idea how to deal with pointers. Using wasmtime APIs to "link" functions with pointers is giving me compiler errors. You can see what the compiler points are by uncommenting the marked lines. Something about traits?

This is a draft PR hoping for some feedback and help.

Final edit I swear:
The two places where I am looking at the functions and how they're used are :

The Design document
The harfbuzz-wasm crate. (yes that's the entire crate.)

The C++ implementation of the wasm api is here But C++ is black magic to me.

Cargo.toml

LaurenzV · 2024-07-03T14:56:18Z

the API has a concept of scale, which rustybuzz (afaik) does not have. Some of the harfbuzz examples depend on scale. What would be the correct strategy to deal with this?

How exactly do they depend on it? We indeed don't have scale, so no scale should be applied (or basically a "scale" of 1).

behdad · 2024-07-03T14:58:12Z

the API has a concept of scale, which rustybuzz (afaik) does not have. Some of the harfbuzz examples depend on scale. What would be the correct strategy to deal with this?

How exactly do they depend on it? We indeed don't have scale, so no scale should be applied (or basically a "scale" of 1).

Or rather, scale of UPEM in HB terms.

asibahi · 2024-07-03T14:59:17Z

the API has a concept of scale, which rustybuzz (afaik) does not have. Some of the harfbuzz examples depend on scale. What would be the correct strategy to deal with this?

How exactly do they depend on it? We indeed don't have scale, so no scale should be applied (or basically a "scale" of 1).

I haven't actually looked through the code , but this example uses optical size to render the glyphs.

behdad · 2024-07-03T14:59:23Z

This is nice, particularly because wasmtime seems to have a "fuel" concept to limit CPU usage:

https://github.com/bytecodealliance/wasmtime/blob/main/examples/fuel.rs

It generally looks like a better runtime than wasm-micro-runtime, which has several problems as a library.

asibahi · 2024-07-03T15:00:11Z

the API has a concept of scale, which rustybuzz (afaik) does not have. Some of the harfbuzz examples depend on scale. What would be the correct strategy to deal with this?

How exactly do they depend on it? We indeed don't have scale, so no scale should be applied (or basically a "scale" of 1).

Or rather, scale of UPEM in HB terms.

So I should just return the upem and be done with it? This leaves the issue of how to deal with pointers :D

behdad · 2024-07-03T15:01:16Z

the API has a concept of scale, which rustybuzz (afaik) does not have. Some of the harfbuzz examples depend on scale. What would be the correct strategy to deal with this?

How exactly do they depend on it? We indeed don't have scale, so no scale should be applied (or basically a "scale" of 1).

Or rather, scale of UPEM in HB terms.

So I should just return the upem and be done with it? This leaves the issue of how to deal with pointers :D

Yes. UPEM works I think. No idea re pointers I'm afraid.

RazrFalcon · 2024-07-03T16:00:17Z

the API has a concept of scale, which rustybuzz (afaik) does not have

Just ignore it.

I have absolutely no idea how to deal with pointers.

Me neither. I never worked with WASM.

asibahi · 2024-07-03T18:20:34Z

Gave more flesh to a few functions. Left plenty of todo!s tho. I'd appreciate someone looking at them.

For dealing with pointers and wasm memory I got thankfully some pointers (hehe) from @CryZe at the Rust Community Server.

src/hb/shape_wasm.rs

RazrFalcon · 2024-07-09T09:55:58Z

A smaller wasm dependency is for sure preferable.

RazrFalcon · 2024-07-09T09:57:16Z

src/hb/shape_wasm.rs

+unsafe impl bytemuck::Pod for CBufferContents {}
+
+#[cfg(test)]
+mod wasm_tests {


Can you move tests to the tests directory?

Done. But I am not sure if that's better for the Ruqaa test, as it now doesn't distinguish between the wasm code failing and faulty output.

How so? Can you elaborate?

It is not a very well formed thought, but the wasm shaper can fail in two ways (categories of ways, rather). Either shape_with_wasm returns with bad output, or it returns None, because of some wasm failure. Since the tests in the tests folder only test the public API instead of private functions, they always return, as they fall back to the regular shaper. So one might not be able to tell if the wasm module crashed or not.

Cargo.toml

tschneidereit · 2024-07-09T10:00:26Z

@asibahi this is very exciting! As a quick note, please don't hesitate to reach out to us in the Bytecode Alliance for any help we might be able to provide with Wasmtime. The Wasmtime maintainers are very responsive on our Zulip.

wasmtime brings Cranelift and all its baggage around. wasmi is much lighter as a dependency and can be compiled to no_std.

Incidentally, we're working on both of those! There's an RFC for adding an interpreter, and recently parts of the runtime gained support for no_std. The latter doesn't yet include Cranelift, but that can be fixed with some elbow grease.

We also have minimal embeddings getting the size of Wasmtime down to about 2MB right now, with a clear path towards the hundreds of kilobytes range if needed.

Having said all this, wasmi is an excellent interpreter, and I'm sure @Robbepop would be excited to help with integrating it, too!

Robbepop · 2024-07-09T12:10:43Z

Hi, I have just been made aware of this topic by @tschneidereit 's mention of me and haven't read everything (no time to do so rn). I am happy to help. Either solution, Wasmtime or Wasmi, should work fine since both mostly share the same API surface. It could even be possible to use both solutions simultaneously for their different strengths.

src/hb/buffer.rs

Cargo.toml

RazrFalcon · 2024-07-10T07:10:43Z

src/hb/shape.rs

@@ -20,7 +20,16 @@ pub fn shape(face: &hb_font_t, features: &[Feature], mut buffer: UnicodeBuffer)
        buffer.0.language.as_ref(),
        features,
    );
-    shape_with_plan(face, &plan, buffer)
+    #[cfg(not(feature = "wasm-shaper"))]


Shouldn't we check for the Wasm table here? I.e. this should be a runtime decision, not compile-time one.

If we have Wasm table, try shape_with_wasm, otherwise shape_with_plan.

This is what this code does. The Wasm table is the first thing shape_with_wasm checks for, and if it is not there it returns None. The compile time check here is for the featuree as shape_with_wasm does not exist then.

ِEdit: I moved it to inside shape_with_plan.

But if there is no Wasm table you exit the shaping immediately, instead of calling the regular shaper. No?
I would expect something like:

if has_wasm_table and run_wasm_shaper(): return run_regular_shaper()

yeah shape_with_wasm fails if there is no "Wasm" table. The fallback to regular shaper happens in shape_with_plan here. You can see the regular tests pass just fine with the feature turned on.

src/hb/shape.rs

src/hb/shape_wasm.rs

asibahi · 2024-07-11T12:26:00Z

Fixed the bug and now the Ruqaa Font tests works if we do not compare clusters.

That'll teach me to read the standard library's documentation.

src/hb/shape_wasm.rs

asibahi · 2024-07-11T13:24:04Z

One thing I learned about recently:

Is scale from harfbuzz (the one expcted by this API) related to pixels_per_em? the skrifa crate seems to imply as much. If that's so we can perhaps just return the pixels_per_em if set, and if not, return UPEM?

Also re: shape_with , doesn’t rustybuzz technically use the AAT shaper as well as OT?

RazrFalcon · 2024-07-11T18:18:31Z

Is scale from harfbuzz (the one expcted by this API) related to pixels_per_em?

Once again, there is no scale in rustybuzz. It was removed during porting. It should always be 1.

Also re: shape_with , doesn’t rustybuzz technically use the AAT shaper as well as OT?

In harfbuzz terminology this is still the ot shaper. See https://github.com/harfbuzz/harfbuzz/blob/main/src/hb-shaper-list.hh

behdad · 2024-07-11T18:52:00Z

Is scale from harfbuzz (the one expcted by this API) related to pixels_per_em?

Once again, there is no scale in rustybuzz. It was removed during porting. It should always be 1.

It should be always units_per_em. And it's NOT related to pixels_per_em.

asibahi · 2024-07-11T19:24:51Z

So I think this is done.

The full defined API in the design document is not implemented, but only the ones used by the examples.

Edit: should you merge this could you make it a squish merge 😁

RazrFalcon · 2024-07-12T07:11:09Z

Thanks! Will be released soon. Will see how it will go.

asibahi added 5 commits July 2, 2024 17:58

add .vscode to gitignore

6053e0b

WIP : initial wasm structure

40a8cf8

typofix

8deb543

implement some of the import functions

def1128

Merge branch 'RazrFalcon:master' into wasm_shaper

5160985

notgull requested changes Jul 3, 2024

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

give more flesh. plenty of todo!()s left

aaef802

asibahi added 2 commits July 3, 2024 21:44

slightly more wrangling

f080e29

more fleshing out wasm work. in need of review.

4865ba0

RazrFalcon reviewed Jul 4, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 4, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 4, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 4, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 4, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 4, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 4, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

asibahi added 6 commits July 4, 2024 13:36

different approach to pointers. unable to massage borrow checker yet

d5ef3cf

revise assumptions about memory allocations

ac25191

Added Test. Not failing but not producing the expected result

2202d8b

Merge branch 'RazrFalcon:master' into wasm_shaper

57da6f0

fixed potential bug. test still failing

06ae9dd

no functional changes, just make it easier to navigate.

ef0b838

RazrFalcon reviewed Jul 9, 2024

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

wasmi, change where tests live, general cleanup

1da938f

RazrFalcon reviewed Jul 10, 2024

View reviewed changes

src/hb/buffer.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 10, 2024

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 10, 2024

View reviewed changes

src/hb/shape.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 10, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 10, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 10, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 10, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 10, 2024

View reviewed changes

src/hb/shape_wasm.rs Show resolved Hide resolved

RazrFalcon reviewed Jul 10, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

RazrFalcon reviewed Jul 10, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

asibahi added 2 commits July 10, 2024 15:16

added log dep. worked around cloning things.

11a062b

found the bug. tests pass (mostly). Ruqaa works.

84b9f58

Robbepop reviewed Jul 11, 2024

View reviewed changes

src/hb/shape_wasm.rs Outdated Show resolved Hide resolved

src/hb/shape_wasm.rs Show resolved Hide resolved

should be done.

234ef56

RazrFalcon merged commit 388d78d into harfbuzz:master Jul 12, 2024
2 checks passed

asibahi deleted the wasm_shaper branch July 12, 2024 08:42

asibahi mentioned this pull request Jul 28, 2024

Caching ShapePlan and the Wasm Module/Linker harfbuzz/harfruzz#9

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft implementation of wasm shaper #122

Draft implementation of wasm shaper #122

asibahi commented Jul 3, 2024 •

edited

Loading

LaurenzV commented Jul 3, 2024

behdad commented Jul 3, 2024

asibahi commented Jul 3, 2024

behdad commented Jul 3, 2024

asibahi commented Jul 3, 2024 •

edited

Loading

behdad commented Jul 3, 2024

RazrFalcon commented Jul 3, 2024

asibahi commented Jul 3, 2024 •

edited

Loading

RazrFalcon commented Jul 9, 2024

RazrFalcon Jul 9, 2024

asibahi Jul 9, 2024

RazrFalcon Jul 11, 2024

asibahi Jul 11, 2024

tschneidereit commented Jul 9, 2024

Robbepop commented Jul 9, 2024 •

edited

Loading

RazrFalcon Jul 10, 2024

asibahi Jul 10, 2024 •

edited

Loading

RazrFalcon Jul 11, 2024

asibahi Jul 11, 2024

asibahi commented Jul 11, 2024

asibahi commented Jul 11, 2024 •

edited

Loading

RazrFalcon commented Jul 11, 2024 •

edited

Loading

behdad commented Jul 11, 2024

asibahi commented Jul 11, 2024 •

edited

Loading

RazrFalcon commented Jul 12, 2024

Draft implementation of wasm shaper #122

Draft implementation of wasm shaper #122

Conversation

asibahi commented Jul 3, 2024 • edited Loading

LaurenzV commented Jul 3, 2024

behdad commented Jul 3, 2024

asibahi commented Jul 3, 2024

behdad commented Jul 3, 2024

asibahi commented Jul 3, 2024 • edited Loading

behdad commented Jul 3, 2024

RazrFalcon commented Jul 3, 2024

asibahi commented Jul 3, 2024 • edited Loading

RazrFalcon commented Jul 9, 2024

RazrFalcon Jul 9, 2024

Choose a reason for hiding this comment

asibahi Jul 9, 2024

Choose a reason for hiding this comment

RazrFalcon Jul 11, 2024

Choose a reason for hiding this comment

asibahi Jul 11, 2024

Choose a reason for hiding this comment

tschneidereit commented Jul 9, 2024

Robbepop commented Jul 9, 2024 • edited Loading

RazrFalcon Jul 10, 2024

Choose a reason for hiding this comment

asibahi Jul 10, 2024 • edited Loading

Choose a reason for hiding this comment

RazrFalcon Jul 11, 2024

Choose a reason for hiding this comment

asibahi Jul 11, 2024

Choose a reason for hiding this comment

asibahi commented Jul 11, 2024

asibahi commented Jul 11, 2024 • edited Loading

RazrFalcon commented Jul 11, 2024 • edited Loading

behdad commented Jul 11, 2024

asibahi commented Jul 11, 2024 • edited Loading

RazrFalcon commented Jul 12, 2024

asibahi commented Jul 3, 2024 •

edited

Loading

asibahi commented Jul 3, 2024 •

edited

Loading

asibahi commented Jul 3, 2024 •

edited

Loading

Robbepop commented Jul 9, 2024 •

edited

Loading

asibahi Jul 10, 2024 •

edited

Loading

asibahi commented Jul 11, 2024 •

edited

Loading

RazrFalcon commented Jul 11, 2024 •

edited

Loading

asibahi commented Jul 11, 2024 •

edited

Loading