Repository navigation
[gpos] Share cursive attachment across glyph-ID widths - #498
Merged
Merged
Conversation
Keep the format-specific coverage and anchor-offset readers in their wrappers, and share anchor resolution and attachment adjustment through a non-generic, non-inlined helper. Reuse child and parent position references instead of repeating bounds-checked indexing. Exercise both CursivePos formats in the all-direction attachment test, including the right-to-left lookup flag. No allocations or trait-object dispatch are added. On x86-64 Rust 1.89 release/fat-LTO builds, ELF text plus data shrinks by 920 bytes for hr-shape and 928 bytes for the C API. Stripped file sizes are unchanged because of segment alignment. Alternating pinned shaping benchmarks on Arabic, Urdu and Latin samples remain within about 1%. Tested with the complete all-feature workspace suite (including 6073 shaping regressions), strict Clippy, formatting, and a no-std/libm check. Shaping output and flags match the baseline in all four directions. Assisted-by: OpenAI Codex
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #495; this diff is only the cursive sharing change.
Keep CursivePos1/2 coverage and offset parsing format-specific, but share anchor resolution and attachment adjustment in one non-generic helper. No heap allocation or trait-object dispatch is introduced. Reuse child/parent position references to avoid repeated indexing, and test both formats against the same four-direction and right-to-left lookup-flag expectations.
Measured against a851c09 with identical dependencies and Rust 1.89 x86-64 release/fat-LTO settings:
hr-shape: ELF text + data decreases by 920 bytes.libharfrust_c.so: ELF text + data decreases by 928 bytes.Eight alternating CPU-pinned samples per case, reusing the font/plan/buffer across inner iterations: Amiri −0.55%, Nastaliq −0.24%, Naskh +0.70%, Latin −0.35% median elapsed time. These small differences do not establish a speed improvement or regression. Before/after shaping output, positions and flags match in all four directions.
Validation: full all-feature workspace tests (6073 shaping regressions), strict all-target/all-feature Clippy, fmt, and no-std/libm check.