Hey @RazrFalcon, just looking over your API for use in KAS-text and I have a few comments.

I notice that Face is a wrapper around ttf-parser::Face, which KAS-text already has an instance of. I can supply the data pointer and face index no problem, but I could also directly supply a &'a Face (or possibly even just store the rustybuzz::Face the embedded ttf-parser::Face could still be accessed). But this isn't really important.

More significant, KAS-text stores text-runs with a FontId (index in internal list) plus a size (DPU aka pixels per font unit). I see your Face object embeds the size. For my purposes would storing a single Face and adjusting the size before each shaping run be the best option?

Your GlyphBuffer reports codepoint and cluster. KAS-text needs the text-index of each glyph (required for editing support), and trying to reconstruct this from codepoints is not ideal.

I see you use the same UnicodeBuffer ←→ GlyphBuffer model as HarfBuzz. I'm not sure how best to cache this type since it is consumed by value in the shape method. Maybe it would be better if UnicodeBuffer and GlyphBuffer were wrappers around Box<Buffer> to avoid large copies? Or does the optimiser avoid this issue anyway?

This ports all of GSUB and GPOS and uses ttf_parser's APIs for GDEF.

Notable changes:

• GDEF, GSUB & GPOS fully in Rust. rb_ot_face_t no longer knows about them.
• Ports the API exposed by OpenType layout (originally in hb-ot-layout.cc, now in src/ot/layout/mod.rs).
• Ports a small bit of kerning (from hb-kern.hh) because it depended on rb_ot_apply_context and rb_ot_apply_context::skipping_iterator_t which are stack-allocated and fully in Rust now, so FFI would have been complicated.
• Removes hb_set_t. This was apparently only used for lookup acceleration (basically collecting the coverage of lookups into sets), which I removed for now.
• Reorganizes buffer/glyph-info var allocation accessors.
• Harfbuzz did GDEF blocklisting for some fonts. Since I use ttf_parser's APIs for this, I couldn't port that.

Note that a lot of the original GSUB/GPOS code was unused from the beginning (originally used for harfbuzz's OpenType API). I removed all of that in the first commit.

This PR depends on a few changes in ttf_parser. Basically, two helpers to read data at an offset, making the get method on Offsets16 public and DynArray. These changes can be found here. For now, I rewired ttf_parser in rustybuzz's Cargo.toml to that fork. I didn't open a PR there for now because I'm not sure on how you plan to integrate with ttf_parser since the crates.io version doesn't have a public parser module.

I apologize for the PR being this huge :/

Okay, I ported the normalization. This one was already a bit trickier because it was more intertwined, especially with the compose/decompose function pointers through FFI.

Some observations:

• The existing complex shaper's compose/decompose functions used Rust's bool through FFI, which I think was incorrect. I changed it to rb_bool_t.
• I had to rename _rb_glyph_info_set_unicode_props to init_unicode_props in Rust because it would have clashed with the existing set_unicode_props. Furthermore, the logic is duplicated in Rust and C because there wasn't any GlyphInfo FFI so far and I didn't think it was worth it.
• I also had to do a mimimal port of rb_ot_complex_shaper_t to get some properties needed for normalization.

I think, maybe the main shaping logic could be ported next, but I have to look more closely whether there are still any obstacles that need to be taken care of before.

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.

In my code, I find a face across Face::glyph_index(ttf-parser) to shape "⚛", but failed. I found those different logic between ttf-parser and rustybuzz.

rustybuzz face only resolve glyph id by prefered_cmap_encoding_subtable.

    pub(crate) fn glyph_index(&self, c: u32) -> Option<GlyphId> {
        let subtable_idx = self.prefered_cmap_encoding_subtable?;
        let subtable = self.tables().cmap?.subtables.get(subtable_idx)?;
        match subtable.glyph_index(c) {
            Some(gid) => Some(gid),
            None => {
             ...
            }
        }
    }

And ttf-parser iterate all subtables

    pub fn glyph_index(&self, code_point: char) -> Option<GlyphId> {
        for encoding in self.tables.cmap?.subtables {
            if !encoding.is_unicode() {
                continue;
            }

            if let Some(id) = encoding.glyph_index(u32::from(code_point)) {
                return Some(id);
            }
        }

        None
    }

Is this a bug? A font face contain this glyph, but not use to shape it. Or I should use another way to find suitable font to shape?

It would be useful if the UNSAFE_TO_BREAK glyph flag were exposed in the public API so that layout algorithms can take it into account. From my understanding, it can enable line-breaking algorithms to avoid reshaping substrings multiple times or making simplifying incorrect assumptions such as that spaces do not participate in shaping.

It seems like the simplest approach would be to expose the mask field of GlyphInfo as a bitflags struct, but I wanted to check if this was a good approach before filing a pull request.

P.S. I'm very excited about this library; great work.

Here are a a few small improvements. I removed the unused buffer functions, used GlyphInfo::{as_char, as_glyph} for less try_from + unwrap, wrapped the AAT and kern functions I missed before and moved the horizontal-to-vertical char mapping from shape.rs to unicode.rs.

PS: Would you want an aat feature-gate upstream or not? Otherwise, I would just remove AAT from my fork. And do you plan to make a new crates.io release soon or only once AAT is done?

@RazrFalcon I've ported all the GSUB/GPOS-specific subtables (you can have a look here) and am now thinking about how to port the remaining code in the hb-ot-layout-* files. In doing that, I want to use some GDEF-related functions from ttf_parser. But it turns out that hb-ot-layout.h and hb-ot-layout.hh expose a lot of functions, which only take rb_face_t (and not rb_font_t). The ttfp_face, however, is stored in the font and not in the face and thus unreachable.

Now, I basically see two options:

• Either somehow move the ttfp_face into the rb_face_t (not that easy because rb_face_t is still in C++),
• or change a lot of occurences all the way up the call chain to rb_font_t and basically don't make the font/face distiction (which rustybuzz currently doesn't do anyway at the API level).

What do you think?

(If you have any other comments on the code I've produced so far, feel also free to review a bit.)

https://github.com/wrl/otf-fea-rs/ is a Rust-based compiler of FEA into GSUB/GPOS, while https://github.com/RazrFalcon/rustybuzz is a Rust-based OT shaper.

Perhaps @wrl and @RazrFalcon could somehow coordinate so that the two libraries can interoperate. No specific idea yet but I can well imagine this to be beneficial.

Since I'm excited to use this in a full-Rust WASM scenario, I decided to try and tackle one of the things on the roadmap that were marked as easy. I tried making the most straightforward, parallel adaption of the original C++ to make it not too hard to review. (Part of this is that I kept the order of functions as originally, even though I guess in Rust the general -> specific order is more common than the C-compiler-inflicted specific -> general.)

Some things I weren't totally sure about, I marked these with NOTE(laurmaedje). One particular thing I hope I didn't confuse are the Modified and Canonical combining classes.

Version 0.5.2 includes commit https://github.com/RazrFalcon/rustybuzz/commit/9e65c89c4dad60aa896ec3ed293773ce2a79d448 that bumps the ttf-parser dependency to a new major. Since ttf_parser::Face is publicly re-exported via the Deref impl for rustybyzz::Face, this results in build issues in downstream dependencies.

I understand that this conceptual problem and that the added module re-export is an attempt at solving this so that applications don't have to depend on ttf-parser with a version and rusty buzz with a possibly incompatible version anymore.

But meanwhile for any apps that use rustybuzz = "0.5" for example in their Cargo.toml and they end up using any ttf-parser functions, their build likely breaks as they also have ttf-parser = "0.15" in their Cargo.toml and that's now incompatible with 0.17 as per the above linked commit.

I'd be happy to elaborate on the concrete build issue, in case that's unclear. In terms of remedy I suggest to release 0.5.3 with ttf-parser downgraded again and release 0.6 of rustbyzz with the newer ttf-parser - in case you agree that this was a breaking release.

HarfBuzz’s hb_buffer_add_codepoints and friends allow specifying an offset and length from the string that was passed in. This allows HarfBuzz to have the full context of the string being shaped in order to perform proper shaping across runs. I’m not sure how feasible it would be for this crate to implement similar functionality, though.

Hi, I just watched the whole talk by Chris Chapman on the unicode text engine stack, but the term "subsetting" that appears in the rustybuzz README many times was still new to me. It sounded as if it was about settings, but searching the web made me believe that it's from "subsets" of fonts. Yet, the explanations I found were about fonts that contain subsets of glyphs, so it is still not clear to me why that would be a feature of a font shaping library.

If you give me an explanation, or point me to one, I can suggest a tiny(!) change to the README in a PR.

The Face::from_face is defined as:

    /// Creates a new [`Face`] from [`ttf_parser::Face`].
    ///
    /// Data will be referenced, not owned.
    ///
    /// Returns `None` when face's units per EM is `None`.
    pub fn from_face(face: ttf_parser::Face<'a>) -> Option<Self> {
        Some(Face {
            units_per_em: face.units_per_em(),
            pixels_per_em: None,
            points_per_em: None,
            prefered_cmap_encoding_subtable: find_best_cmap_subtable(&face),
            gsub: face.tables().gsub.map(SubstitutionTable::new),
            gpos: face.tables().gpos.map(PositioningTable::new),
            ttfp_face: face,
        })
    }

This function always returns Some and I believe the docs are invalid, as face.units_per_em() returns u16.

*** ERROR: ambiguous python shebang in /usr/share/cargo/registry/rustybuzz-0.5.0/scripts/gen-vowel-constraints.py: #!/usr/bin/env python. Change it to python3 (or python2) explicitly. *** ERROR: ambiguous python shebang in /usr/share/cargo/registry/rustybuzz-0.5.0/scripts/gen-universal-table.py: #!/usr/bin/env python. Change it to python3 (or python2) explicitly. mangling shebang in /usr/share/cargo/registry/rustybuzz-0.5.0/scripts/gen-unicode-norm-table.py from /usr/bin/env python3 to #!/usr/bin/python3 *** ERROR: ambiguous python shebang in /usr/share/cargo/registry/rustybuzz-0.5.0/scripts/gen-tag-table.py: #!/usr/bin/env python. Change it to python3 (or python2) explicitly. *** ERROR: ambiguous python shebang in /usr/share/cargo/registry/rustybuzz-0.5.0/scripts/gen-shaping-tests.py: #!/usr/bin/env python. Change it to python3 (or python2) explicitly. *** ERROR: ambiguous python shebang in /usr/share/cargo/registry/rustybuzz-0.5.0/scripts/gen-indic-table.py: #!/usr/bin/env python. Change it to python3 (or python2) explicitly.

https://download.copr.fedorainfracloud.org/results/remilauzier/rust-unicode/fedora-rawhide-x86_64/03707746-rust-rustybuzz/builder-live.log.gz

Fedora need a precise version of python to be declare to know which one to use.

I want push some chars into UnicodeBuffer for now, I have to collect into String then use push_str

however, push_str seems just decode it into chars so I think fn push(ch: char) method can be possible or at least fn push_chars(chars: impl Iterator<Item = char>) but I'm not sure about cluster

I'm storing a Face in an Arc that is shared across multiple threads. When I need to perform shaping, I call Face::clone and set the font size on the cloned Face. This clone is not entirely free because Face contains a Vec.

I would like for shape to take an additional argument that contains all of the mutable properties of a Face so that Face can be treated as immutable after construction. E.g.

struct FaceConfig {
    pub pixels_per_em: Option<(u16, u16)>,
    pub points_per_em: Option<f32>,
}

pub fn shape(face: &Face, config: &FaceConfig, features: &[Feature], buffer: UnicodeBuffer) -> GlyphBuffer {