Conversation
|
This is great! Truly agentic possibility... Idea: Maybe add a hr-hb.h that macro-defines all hb_ symbols to hr_ symbols, such that clients can just include that and compile business as usual. |
I think this is not the case: |
|
Removing that error case from the buffer branch which will follow here. |
|
Fixed some soundness issues, improved performance and made it pass miri. |
This comment was marked as outdated.
This comment was marked as outdated.
|
@dfrg PTAL |
5ba11ba to
4c90e8e
Compare
Adds the harfrust-capi crate, mirroring the shaping half of HarfBuzz's API with an `hr_` prefix in place of `hb_`. Every type, function, enumerator and constant is named and numbered to match its HarfBuzz counterpart, so C code can usually be ported by renaming `hb_` to `hr_`, and the prefix lets the two libraries coexist in one process. Covered are blobs, faces, fonts, font callbacks, buffers, shape plans and `hr_shape`, along with the tags, directions, scripts, languages, features and variations they need. Faces can be built over a blob or from a table callback. Objects are reference counted, carry user data and can be made immutable, and the `_create` functions return immortal empty objects rather than NULL, all as in HarfBuzz. `hr_glyph_info_t` and `hr_glyph_position_t` share HarfRust's layout exactly, asserted at compile time, so the buffer accessors hand back pointers into its storage rather than copying. Since HarfRust builds a shape plan per call and has no cache of its own, one is kept per face, so repeated calls over the same face, segment properties and features reuse a plan the way `hb_shape` does internally. Nothing needs invalidating: everything else a plan depends on is fixed for the life of a face, and the feature variation indices that are not are part of the key. `hr_shape_plan_create_cached` draws from that same cache, so a plan the caller holds and one `hr_shape` uses are the same object. `hr_shape_plan_execute` aborts when handed a plan that does not apply, as HarfBuzz's assertions do: the plan must have been built over the same face, for the same variation settings, and for the properties the buffer carries. HarfBuzz compiles its assertions out with NDEBUG; these are always on, since HarfRust asserts unconditionally on a mismatched plan anyway and a clear message beats one from deep inside the shaper. Flag types, `hr_script_t` and `hr_direction_t` are integer typedefs rather than enumerations. C combines flags with a bitwise or and fills a `hr_segment_properties_t` in itself, so neither can be held in a Rust enumeration without inviting undefined behaviour on values it does not list. Their constants keep HarfBuzz's values and still work as switch labels. The header is generated with cbindgen and committed at include/hr.h. It compiles clean as C99, C11 and C17 and as C++11, C++17 and C++20, under gcc, clang and MSVC.
`Buffer::shape` now says why it could not shape, so `hr_shape_full` reports that rather than only whether the call panicked: a buffer already holding glyphs, a font with nothing to shape with, or shaping that ran past its limits all come back as false. `hr_shape_plan_execute` is unaffected. It checks the plan against the font and buffer itself, and aborts before reaching this.
Two changes toward being a drop-in replacement. `hr_shape` and `hr_shape_full` now abort when the API is misused, as HarfBuzz's assertions do, rather than reporting it: a buffer that already holds glyphs, or a font with nothing to shape with. `hr_shape` returns nothing, so it could not otherwise report these at all. Running past the length, operation or nesting limits stays a reported failure, since pathological input can provoke it and a caller can recover; that is the one failure HarfBuzz also lets `hb_shape_full` return. `include/hr-hb.h` maps every HarfBuzz name onto its HarfRust counterpart, so existing shaping code builds against this library by including it in place of <hb.h>. It is generated from hr.h by scripts/gen-hb-compat-header.py, and two tests check that every name is covered and that none is mapped that hr.h no longer declares. `examples/hb-compat.c` is `shape.c` written entirely in HarfBuzz's names, and mentions HarfRust nowhere. Writing that example turned up macros hr.h was missing, which are now there in their own right rather than only for the shim: `HR_TAG` and `HR_UNTAG`, the `HR_DIRECTION_IS_*` predicates and `HR_DIRECTION_REVERSE`, and the version macros. `HR_FEATURE_GLOBAL_END` was declared as `c_uint::MAX`, which cbindgen cannot evaluate and had silently dropped; it is a literal now, with a test that the committed header carries the crate's version.
`ShapeError` no longer carries the case where shaping ran out of room, so `hr_shape_full` reads it from `Buffer::allocation_successful` instead. Its contract is unchanged: false when the shaper gave up on pathological input, and an abort when the API is misused.
An audit of the unsafe code, backed by Miri, turned up five ways a caller following the documented rules could still reach freed memory. A sub-font copied its parent's `font_data` pointer without owning it, so replacing the parent's callbacks ran the destroy callback while the sub-font was still using the data. Font data now lives in an `Arc`, and the last font holding it releases it. Shaping held a borrow of the font across the call. Callbacks are handed that same font, so one that set its variations dropped the very `FontInstance` being shaped through. Instances are now shared, and shaping takes a snapshot of everything it needs before starting, so nothing borrows the font while callbacks can run. The snapshot also owns a reference to the callbacks and to the font data, and `hr_shape_plan_execute` takes a share of the plan, so a callback replacing or destroying any of them cannot pull it out from under the call. Miri caught the fifth: `shaper_list_allows_ot` built a reference to the first element of the shaper list and walked past it, which reads the right memory but is out of bounds of that reference. It walks the caller's pointer now. The empty singletons cached an address rather than a pointer, losing the provenance needed to dereference it; they hold the pointer itself now. Reads outnumber writes at every lock here, so `user_data`, the interned languages and the per-face plan cache move to `RwLock`. The plan cache is the one that matters: a hit only reads, so shaping the same face from several threads no longer serialises. The default language was taking a lock on every call to read something computed once, and is a `OnceLock` now. Tests cover the font data lifetime, and shaping through a shared face and a shared font from several threads. The whole suite runs clean under Miri, on both Stacked and Tree Borrows, and the threading tests run clean under its data race detector across a spread of interleavings.
Profiling a two-glyph CJK shape with caller-supplied font funcs, which is what a browser integration does, put roughly 44% of the call in this crate rather than in the shaper. Four things were being paid for on every call: - The plan cache took a read lock and returned a counted reference. It now walks a singly linked list whose links are written once, the way HarfBuzz walks the atomic list on its own faces. No node is ever unlinked, so a reader needs no lock, and the nodes outlive any shaping call, so a hit hands back a borrow rather than touching a reference count. Four atomics become none. `hr_shape_plan_create_cached` still wants an owned plan, so the nodes hold counted ones and only that path clones. - The plan cache built an owned key, cloning the language and copying the features, before searching for one. It now compares against the caller's values and only builds a key when it has a plan to store under it. - `shape_with_plan` cloned the font instance out of its own snapshot, for a callback adapter that never reads it. The clone was there because the snapshot has a `Drop` implementation and so cannot be destructured; the half that needs dropping now lives in its own type, and the instance is moved. - Text arrived through the lossy UTF-8 decoder, which walks the bytes to find ill-formed sequences and then again to yield characters. Well-formed text now takes `str::from_utf8`, with the lossy path kept for the rest. Shaping two Chinese characters with caller-supplied funcs is 17% faster, Japanese 9%, Korean 8%, and a Latin word 2%. Against HarfBuzz built with clang, the short CJK cases go from 1.23x, 1.12x and 1.06x to 0.99x, 1.03x and 1.00x. Output is unchanged: the shaped glyphs and positions checksum identically across every case.
The cache was capped at 32 plans. That made sense when it was a vector that could drop its oldest entry, but the list that replaced it never unlinks a node, because handing out borrows depends on nodes outliving the call. So the cap no longer evicted anything: a face that saw a thirty-third key simply stopped caching, and every call after that rebuilt its plan. Going cold permanently is a worse failure than growing, and it is not the behaviour callers are used to. HarfBuzz keeps one list per face, prepends to it, and frees it only when the face is destroyed; there is no cap and no eviction anywhere in it. Growth is bounded in practice by the distinct combinations of segment properties, user features and feature variation indices a face actually sees. Dropping the cap makes the fallback that built an uncached plan unreachable, so the enum that carried it goes too, and a hit now returns the counted plan directly. The list is freed iteratively. Boxed nodes would otherwise drop recursively, one stack frame per plan, which is fine for 32 and not for an unbounded list; HarfBuzz frees its own list with the same explicit loop.
hr_shape_full returned false when the buffer had run past its length, operation or nesting limits. HarfBuzz does not do that: hb_shape_full reports failure only when it cannot find or instantiate a shaper -- an invalid shape plan, a shaper_list naming none that it has, or shaper data it could not build -- and _hb_ot_shape returns true unconditionally whatever became of the buffer. A caller there learns about exhaustion from hb_buffer_allocation_successful, which we already expose as hr_buffer_allocation_successful. False now means only that no shaper could be run, which also settles a disagreement inside this crate: hr_shape_plan_execute shapes through shape_with_plan, which already returned true whenever shaping ran. Regenerated hr.h; hr-hb.h declares no new names and is unchanged.
hr_buffer_add_utf8 decoded the entire pre- and post-context even though a buffer retains only five context codepoints. Repeatedly shaping lines from one large text therefore rescanned the remaining suffix for every line and made the workload quadratic. Limit decoding on either side to the 20 bytes that can contribute those five codepoints. This reduces the Little Prince C API benchmark from about 50 ms to 8 ms and the English word-list benchmark from 266 ms to 13 ms. Tested with cargo test --workspace. Assisted-by: OpenAI Codex <codex@openai.com>
Return a mutable pointer to the static array of const strings so cbindgen emits HarfBuzz's const-char double-pointer declaration. The ABI was already compatible, but the extra outer const qualifier broke source compatibility. Test the exact generated declaration to keep the compatibility header usable with code written against HarfBuzz. Assisted-by: OpenAI Codex <noreply@openai.com>
Use Buffer::push_str for valid UTF-8 instead of growing the buffer one codepoint at a time. Adjust the generated relative clusters to retain the C API's absolute-offset convention. Assisted-by: OpenAI Codex <noreply@openai.com>
Cargo suppresses final LTO when an rlib is emitted alongside staticlib and cdylib outputs. The result costs about 17 percent on long shaping workloads. Keep the C API crate limited to its two C artifacts and run its API and header checks through the unit-test harness, which still gets a Rust test artifact without affecting release output. Assisted-by: OpenAI Codex <noreply@openai.com>
Building the FontInstance shaper view for every C API shape call rebuilt the OpenType and AAT table views and glyph metrics. This dominated the overhead when shaping many short buffers. Build the prepared shaper whenever the font instance changes and reuse it for subsequent shape calls. Font callbacks already may not mutate or free their font while shaping, so borrow the callback state directly instead of cloning per-call snapshots. Roboto en-words improves from 7.83 ms to 6.92 ms and is now faster than the 7.95 ms HarfBuzz-HarfRust bridge path. Assisted-by: OpenAI Codex <noreply@openai.com>
Asking hr_font_set_funcs for no callbacks now means no callbacks, which left nothing that put the built-in ones back. HarfBuzz has hb_ot_font_set_funcs for exactly that and this is its counterpart: it installs the callbacks a font starts with, and releases whatever data the callbacks it replaces were given, as replacing them with others does. Verified against HarfBuzz 14.4.0 through the same C: shaping and the glyph getters both answer from the font's tables again afterwards, and the replaced font data is released once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Added this, thanks. |
|
Hi — Codex here (OpenAI’s coding agent). I reviewed the follow-up comment and the 13 commits through The narrow reproductions behind the original eight findings are addressed, and the append, callback dispatch, destructor reentrancy, unset-property, empty-shaper-list, and shape-plan changes look sound. However, I do not think the branch is merge-ready yet:
Validation: all workspace tests passed when run with registry dependencies (including 6,065 shaping tests and 75 C-API tests), along with formatting and — Codex |
|
Hi — I’m Codex, OpenAI’s coding agent. At Behdad’s request, I measured the binary-size and unsafe-code impact of this branch at Binary sizeMeasured on x86-64 Linux with
So the C target’s complete standalone footprint is about 1.31 MiB, but the actual ABI-wrapper portion is about 91 KiB, and ordinary Rust consumers pay essentially nothing because the new target is opt-in. Unsafe-code impactThe safe core remains under Across its 5,978 lines of production Rust, I counted:
The 136 unsafe exports mostly reflect raw-pointer C signatures rather than 136 distinct implementations of dangerous logic. The higher-value audit surface is the 246 operation blocks, the 13 Bottom line: the binary overhead is modest and fully opt-in; the unsafe footprint is substantial in raw count, as expected for this breadth of C API, but is cleanly confined to the FFI crate. |
|
Hi — I’m Codex, OpenAI’s coding agent. I reviewed this C API against the reference behavior in a current HarfBuzz checkout ( I found the following behavioral and ABI bugs, ordered roughly by impact:
The existing |
Serializing a range worked on a copy of it and then found the item boundaries by counting separators in the result, which went wrong four ways. A buffer that was never given positions serialized as nothing at all, because the serializer walks glyphs alongside their positions and there were none to walk; HarfBuzz turns the request for positions off in that case and reports the glyphs. Asking for a sub-range of such a buffer went further and aborted the process, running off the end of the empty string it had just produced. With advances suppressed, each item reports the pen position it sits at, which counts from the start of the buffer, and working from a copy of the range lost everything before it. Extents that cannot be worked out at all -- with no font, nothing can -- were reported as zero rather than left out. And a glyph name containing a separator threw the boundary count off, at which point partial writes gave up and wrote nothing. Items are now serialized one at a time and assembled, so the boundaries are where the items are, the pen carries across the range, and the flags say only what can actually be answered. hr_buffer_add_codepoints had come to validate its input, having been written in terms of hr_buffer_add_utf32, which now replaces what are not characters. HarfBuzz makes them counterparts: add_utf32 checks, and add_codepoints is for callers that have already checked, or that mean it. They share a body and differ in that. Guessing a buffer's properties left the language unset where HarfBuzz settles on the one the process runs under, which language-specific shaping then follows. harfrust leaves it alone, having no business reading the environment, so the C API fills it in from the same default hr_language_get_default reports. Found by Codex on #460. Verified against HarfBuzz 14.4.0 through the same C, including the sweep of every destination size. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The abort tests ran a case in a child process and read any unsuccessful
exit as proof it had aborted -- but the child ended in unreachable!(),
which panics, so a case that quietly returned looked exactly like one
that stopped the process. aborts("coords") had been passing since
coordinate mismatches stopped aborting, testing nothing. The child now
exits with a status of its own when the call returns, and the case that
no longer aborts asserts that it does not, which is what shows the check
can tell the difference.
The documentation had drifted the same way: hr_shape_plan_execute and
the README still said a plan built for other variation settings aborts,
and the README still said hr_shape_full returns false when a buffer runs
past its limits. Neither has been so for several commits.
Also covers the fixes in the previous commit: a buffer with no
positions, a range serialized with advances suppressed, extents nothing
can answer, add_codepoints passing its input through, and guessing
settling on a language.
Found by Codex on #460.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
All seven addressed and pushed (
Findings 1, 4, 5 and 6 turned out to be one root cause. Serializing a range worked on a copy of it and then found item boundaries by counting separators in the output. Serializing one item at a time and assembling them fixed all four at once: the boundaries are where the items are, the pen carries across the range, and the flags now say only what can actually be answered — positions turned off when the buffer has none, extents dropped when there is no font to ask. Worth noting #1 was worse than reported: the sub-range path didn't just return nothing, it aborted the process running off the end of the empty string it had produced. Finding 3 had a knock-on you should know about. Once guessing sets a language, a plan built from hand-written Finding 7 was the most useful one. One residual on #5. Extents are omitted when there is no font at all, but if a real font cannot produce extents for a particular glyph we still write Verification: 81 C-API tests (up from 76), 6065 shaping tests, hr-shape, |
hr_buffer_content_type_t, hr_buffer_cluster_level_t, hr_buffer_serialize_format_t and hr_memory_mode_t were Rust enumerations carried across the boundary, so a caller passing anything but the exact values named in them was undefined behaviour rather than merely wrong. HarfBuzz invites precisely that: serialize_format_from_string hands back whatever tag it was given, named or not, and that value is meant to come back in. They are integer typedefs with constants now, as the flags already were, and the conversions answer for values that name nothing: an unknown content type says as little as the invalid one, an unknown cluster level leaves the buffer as it starts, and an unknown format serializes nothing and has no name. Reading a format from a string is now what HarfBuzz does, the tag with the case bit cleared, so a name this library does not know comes back as itself instead of collapsing to invalid. Found by Codex on #460. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A sub-font copied its parent's callbacks and font data when it was made, so callbacks installed on the parent afterwards were invisible to it, the callbacks it did run were handed the child and the child's data rather than the parent's, and a partial set installed on the child turned off everything the parent could still have answered. A sub-font now carries nothing of its own and every dispatch walks the chain: the nearest font whose callbacks answer for a kind answers, gets handed its own font pointer and data, and distances come back rescaled between the two fonts' scales. A chain with callbacks nowhere falls to the font's own tables, and one with callbacks but not this kind reports nothing available -- which for an advance means the font's own scale, as HarfBuzz answers, and not zero. The public glyph getters go through the same walk, so a font answers the same whichever way it is asked, and their built-in path now reads the charmap shaping reads rather than the raw cmap subtable, which knows about the Macintosh Roman and Windows symbol encodings. That needs harfrust to hand out the built-in callbacks, which it now does through Shaper::builtin_font_funcs. Serializing asks the same way, so extents a caller's callback answers for are the extents that get written. Replacing a parent's callbacks now releases the data they were given even while a sub-font is alive, because the sub-font no longer holds it. Confirmed the same in HarfBuzz. Found by Codex on #460, all of it probed against HarfBuzz 14.4.0 through the same C. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nine more places where the C API and HarfBuzz disagreed, each confirmed by probing both through the same C. Context belonged to whatever had been added last rather than to the text it came with: adding a codepoint left the old post-context in place, adding text installed its pre-context even onto a buffer that already held some, and an item running to the end of the text left the previous post-context standing. Emptying a buffer by length kept its content type and its context. All of that changes how the next shape joins, so the rules are HarfBuzz's now: pre-context only onto an empty buffer, post rebuilt every time, and a buffer emptied by length holding nothing. hr_language_matches had the relationship the wrong way round -- the second argument is the more specific language -- and the test enshrined it. Canonicalizing a tag now ends at the first character that can be neither a subtag nor a separator, so a locale's codeset and modifier are not part of the language. A buffer made like another copied what the source said about its text, where HarfBuzz copies only how it is set up; clearing a buffer's contents reset its cluster level and variation-selector fallback, which HarfBuzz keeps. That last one is harfrust's own Buffer::clear, whose documentation already claimed to match hb_buffer_clear_contents. An unresolved variation selector is spelled with the codepoint that is not one, which is also the default, rather than glyph zero. The none tag names the whole font, so a face built from a table callback answers hr_face_reference_blob with it and rejects it in hr_face_reference_table, which is the opposite of what it did. A face asked for with no callback at all is the empty face, and the data handed over with it is released rather than held. A zero-length blob is a blob of nothing rather than a failure, and holds the caller's data until it is destroyed; a length that cannot be represented is what fails instead. Segment properties compare their reserved fields, and overlaying stops where the two describe the text differently. An empty buffer shapes successfully whatever else was asked for, and a parse that fails leaves the destination describing nothing rather than whatever the caller left in it. Found by Codex on #460. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
All fourteen addressed, pushed as three commits. Two of them (#5, and most of #6) were already fixed by the previous push, which this review predates — it cites the old #1 was the one I fixed first, and the visible symptom understates it. The four #2 needed the dispatch rebuilt. A sub-font now carries nothing of its own and every lookup walks the parent chain: the nearest font whose callbacks answer does answer, gets handed its own font pointer and data, and distances come back rescaled between the two fonts' scales. Two things fell out of getting this right:
#12 came along with it: the built-in path now reads the charmap shaping reads — which knows the Macintosh Roman and Windows symbol encodings — rather than the raw cmap subtable, so a glyph shaping can find is one the getter reports. That needed #3 changes real shaping, not just bookkeeping — the two-adds case produced different glyphs. Context now follows HarfBuzz's rules exactly: pre-context only onto an empty buffer, post-context rebuilt on every add, and a buffer emptied by length keeping neither its content type nor its context. #4's fix had to include the test, which enshrined the reversed relationship; it now asserts that the second argument is the more specific language. Canonicalization ends at the first character that can be neither subtag nor separator, so The rest — #7 (copy configuration, not properties; keep cluster level and the variation-selector fallback across Two notes on scope:
89 C-API tests (up from 76), 6065 shaping tests, |
|
Hi — I’m Codex, OpenAI’s coding agent. I reviewed the follow-up commits through
There is also a validation failure outside those behavioral findings:
— Codex |
Three local benchmarking and dumping examples went in with an over-broad `git add`. They were never meant to be here -- one says so in its own header -- and they are what makes clippy fail over the workspace: an undeclared feature and a lint the crate denies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A sub-font carrying no callbacks asks its parent, which left nowhere to record that a font reads its own tables and asks nobody: hr_ot_font_set_funcs said it by carrying nothing, which is what asking the parent looks like. So a sub-font told to read the tables went on delegating, and a sub-font carrying callbacks that did not cover what was asked reported nothing available even when its parent could have answered from the tables. The built-in callbacks are an object a font can carry now, as they are in HarfBuzz, and the walk has three answers to give rather than two: a font carrying that object reads its tables and nothing above it is asked, one carrying nothing asks its parent, and one at the top of the chain answers from its tables or reports nothing depending on whether it carries callbacks at all. Shaping still installs no adapter for a chain carrying only built-ins, so the plain path is unchanged. Overlaying segment properties onto themselves is allowed and did not have to be: the source is read before the destination is borrowed, rather than holding a shared and an exclusive reference to one struct at once. A blob of no length means different things to the two creators. hr_blob_create hands back the blob that holds nothing, so the caller's data is released before returning, since nothing will be left to release it; hr_blob_create_or_fail makes one that can hold it and does. The previous commit gave both the second behaviour. An empty buffer executes any plan, the empty one included, because there is nothing to shape and so nothing to check a plan against. The check had been sitting behind the plan's own. Found by Codex on #460, each probed against HarfBuzz 14.4.0 through the same C. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
All four fixed, plus the validation failure, pushed as
Finding 1 was the interesting one, and you put your finger on exactly why. "Reads its own tables" and "has nothing of its own to say" were both spelled as Finding 2 was real UB and worth catching. Fixed by reading the source through the raw pointer before borrowing the destination, so no shared and exclusive reference to one struct exist at once. Finding 3 was a regression I introduced fixing the previous round — I gave both creators the Finding 4: the check was sitting behind the plan's own guard, so the empty plan failed before the buffer was ever looked at. It is now the first thing the function does. On the validation failure: those three examples were local scratch tools that went in with an over-broad 97 C-API tests now, 6065 shaping tests, |
A sweep over the exports no review had probed yet, one C file compiled against both libraries. Five answers differed. An empty name names no tag. hr_tag_from_string padded nothing out to four spaces and handed back a tag of blanks, which then read back as a script and a language of its own rather than as the absence of one. It answers HR_TAG_NONE now, and hr_script_from_string routes through the tag so the two agree. A script that is not one does not say which way text runs, so hr_script_get_horizontal_direction answers LTR for it rather than HR_DIRECTION_INVALID. Text runs left to right unless its script says otherwise. Reversing a direction is the bit flip HarfBuzz spells it as. The match arm left anything that was not a direction alone; the flip carries it to the other half of the pair it is not in, which is what HB_DIRECTION_REVERSE does, and reversing twice still gets back where it started. A sub-blob fixes what it is a part of. The part cannot outlive its parent being what it was, so making one makes the parent immutable. Every font has a parent to stop at. A font made from a face had none and answered with nothing; it answers with the font that is empty now, so a caller walking up the chain terminates on a font rather than on NULL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hr-hb.h exists so that source written against HarfBuzz builds against this library unchanged. The generator mapped every hr_ name it found, including eight that HarfBuzz has no spelling for: hb_buffer_reset_clusters and hb_shape_plan_get_segment_properties, which HarfBuzz does not have at all, and six lower-case hb_direction_is_* and hb_direction_reverse, which HarfBuzz has only as upper-case macros -- mapped below already. Nothing written against HarfBuzz can be using those names, and anything written against one of them would fail to build against HarfBuzz, which is the one thing this header is for. They are excluded now, and the test that checks the header is current knows about the same set. Checked against HarfBuzz 14.4.0's own headers: 188 names mapped, none that HarfBuzz lacks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Proactive sweep — no review needed this roundRather than wait for the next set of findings, I mapped the C API's coverage: Six divergences, all now fixed and re-verified as MATCH. Finding 1 — an empty name named a tag of blanks. Finding 2 — a script that is not one claimed no direction. Finding 3 — reversing a direction was a match, not a bit flip. Finding 4 — a sub-blob did not fix its parent. Finding 5 — a font made from a face had no parent. Finding 6 — the compatibility header was inventing HarfBuzz spellings. Verification. Six new regression tests. Full sweep re-run: 31 probes plus Two differences remain intentional and documented: one shaper ( Pushed as |
Found by running HarfBuzz's own shaping corpus through both libraries: the 6054 cases the test suites are generated from, driven through the same C against HarfBuzz and against harfrust-capi, and compared with everything turned on -- glyph names, clusters, positions, advances, flags and extents. The corpus itself asks for none of the last three in most cases, so nothing had been comparing them. A glyph past the last one the face has had an advance. hmtx repeats its final entry for glyphs beyond the long-metric count, which is right up to the glyph count and wrong past it: a malformed cmap can point shaping at a glyph the face does not have, and HarfBuzz gives that no advance rather than the last one it has. A face carrying no horizontal metrics at all still gives every glyph the default, in range or not, which is also what HarfBuzz does. Both spellings of the lookup, one glyph at a time and the batch shaping uses, now go through one place that says this once. Glyph extents began at the bounding box rather than at the side bearing. There is an undocumented rasterizer behaviour HarfBuzz matches, and says so in a comment: the glyph is shifted left by (lsb - xMin), so the ink starts at the left side bearing. Faces where the two agree saw nothing; faces where they do not were off by anything from one unit to 258. While here, the box is no longer assumed to be stored the right way round. What remains between the two is extents the tables cannot answer for yet: CFF and CFF2 outlines, sbix and CBDT bitmaps, COLR, and any glyph in a variable font once coordinates are set. Everything else -- every glyph, cluster, position, advance and flag in all 6051 comparable cases -- now matches HarfBuzz f5efbbef3 exactly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by walking every glyph of all 502 test faces through both libraries and comparing what the built-in callbacks say -- advances, origins, extents and names -- rather than only what shaping happens to ask them for. A character the cmap sends to .notdef is a character the font does not have. The lookup handed back glyph 0 and said it had found one, so a caller asking whether a font covers a character was told yes for 3282 characters across three faces that it does not cover. It matters to shaping too: a buffer told to use its own not-found glyph never got to, because nothing was ever not found. HarfBuzz reads a .notdef mapping as no mapping. The ascender came from OS/2 whether or not OS/2 said to. The typographic metrics are the ones to use only when the face sets USE_TYPO_METRICS; otherwise hhea carries them, and a face with neither is guessed at four fifths of the way up the em. Reading OS/2 whenever it was present, which is nearly always, gave a different vertical advance or origin for 21718 and 42394 glyphs. The ascender is also taken as positive and the descender as negative whichever way the face stores them, as HarfBuzz takes them. A glyph past the last one the face has had a vertical advance, the same way it had a horizontal one before the previous commit, and for the same reason: vmtx repeats its final entry, which is right up to the glyph count and wrong past it. Two vertical cases move from passing to failing, and are excluded through the generator's own list alongside the two already there for the same reason. Both are variable fonts whose vertical origin HarfBuzz reads from variable glyf extents, which HarfRust does not have yet: they matched before only because the ascender was being read from the wrong table and these two faces happen to put their typographic ascender exactly where the origin belongs. The gap they point at is real and was already there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The C API could install a callback for a glyph's advance, its vertical origin and its extents, and then had no way to ask for any of them. Only the two glyph lookups had getters. A client measuring text without shaping it -- which is what hb_font_get_glyph_h_advance is for, and what Chrome reaches for -- had nothing to call, and nothing outside shaping could be compared against HarfBuzz at all. Eighteen entry points, each one HarfBuzz's: hr_font_get_glyph the two lookups in one hr_font_get_glyph_h_advance and _v_advance hr_font_get_glyph_h_advances and _v_advances, over strides hr_font_get_glyph_h_origin and _v_origin hr_font_get_glyph_extents hr_font_get_glyph_advance_for_direction and the plural hr_font_get_glyph_origin_for_direction and add_ and subtract_ hr_font_get_glyph_extents_for_origin hr_font_get_glyph_name and _from_name hr_font_glyph_to_string and _from_string They answer through the same walk up the callback chain shaping uses, so a font cannot say one thing to a caller and another to the shaper: the advance and origin dispatch that lived inside the shaping trait moved into call_ methods both go through, as the glyph lookups and extents already did. Which turned up a scaling bug. Extents read from the font's own tables came back in design units while everything else came back in the units hr_font_set_scale asks for. Nothing had caught it because hb-shape sizes a font at its own upem, where the two are the same. Scaling is HarfRust's own now -- exported as Scale, so the getters and shaping round identically rather than each spelling HarfBuzz's arithmetic separately. Glyph names come from the face through GlyphNames, now public, and a name outlives the lookup that found it so a caller can hold one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Differential shaping and font funcs against HarfBuzzTwo new harnesses, each one C file compiled twice — once against HarfBuzz, Correction to my earlier rounds: my HarfBuzz checkout was ten days Harness one: the shaping corpusThe 6,054 cases the generated shaping suites are built from — font, text, Run as the suites run it, HarfBuzz and harfrust agree everywhere. But most Finding 1 — a glyph past the last one the face has had an advance. Finding 2 — extents began at the bounding box, not the side bearing. Harness two: font funcs, every glyph of every faceFilling in the getter surface (below) unblocked walking all 502 test faces Finding 3 — a character the cmap sends to .notdef was reported as Finding 4 — the ascender came from OS/2 whether or not OS/2 said to. Finding 5 — the vertical advance had finding 1's bug too, A trade worth flagging. Finding 4 moves two corpus cases from passing Filling in the getter surfaceThe C API let you install a callback for a glyph's advance, vertical origin They answer through the same walk up the callback chain shaping uses — the Which exposed a sixth bug. Extents read from the font's own tables came Where it stands
Pushed as |
One less kebab-case crate name to carry. The directory keeps its own name, so every path that spells it with a dash stays as it is, and the C library this builds was already harfrust_c through an explicit [lib] name, so nothing linking against it moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The directory kept the dash after the package lost it. Everything that spelled the path follows it: the workspace member list, the compatibility header generator, both example build lines, and the regenerate command the header carries in its own banner. The C library is still harfrust_c, through the explicit [lib] name, so nothing linking against it moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Adds the harfrust-capi crate, mirroring the shaping half of HarfBuzz's API
with an
hr_prefix in place ofhb_. Every type, function, enumerator andconstant is named and numbered to match its HarfBuzz counterpart, so C code
can usually be ported by renaming
hb_tohr_, and the prefix lets the twolibraries coexist in one process.
Covered are blobs, faces, fonts, font callbacks, buffers, shape plans and
hr_shape, along with the tags, directions, scripts, languages, features andvariations they need. Faces can be built over a blob or from a table callback.
Objects are reference counted, carry user data and can be made immutable, and
the
_createfunctions return immortal empty objects rather than NULL, all asin HarfBuzz.
hr_glyph_info_tandhr_glyph_position_tshare HarfRust's layout exactly,asserted at compile time, so the buffer accessors hand back pointers into its
storage rather than copying.
Since HarfRust builds a shape plan per call and has no cache of its own, one
is kept per face, so repeated calls over the same face, segment properties
and features reuse a plan the way
hb_shapedoes internally. Nothing needsinvalidating: everything else a plan depends on is fixed for the life of a
face, and the feature variation indices that are not are part of the key.
hr_shape_plan_create_cacheddraws from that same cache, so a plan thecaller holds and one
hr_shapeuses are the same object.hr_shape_plan_executeaborts when handed a plan that does not apply, asHarfBuzz's assertions do: the plan must have been built over the same face,
for the same variation settings, and for the properties the buffer carries.
HarfBuzz compiles its assertions out with NDEBUG; these are always on, since
HarfRust asserts unconditionally on a mismatched plan anyway and a clear
message beats one from deep inside the shaper.
Flag types,
hr_script_tandhr_direction_tare integer typedefs ratherthan enumerations. C combines flags with a bitwise or and fills a
hr_segment_properties_tin itself, so neither can be held in a Rustenumeration without inviting undefined behaviour on values it does not list.
Their constants keep HarfBuzz's values and still work as switch labels.
The header is generated with cbindgen and committed at include/hr.h. It
compiles clean as C99, C11 and C17 and as C++11, C++17 and C++20, under gcc,
clang and MSVC.