Port main shaping logic. - Githubissues

harfbuzz / rustybuzz

A complete harfbuzz's shaping algorithm port to Rust

MIT License

551 stars 37 forks source link

Port main shaping logic. #18

Closed laurmaedje closed 3 years ago

laurmaedje commented 3 years ago

This ports the main shaping logic, including the map, shape plan and complex shaper data structures. Basically, the only things that remain in C++ are AAT and kerning.

Notes:

I merged rb_shape_plan_t and rb_ot_shape_plan_t into a single thing because the shape plan was just a thin wrapper around the OpenType shape plan.
There is a new aat module, but that's just a thin layer between Rust and C++. The AAT-Map is stack allocated in the shape plan on the Rust side and accessed on the C++ side. I took care that both sides have the same memory size, but I don't know if there are any pitfalls with doing it this way.
Shaper-specific data is stored in a Box<dyn Any>. This means that the data has to be 'static. This was mostly not a problem, except for the indic shape plan that had a reference to some lookups in the ot_map. Because the data and map are both stored in the global shape plan, this didn't work anymore (in safe Rust, without pinning or some other trickery), so I replaced the lookup slice with a Range<usize>.
I moved fallback shaping and normalization from the ot module into the root module because from what I saw in the shaping logic they didn't seem to be ot-specific.
The coords vector in Face wasn't needed anymore since I can now use ttfp_face.variation_coordinates().
I removed a lot of pub(crate) that was spread onto everything that touches a Buffer. Since the Buffer is not re-exported, normal pub is fine I think.
A lot of the rb_buffer_... functions are probably not needed anymore, but I kept them for now.

RazrFalcon commented 3 years ago

Nice! Can you explain in more details why we need Box<dyn Any>?

RazrFalcon commented 3 years ago

Also, what's your plans on AAT? I would say that the kerx branch has like 50% done, so it should take a few days to finish.

laurmaedje commented 3 years ago

The Box<dyn Any> is needed because each complex shaper has custom data and that needs to be stored in the shape plan somehow. The C++ version used a void pointer for that and a create and destroy function pointer. I guess we could also use a big enum that can handle every shaper's data, that would save the allocation.

laurmaedje commented 3 years ago

Is the kerx branch somehow tested? If it's a lot of untested code I fear that it might be hard to debug (if some tests fail, otherwise great). And what's the relation of that code to the kerning code in ttf_parser?

RazrFalcon commented 3 years ago

Oh, I see. Yes, the enum is a more common way to do this. But I guess we can do refactoring later.

AAT is partly tested. Not as well as OpenType. You can target the current tests. We'll see what to do with tests that use licensed fonts later. Having a pure Rust hb implementation is far more important.

As for ttf_parser, you should basically ignore its kern implementation. The kerx module already has kern table copy-pasted from ttf_parser.

RazrFalcon commented 3 years ago

By the way, if you have any refactoring ideas - feel free to send a pull request. Right now, as you know, a lot of internals are not very idiomatic, because of the incremental porting process.

laurmaedje commented 3 years ago

It'd be nice to have this fully in Rust, but I don't really want to spend time on AAT right now since its rather niche. I think featuring-gating it for now is a reasonable way forward. I'm not sure what to do with kern. Either we also feature-gate it or move at least that over from ttf-parser.

RazrFalcon commented 3 years ago

I see. Then I will work on it in my spare time.