3.3.2

[LaurenzV]

Status	Commit message	HB	Comments
⚪️	[ci] Try to fix the randomly failing valgrind job	Link
⚪️	[ci] Try harder to fix this randomly failing job	Link
⚪️	[repacker] don't infinite loop if visited or roots is in error.	Link
⚪️	[subset] Fix for fuzzer timeout.	Link
⚪️	[font] Add public API for slant	Link
⚪️	[GPOS] Apply font synthetic slant	Link
⚪️	[style] Adjust font slant angle for synthetic slant value	Link
🟢	Use invisible-glyph for spaces if font has no ASCII space	Link
⚪️	[hb-ot-var] Actually set in/out argument	Link
⚪️	[hb-ot-var] Specify normalized 2.14 docs	Link
⚪️	[face] Clarify face_index handling	Link
⚪️	[font] Load named-instance if face index top bits are set	Link
⚪️	[font] Mark hb_font_get_var_coords_design() non-experimental	Link
⚪️	[docs] Update	Link
⚪️	Fix test	Link
⚪️	Typo	Link
⚪️	[metrics] Scale up horizontal caret rise/run	Link
⚪️	[metrics] Implement synthetic slant for caret slope	Link
⚪️	[metrics] Harden math	Link
⚪️	[metrics] Only scale caret rise/run if font is slanted	Link
⚪️	[metrics] Simplify	Link
⚪️	[metrics] Ouch. Fix slant code	Link
⚪️	[docs] Add hb_font_[gs]et_synthetic_slant()	Link
⚪️	[metrics] Fix slant calc	Link
⚪️	Merge pull request #3338 from harfbuzz/slant	Link
🟢	Add hb_segment_properties_overlay()	Link
🟢	[buffer] Overlay segment-properties in hb_buffer_append()	Link
⚪️	[util] Move copy_buffer_properties() out of loop	Link
⚪️	[util] Simplify copy_buffer_properties()	Link
⚪️	[util] Copy unicode_funcs in copy_buffer_properties()	Link
🟢	[buffer] Clean up internal state bookkeeping	Link
⚪️	[buffer] Add hb_buffer_create_similar()	Link
🟢	[buffer] Group shape-related members together	Link
🟢	[buffer] Add enter()/leave() pair around shape()	Link
⚪️	Merge pull request #3353 from harfbuzz/buffer-create-similar	Link
🟢	[buffer] Rename swap_buffers() to sync()	Link
⚪️	[failing-alloc] Make it compile as C++ as well	Link
⚪️	docs: Add some details about coordinates	Link
⚪️	Fix unintentional locale dependency (#3358)	Link
⚪️	[map] Actually use k/v invalid types for declaration!	Link
⚪️	Revert "[map] Actually use k/v invalid types for declaration!"	Link
⚪️	Use freetype from CMake target when present (#3361)	Link
⚪️	[map] Map == / != use correct types	Link
⚪️	[algs] Call std::hash from hb_hash()	Link
⚪️	Clean up HB_NO_SETLOCALE	Link
⚪️	color: Document empty returns (#3362)	Link
⚪️	docs: Clarify variation apis (#3363)	Link
⚪️	Fix compiler warning	Link
⚪️	More macro cleanup	Link
⚪️	Preserve glyph props	Link
⚪️	Rename variable	Link
⚪️	Rename method	Link
⚪️	[doc] Fix hb_font_set_synthetic_slant param name	Link
⚪️	[git.mk] Update	Link
⚪️	docs: Fix a typo	Link
⚪️	docs: Add some details	Link
⚪️	[subset] fix fuzzer timeout if visisted_paint goes into error.	Link
⚪️	[algs] Fix hash chaining to std::hash()	Link
⚪️	[map] Massage some more	Link
⚪️	Fix the docs build	Link
⚪️	[map] Construct objects	Link
⚪️	[map] Remove constexpr invalid items	Link
⚪️	[algs] Add hb_coerce()	Link
⚪️	[check-static-inits] Only check library object files	Link
⚪️	[map] Allow invalid items to be pointer to static object	Link
⚪️	[test-map] Add disabled tests with std::string	Link
⚪️	Convert fallback kwargs to [provide] entries.	Link
⚪️	[map] Destruct objects	Link
⚪️	[meta] Replace hb_is_pointer with std::is_pointer	Link
⚪️	[meta] Use std::is_reference instead of hb_is_reference	Link
⚪️	[meta] Use std::is_const instead of hb_is_const	Link
⚪️	[meta] Remove hb_add_const	Link
⚪️	[meta] Use std::addressof() instead of hb_addressof()	Link
⚪️	[map] Fix bad memory access if hb_map.fini() was called twice.	Link
⚪️	[map] Correct previous commit	Link
⚪️	[meta] Remove unused hb_ref()	Link
⚪️	[meta] Include	Link
⚪️	[vector] Move semantics when resizing	Link
⚪️	Further adjust setlocale	Link
⚪️	Fix previous commit	Link
🟢	Fix various typos	Link
⚪️	[algs] Add default-construtor to hb_pair_t	Link
⚪️	Try fix Mac build	Link
⚪️	Better try at previous commit	Link
⚪️	Make hb_coerce static inline	Link
⚪️	[vector] Add XXX markers for remaining places that need work	Link
⚪️	[vector] Start adding destruction	Link
⚪️	[vector] Construct items when enlarging	Link
⚪️	[vector] Destruct in pop()	Link
⚪️	[vector] Move semantics in vector remove()	Link
⚪️	[vector] Adjust construction criteria	Link
⚪️	[repacker] Replace fini_deep() with fini()	Link
⚪️	Merge pull request #3376 from harfbuzz/auto-vector	Link
⚪️	[cff] Remove unneeded init/fini	Link
⚪️	[bimap] Remove init/fini	Link
⚪️	[str_buff_vec_t] Remove unused fini method	Link
⚪️	[subset-cff2] Drop an constructor/destructor pair	Link
⚪️	[subset-cff1] Remove a constructor/destructor pair	Link
⚪️	[subset-cff] Remove another set of fini_deep	Link
⚪️	[subset-cff] Convert subr_closures_t to constructor/destructor instead of init/fini.	Link
⚪️	[vector] Add TODO Emplace?	Link
⚪️	[cff] Remove init/fini from blend_arg_t	Link
⚪️	[cff] Remove some more fini_deep()	Link
⚪️	[meson] add icu DEFS required for compilation	Link
⚪️	[vector] Remove .fini_deep()	Link
⚪️	[cff] Remove init/fini from number_t	Link
⚪️	[cff] Remove op_str_t nop init/fini	Link
⚪️	Merge pull request #3381 from harfbuzz/clean-vector-use	Link
⚪️	[subset] Fix bound check when setting overlap bit.	Link
🟢	Add test for #2516	Link
🟢	Test for #2140	Link
⚪️	meson: Enable big objects support when building for windows	Link
⚪️	Merge pull request #3365 from harfbuzz/gdef-fix	Link
🟢	[gpos] Fix unsafe-to-break of mark-attachment	Link
⚪️	[ms-feature-ranges] Use preferred vector search API	Link
⚪️	[vector] Remove old find() method	Link
⚪️	[vector] Add sorted template argument	Link
⚪️	[vector] Merge sorted-vector into vector	Link
⚪️	[coretext] Fix lsearch	Link
⚪️	Merge pull request #3386 from harfbuzz/unify-sorted-vector	Link
⚪️	[ms-feature-ranges] Inline code in header file	Link
⚪️	[ms-feature-ranges] Pass reference to cmp function	Link
⚪️	[test] Add --single-par to more places in hb-aots-tester [ci skip]	Link
⚪️	[machinery] Make accelerator lazy-loader call Xinit/Xfini	Link
⚪️	[repacker] Fix missing initilization of obj in vertex_t.	Link
⚪️	[machinery] Move accelerators to constructor/destructor	Link
⚪️	Merge pull request #3392 from harfbuzz/auto-accelerators	Link
⚪️	Clean accelerators a bit more	Link
🟢	[PairPos] Split GPOS kerning to both sides (#3235)	Link
⚪️	Fix failing Mac test for previous commit	Link
⚪️	One more fix	Link
⚪️	[post] Initialize variables	Link
⚪️	[gpos] Fix conditional	Link
⚪️	[cff] Initialize accelerator members	Link
🟢	[buffer] Add HB_GLYPH_FLAG_UNSAFE_TO_CONCAT	Link
⚪️	Implement hb-shape --verify unsafe-to-concat flag	Link
🟢	[unsafe-to-concat] Add annotations to GPOS and kern	Link
🟢	[unsafe-to-concat] Add to GPOS kerning	Link
🟢	[gsubgpos] Combine input/backtrack/lookahead unsafe-to-concat	Link
🟢	[buffer] Rename _unsafe_to_break_set_mask to _infos_set_glyph_flags	Link
🟢	[buffer] Add default cluster value in find_min_cluster	Link
🟢	[buffer] Consolidate glyph-flags implementation	Link
🟢	[gsubgpos] Adjust chaining unsafe-to-concat application	Link
🟢	[unsafe-to-concat] Mark entire buffer unsafe-to-concat if kerx format2	Link
🟢	[unsafe-to-concat] Fix PairPos2 logic	Link
🟢	[unsafe-to-concat] Adjust Arabic joining logic	Link
🟢	[unsafe-to-concat] Further adjust Arabic joining logic at boundary	Link
🟢	[unsafe-to-concat] Adjust "interior"ness of "from_out_buffer"	Link
🟢	[unsafe-to-concat] Adjust CursivePos	Link
🟢	[unsafe-to-concat] Adjust Arabic-joining start boundary condition more	Link
🟢	[unsafe-to-concat] Adjust GPOS lookbacks	Link
🟢	[unsafe-to-concat] Adjust MarkBasePos	Link
🟢	[unsafe-to-concat] Mark LigatureSubst	Link
🟢	[unsafe-to-concat] More annotations for MarkLigaturePos	Link
🟢	[unsafe-to-concat] More annotations for MarkMarkPos	Link
🟢	[unsafe-to-concat] Adjust end conditions	Link
🟢	[unsafe-to-concat] Mark as unsafe in kern machine	Link
⚪️	[fallback-shape] Add buffer trace log	Link
🟢	Cosmetic	Link
⚪️	[unsafe-to-concat] Mark in all other shapers	Link
⚪️	[buffer] Oops	Link
⚪️	[test] Add test-serialize	Link
⚪️	[test-serialize] Assert len	Link
⚪️	[test] Remove HB_UNUSED	Link
🟢	[unsafe-to-concat] Clarify documentation as per feedback	Link
⚪️	Merge pull request #3297 from harfbuzz/unsafe-to-concat	Link
⚪️	[doc] Fix generation of hb_glyph_flags_t docs	Link
⚪️	Avoid redefinition of HB_NO_SETLOCALE in certain configs	Link
⚪️	[font] Fix build with no-var configs	Link
⚪️	[buffer] Make hb_buffer_append() take a const argument	Link
⚪️	[buffer] Add HB_BUFFER_FLAG_VERIFY	Link
⚪️	[config] Enable HB_NO_BUFFER_VERIFY in HB_LEAN	Link
⚪️	[fuzz] Verify shape results	Link
⚪️	[fuzz] Disable verification for now.	Link
⚪️	[buffer] Document HB_BUFFER_FLAG_VERIFY	Link
⚪️	[subset] convert active_glyphs_stack to be a vector of hb_set_t instead of hb_set_t*.	Link
⚪️	[subset] Fix for issue #3397.	Link
⚪️	Add the language system tag INUK	Link	already existed
🟢	Infer tag mappings for unregistered macrolanguages	Link
⚪️	Replace “[family]” with “[collection]”	Link
⚪️	[buffer] Whitespace	Link
⚪️	[util] Change "All shapers failed." message to "Shaping failed."	Link
⚪️	[verify] Show buffer input text when verification fails	Link
⚪️	[fallback-kern] Move buffer message to correct position	Link
🟢	Don't always inherit from macrolanguages	Link
🟢	Let BCP 47 tag "mo" fall back to OT tag 'ROM '	Link
⚪️	Merge pull request #3402 from harfbuzz/language-tags	Link
⚪️	3.3.0	Link
⚪️	[subset] Don't hold references to members of the active_glyph_stack.	Link
⚪️	3.3.1	Link
⚪️	[serialize] document how the serializer works.	Link
⚪️	[glyf] Don't store face in accelerator	Link
🟢	[buffer] Comment	Link
⚪️	[atexit] Allow hb_atexit redefinition	Link
🟢	[GPOS] Disable split-kerning	Link
⚪️	Revert "One more fix"	Link
⚪️	Revert "Fix failing Mac test for previous commit"	Link
⚪️	3.3.2	Link

[LaurenzV]

Currently stuck at harfbuzz/harfbuzz@84aa1a836. It seems to me like the code we currently have is somewhat different and I also don't understand what's going on there. I think I will have to start familiarize myself with how GPOS works instead of trying to brainlessly copy it, so it might be a while before I make progress on that.

[LaurenzV]

Or well, I guess I do understand what's happening more or less, but some of the parts I'm unsure what their equivalent in Rust is. For example the get_len method of ValueFormat.

[RazrFalcon]

We're talking about src/ot/position.rs PairAdjustment::apply, right?

[LaurenzV]

Yes, I think that's the corresponding code.

[RazrFalcon]

If you look at git blame, I'm not the one who wrote that code 😄
So I guess we have to figure out it together.

[RazrFalcon]

As for ValueFormat::get_len, this is ValueFormatFlags::size in Rust.

[RazrFalcon]

But it's private API...

[LaurenzV]

Ah! I didn't check ttf-parser...

[LaurenzV]

Okay, that helps already though, thanks. I'll still do some more background reading though and then give it another shot.

[RazrFalcon]

Basically, in harfbuzz we have a mix of shaping and font parsing at the same time here. But in rustybuzz it's all hidden it ttf-parser. And it works quite well.

But now we have to figure out if this change could be done on the rustybuzz side or should be implemented in ttf-parser first. I will take a look into it more to say for sure.

Don't worry, this part is known to be very complicated. And there is not much we can do about it. It's just inherent complexity of TrueType.

[RazrFalcon]

PS: the weird // Note the intentional use of "|" instead of short-circuit "||". comment is outdated. We can use || for flags here.

[LaurenzV]

PS: the weird // Note the intentional use of "|" instead of short-circuit "||". comment is outdated. We can use || for flags here.

Yes, I already fixed it locally. I had forgotten to remove it in the original commit.

[RazrFalcon]

As for:

if (valueFormat1 & !mask)
	  goto bail;

This should be:

// Or || ??? Not sure...
if records.0.x_advance == 0 && records.0.x_advance_device.is_none() {
    // goto bail;
}

Typical C-style bitflags magic...

And similar logic for Y-axis.

[RazrFalcon]

And the other stuff seems to be pretty straight-forward.

valueFormat1.apply_value() is records.0.apply()

applied_first is flag1

[RazrFalcon]

What harfbuzz doing here is parsing ValueRecord, but ttf-parser already parsed it. This structure is a bit insane. It's a dynamically sized collection of data of various types. Pretty absurd even by TrueType standards.

[LaurenzV]

Alright, took me many hours, but I think I got it now (at least the new test case passes). Not very pretty, but I don't see any better way to translate the goto statements and struct offset magic. Will push it (and prettify it) once I'm done with the remaining commits.

[RazrFalcon]

If it takes too much time and effort - just ask and I will try to do it myself. I don't think it would be much faster, most of the code I wrote was 3 years ago, but I don't want you to burn out immediately.

[RazrFalcon]

Jesus... Can you point how it looks in harfbuzz?

[LaurenzV]

Here:

https://github.com/harfbuzz/harfbuzz/blob/ac46c3248e8b0316235943175c4d4a11c24dd4a9/src/hb-ot-layout-gsubgpos.hh#L1205-L1230

That pattern appears a couple of times unfortunately (i.e. having arguments with default values), which doesn't exist in Rust... So in order to stay as close to the original as possible, I decided to use options instead and unwrap them with the default value in the method. Which results in a lot of clutter, but I think that's as close as it gets to the original code.

[RazrFalcon]

The changes looks fine at the first glance. And there we're a lot of changes... But if tests are passing - we're fine.

Use invisible-glyph for spaces if font has no ASCII space

Yes, the Rust code is very different, but here it is: https://github.com/RazrFalcon/rustybuzz/blob/7b5c622fcd33e0879b2dd99e4a436cff774af749/src/fallback.rs#L413-L416

But we don't have x_scale/y_scale anyway, so...

Add hb_segment_properties_overlay()

I don't think we have hb_buffer_append, so ignore.

[buffer] Clean up internal state bookkeeping

We don't have Buffer::reset, but we have Buffer::clear.
And it seems we should reset cluster_level as well.

[machinery] Make accelerator lazy-loader call Xinit/Xfini

Completely irrelevant.

[unsafe-to-concat] Adjust end conditions

I have no idea... Let's ignore for now.

[test] Add test-serialize

Irrelevant.

[buffer] Add HB_BUFFER_FLAG_VERIFY

Brrrr... Let's ignore it for now. It's an optional feature after all.
I think we should remove this flag from rustybuzz completely.

[RazrFalcon]

As for ttf-parser changes, I will look into them tomorrow. But aren't we decided to ignore HB_SPLIT_KERN?
Can you point where you use new ttf-parser API and corresponding harfbuzz code?

[behdad]

As for ttf-parser changes, I will look into them tomorrow. But aren't we decided to ignore HB_SPLIT_KERN?

Yeah. Ignore it.

[behdad]

[unsafe-to-concat] Adjust end conditions

I have no idea... Let's ignore for now.

This one port please if you can.

[LaurenzV]

This one port please if you can.

Would you mind explaining it to me? My understanding would be:
Before: if end is unintialized (= -1), then set it to len
After: always set it to the min of end and len, so if end is uninitialized (= -1), then it stays -1?

But that doesn't sound right...

[RazrFalcon]

@LaurenzV Also, while I know that porting harfbuzz is pain, I'm open to any ideas how we can simplify it. One I have in mind are:

1. Try to make code to look as close as possible to harfbuzz one. The current one is too idiomatic which causes only issues.
2. Try to use the same files structure?
3. Add original names to all the functions to simplify search.

When I was originally porting harfbuzz to Rust I've tried to make the code as idiomatic and as nice as possible, but it was a big mistake... It makes reading code way nicer than original, but hurts porting immensely.

[behdad]

Would you mind explaining it to me? My understanding would be: Before: if end is unintialized (= -1), then set it to len After: always set it to the min of end and len, so if end is uninitialized (= -1), then it stays -1?

But that doesn't sound right...

This is (unsigned) -1... The difference is if somehow end == len + 1; we want to set end to len or BAD things can happen.

[LaurenzV]

This is (unsigned) -1... The difference is if somehow end == len + 1; we want to set end to len or BAD things can happen.

Okay, thanks for clarifying! I added it.

[LaurenzV]

Try to make code to look as close as possible to harfbuzz one. The current one is too idiomatic which causes only issues.

Yes, definitely. Idiomatic code is nice, but only if we can stick close to the harfbuzz code, otherwise it will haunt us in the future... One example being the Matched struct that I had to dissolve in order to port this time's changes.

Try to use the same files structure?

In general yes, although for me that wasn't as big of a problem. I think it's more important that the functions are named the same (we don't even need to link the original harfbuzz function, just name it the same works I think). My two main ways of finding the corresponding code locations was 1. search the function names and, believe it or not, what actually helped most was searching for comments that were the same.

Another thing that tripped me up was the way that for example for PairPos adjustment, we handle Format1 and Format2 in the same function, while in harfbuzz they are different functions. So when a change happens to only one of them, things get a bit tricky... Or another problem was that for some reason we seem to have two kerning functions? While in harfbuzz I found only one corresponding one. But I think as long as we comment these deviations, it shouldn't be too bad.

[LaurenzV]

Can you point where you use new ttf-parser API and corresponding harfbuzz code?

Sure thing.

Change 1

pub fn size(self) -> usize {
        // The high 8 bits are not used, so make sure we ignore them using 0xFF.
        u16::SIZE * self.len()
}

  /// The number of ones in the underlying `ValueFormatFlag`.
  pub fn len(self) -> usize {
      usize::num_from(self.0.count_ones())
  }

So here I introduced the len method. Before, it was just:

fn size(self) -> usize {
        // The high 8 bits are not used, so make sure we ignore them using 0xFF.
        u16::SIZE * usize::num_from(self.0.count_ones())
 }

Here is the apply method for PosFormat2:

https://github.com/harfbuzz/harfbuzz/blob/ac46c3248e8b0316235943175c4d4a11c24dd4a9/src/hb-ot-layout-gpos-table.hh#L1555-L1666

Note in particular this len2 variable, whose value is derived from the get_len method. I don't know how the previous code worked, but it didn't seem to directly correspond to the harfbuzz code...

That's for PairPos1:

https://github.com/harfbuzz/harfbuzz/blob/ac46c3248e8b0316235943175c4d4a11c24dd4a9/src/hb-ot-layout-gpos-table.hh#L1357-L1374

Although I just noticed they don't have len2here... Not sure, maybe I need to look into it again.

Change 2

The valueFormatFlags(0) method we need to add because... well, we can only get len2 if we can access valueFormat2.

Change 3

The counts method I need because of the following piece of code (from PairPos2):

    if (unlikely (klass1 >= class1Count || klass2 >= class2Count))
    {
      buffer->unsafe_to_concat (buffer->idx, skippy_iter.idx + 1);
      return_trace (false);
    }

ttf-parser doesn't know anything about a buffer, so it can't set unsafe_to_concat, which means I need to check the condition on the rustybuzz side, which means I need access to the counts...

[RazrFalcon]

Yes, definitely. Idiomatic code is nice, but only if we can stick close to the harfbuzz code, otherwise it will haunt us in the future...

I will look into this on weekends.

what actually helped most was searching for comments that were the same.

yeah...

Or another problem was that for some reason we seem to have two kerning functions?

I don't remember, but maybe. harfbuzz relies heavily on C++ templates and it's ofter hard or impossible to port them, because C++ templates are duck typed, unlike Rust one. Which often leads to code duplication.
All I remember is that kern/kerx implementation was an absolute pain and took forever.

I will see how can we simplify porting.
The only thing that I do not plan changing is merging ttf-parser into rustybuzz, which is what harfbuzz does. I would prefer to keep them separated.

[RazrFalcon]

@LaurenzV As for ValueRecord, how about: positions.patch

No need to change ttf-parser at all. Sure, mine is_empty method is not really lossless in case when positions are actually 0, but we can ignore it for now.

[LaurenzV]

Looks sensible, thanks!

[LaurenzV]

The only thing that I do not plan changing is merging ttf-parser into rustybuzz, which is what harfbuzz does. I would prefer to keep them separated.

It does make backporting harder, but understandable that you want to keep them separate, I think it's also useful for the Rust ecosystem to have them separate.

[RazrFalcon]

Ready to merge?

As for ttf-parser, while it does make porting a bit harder, I think it's better to have parsing and shaping completely separated.
harfbuzz has a way too low-level access to TrueType internals. I don't like it. It makes code harder to follow.

[LaurenzV]

From my side yes!

[RazrFalcon]

Done.

I'm not sure if you still have energy working on it, but I plan to start renaming stuff on weekends, which would make merging PRs impossible. Therefore I would suggest to wait a bit.

[LaurenzV]

I'm more or less done with 4.0 (started working on it on a separate branch, just need to rebase), apart from some unclarities. Will open a PR soon. Then I can wait for a while if you want.

PS: Starting from 4.1, they seem to have a couple of commits that reorganize the structure:

So maybe you can base it of this then, to save us double the work. But let's see how 4.0 goes.

[RazrFalcon]

Got it. Thanks!

[khaledhosny]

2. if I understood correctly unsafe_to_concat etc. API seems to mainly be for the verify API, which we don't have yet...

Verify is a way of testing that these APIs work as intended. unsafe_to_concat is for optimizing re-shaping after line breaking. harfbuzz/harfbuzz#1463

[LaurenzV]

Thanks for clarifying!