From 3d2106ec825e9c92c5814fcd881ae71a5d5a5569 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 8 Oct 2026 13:24:35 -0600 Subject: [PATCH 1/2] capi: collect nominal cmap Unicode coverage --- harfrust_capi/README.md | 1 + harfrust_capi/include/hr-hb.h | 1 + harfrust_capi/include/hr.h | 12 +++ harfrust_capi/src/face.rs | 69 ++++++++++++++++- harfrust_capi/tests/skia_face.rs | 72 ++++++++++++++++++ scripts/test-capi-coverage.py | 124 +++++++++++++++++++++++++++++++ 6 files changed, 278 insertions(+), 1 deletion(-) create mode 100644 scripts/test-capi-coverage.py diff --git a/harfrust_capi/README.md b/harfrust_capi/README.md index a4f71871..e6a5ccef 100644 --- a/harfrust_capi/README.md +++ b/harfrust_capi/README.md @@ -117,6 +117,7 @@ callback with `hr_face_create_for_tables`, which asks for one table at a time. Use `hr_face_set_get_table_tags_func` to enumerate the tables of a callback face; blob faces enumerate their SFNT directory automatically. `hr_face_set_index` changes the reported index without selecting new tables. +`hr_face_collect_unicodes` adds the selected cmap subtable's coverage to a set. `hr_shape` already reuses shape plans through a per-face cache, the way `hb_shape` does internally, so reach for `hr_shape_plan_create` only when you diff --git a/harfrust_capi/include/hr-hb.h b/harfrust_capi/include/hr-hb.h index cfa4c5cb..e2dfdccc 100644 --- a/harfrust_capi/include/hr-hb.h +++ b/harfrust_capi/include/hr-hb.h @@ -114,6 +114,7 @@ #define hb_direction_from_string hr_direction_from_string #define hb_direction_t hr_direction_t #define hb_direction_to_string hr_direction_to_string +#define hb_face_collect_unicodes hr_face_collect_unicodes #define hb_face_count hr_face_count #define hb_face_create hr_face_create #define hb_face_create_for_tables hr_face_create_for_tables diff --git a/harfrust_capi/include/hr.h b/harfrust_capi/include/hr.h index a692f627..16d8e939 100644 --- a/harfrust_capi/include/hr.h +++ b/harfrust_capi/include/hr.h @@ -2886,6 +2886,18 @@ struct hr_blob_t *hr_face_reference_blob(struct hr_face_t *face); */ struct hr_blob_t *hr_face_reference_table(struct hr_face_t *face, hr_tag_t tag); +/** + * Adds the face's nominal cmap coverage to `out`, preserving existing entries. + * + * Uses the selected cmap subtable's stored character codes, without the symbol + * fallback or Mac Roman conversion applied by nominal glyph lookup. Variation + * sequences are not included. A missing cmap adds nothing. + * # Safety + * `face` must be null or live; `out` must be null or live and must not be + * accessed concurrently while it is being modified. + */ +void hr_face_collect_unicodes(struct hr_face_t *face, struct hr_set_t *out); + /** * Creates a font over a face. * diff --git a/harfrust_capi/src/face.rs b/harfrust_capi/src/face.rs index 76244faf..0f9a4f16 100644 --- a/harfrust_capi/src/face.rs +++ b/harfrust_capi/src/face.rs @@ -6,12 +6,17 @@ use std::sync::{Arc, OnceLock}; use harfrust::font::{Blob, Font, TableFunction}; use harfrust::Tag; -use read_fonts::{model::Format, TableProvider}; +use read_fonts::{ + model::Format, + tables::cmap::{CmapIterLimits, CmapSubtable}, + TableProvider, +}; use crate::blob::hr_blob_t; use crate::common::{hr_bool_t, hr_tag_t, tag_from_rust, tag_to_rust}; use crate::object::{self, hr_destroy_func_t, hr_user_data_key_t, Empty, Object, ObjectHeader}; use crate::plan::PlanCache; +use crate::set::hr_set_t; /// Callback returning the data for one table of a face. /// @@ -565,3 +570,65 @@ pub unsafe extern "C" fn hr_face_reference_table( } } } + +/// Adds the face's nominal cmap coverage to `out`, preserving existing entries. +/// +/// Uses the selected cmap subtable's stored character codes, without the symbol +/// fallback or Mac Roman conversion applied by nominal glyph lookup. Variation +/// sequences are not included. A missing cmap adds nothing. +/// # Safety +/// `face` must be null or live; `out` must be null or live and must not be +/// accessed concurrently while it is being modified. +#[no_mangle] +pub unsafe extern "C" fn hr_face_collect_unicodes(face: *mut hr_face_t, out: *mut hr_set_t) { + let face = unsafe { object::or_empty(face.cast_const()) }; + let Some(font) = face.font() else { return }; + let Some((_, _, subtable)) = font + .tables() + .cmap() + .ok() + .and_then(|table| table.best_subtable()) + else { + return; + }; + let Some(out) = (unsafe { object::as_mutable(out) }) else { + return; + }; + match subtable { + // The generic Fontations iterator does not implement format 0. + CmapSubtable::Format0(table) => { + for (codepoint, &glyph) in table.glyph_id_array().iter().enumerate() { + if glyph != 0 { + out.add(codepoint as u32); + } + } + } + // A format 13 group maps an entire range to one glyph. Insert ranges + // directly, both to bound malformed glyph IDs and to avoid iterating + // over a million characters in a last-resort font. + CmapSubtable::Format13(table) => { + for group in table.groups() { + let first = group.start_char_code(); + let last = group.end_char_code().min(0x0010_FFFF); + let glyph = group.glyph_id(); + if first <= last && glyph != 0 && glyph < font.num_glyphs() { + out.values.insert_range(first..=last); + } + } + } + _ => { + let limits = CmapIterLimits { + // The iterator's character limit is exclusive. + max_char: 0x0011_0000, + glyph_count: font.num_glyphs(), + }; + let is_format4 = matches!(subtable, CmapSubtable::Format4(_)); + for (codepoint, glyph) in subtable.iter_with_limits(limits) { + // Format 4's final U+FFFF segment is a sentinel. + if glyph.to_u32() != 0 && !(is_format4 && codepoint == 0xFFFF) { + out.add(codepoint); + } + } + } + } +} diff --git a/harfrust_capi/tests/skia_face.rs b/harfrust_capi/tests/skia_face.rs index 5b0a762c..68399400 100644 --- a/harfrust_capi/tests/skia_face.rs +++ b/harfrust_capi/tests/skia_face.rs @@ -155,3 +155,75 @@ fn default_unicode_scripts_match_itemization_needs() { assert_eq!(hr_unicode_script(ptr::null_mut(), codepoint), script); } } + +#[test] +fn unicode_collection_adds_coverage_without_clearing_the_set() { + unsafe { + let face = face(); + let set = hr_set_create(); + hr_set_add(set, 0x0010_FFFF); + hr_face_collect_unicodes(face, set); + for codepoint in ['A', 'a', 'b'] { + assert_eq!(hr_set_has(set, codepoint as u32), 1); + } + assert_eq!(hr_set_has(set, 0x0010_FFFF), 1); + let population = hr_set_get_population(set); + assert!(population > 3); + hr_face_collect_unicodes(face, set); + hr_face_collect_unicodes(ptr::null_mut(), set); + hr_face_collect_unicodes(face, ptr::null_mut()); + hr_face_collect_unicodes(face, hr_set_get_empty()); + assert_eq!(hr_set_get_population(set), population); + hr_set_invert(set); + hr_set_del(set, 'a' as u32); + hr_face_collect_unicodes(face, set); + assert_eq!(hr_set_has(set, 'a' as u32), 1); + hr_set_destroy(set); + hr_face_destroy(face); + } +} + +#[test] +fn unicode_collection_covers_format0_and_constant_format13_ranges() { + unsafe { + for (data, population, first, last) in [ + ( + &include_bytes!("../../harfrust/tests/fonts/aots/cmap0_font1.otf")[..], + 3, + 52, + 54, + ), + ( + &include_bytes!( + "../../harfrust/tests/fonts/text-rendering-tests/TestCMAPMacTurkish.ttf" + )[..], + 221, + 32, + 255, + ), + ( + &include_bytes!("../../harfrust/tests/fonts/in-house/AdobeBlank2.ttf")[..], + 1_111_998, + 0, + 0x0010_FFFD, + ), + ] { + let blob = hr_blob_create( + data.as_ptr().cast(), + data.len() as u32, + HR_MEMORY_MODE_READONLY, + ptr::null_mut(), + None, + ); + let face = hr_face_create(blob, 0); + hr_blob_destroy(blob); + let set = hr_set_create(); + hr_face_collect_unicodes(face, set); + assert_eq!(hr_set_get_population(set), population); + assert_eq!(hr_set_has(set, first), 1); + assert_eq!(hr_set_has(set, last), 1); + hr_set_destroy(set); + hr_face_destroy(face); + } + } +} diff --git a/scripts/test-capi-coverage.py b/scripts/test-capi-coverage.py new file mode 100644 index 00000000..137d6898 --- /dev/null +++ b/scripts/test-capi-coverage.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +"""Compare nominal cmap coverage with HarfBuzz. + +Build harfrust_capi, then run: + python3 scripts/test-capi-coverage.py --library /path/to/libharfrust_c.so \ + --harfbuzz /path/to/libharfbuzz.so --fonts harfrust/tests/fonts + +HarfBuzz 14.5.1 incorrectly truncates format-13 constant-glyph ranges as though +glyph IDs incremented within each range. For those subtables, independently +check HarfRust's complete ranges and report the HarfBuzz difference separately. +""" +import argparse +import ctypes as C +from pathlib import Path +import struct + +U, P = C.c_uint, C.c_void_p +SEED = 0x10FFFF + + +class Api: + def __init__(self, library, prefix): + self.library = C.CDLL(library) + + def bind(name, result, arguments): + function = getattr(self.library, prefix + name) + function.restype = result + function.argtypes = arguments + return function + + self.blob = bind("blob_create", P, [P, U, C.c_int, P, P]) + self.drop_blob = bind("blob_destroy", None, [P]) + self.blob_data = bind("blob_get_data", P, [P, C.POINTER(U)]) + self.face = bind("face_create", P, [P, U]) + self.face_count = bind("face_count", U, [P]) + self.drop_face = bind("face_destroy", None, [P]) + self.table = bind("face_reference_table", P, [P, U]) + self.glyph_count = bind("face_get_glyph_count", U, [P]) + self.make_set = bind("set_create", P, []) + self.drop_set = bind("set_destroy", None, [P]) + self.add = bind("set_add", None, [P, U]) + self.collect = bind("face_collect_unicodes", None, [P, P]) + self.next = bind("set_next", C.c_int, [P, C.POINTER(U)]) + + def coverage(self, face): + output = self.make_set() + self.add(output, SEED) + self.collect(face, output) + value, values = U(0xFFFFFFFF), set() + while self.next(output, C.byref(value)): + values.add(value.value) + self.drop_set(output) + return values + + def format13_coverage(self, face): + blob = self.table(face, int.from_bytes(b"cmap", "big")) + size = U() + pointer = self.blob_data(blob, C.byref(size)) + data = C.string_at(pointer, size.value) if size.value else b"" + self.drop_blob(blob) + if len(data) < 4: + return None + count = struct.unpack_from(">H", data, 2)[0] + records = [struct.unpack_from(">HHI", data, 4 + i * 8) for i in range(count)] + priority = [(3, 0), (3, 10), (0, 6), (0, 4), (3, 1), + (0, 3), (0, 2), (0, 1), (0, 0), (1, 0)] + for platform, encoding in priority: + for record_platform, record_encoding, offset in records: + if (platform, encoding) != (record_platform, record_encoding): + continue + format_id = struct.unpack_from(">H", data, offset)[0] + if format_id not in (0, 4, 6, 10, 12, 13): + continue + if format_id != 13: + return None + groups = struct.unpack_from(">I", data, offset + 12)[0] + glyph_count = self.glyph_count(face) + result = {SEED} + for i in range(groups): + first, last, glyph = struct.unpack_from(">III", data, offset + 16 + i * 12) + if 0 < glyph < glyph_count: + result.update(range(first, min(last, 0x10FFFF) + 1)) + return result + return None + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--library", required=True) + parser.add_argument("--harfbuzz", required=True) + parser.add_argument("--fonts", type=Path, required=True) + args = parser.parse_args() + hr, hb = Api(args.library, "hr_"), Api(args.harfbuzz, "hb_") + matched, corrected = 0, 0 + for path in sorted(args.fonts.rglob("*")): + if path.suffix.lower() not in {".ttf", ".otf", ".ttc"}: + continue + data = path.read_bytes() + hr_blob = hr.blob(data, len(data), 0, None, None) + hb_blob = hb.blob(data, len(data), 0, None, None) + for index in range(hb.face_count(hb_blob)): + hr_face, hb_face = hr.face(hr_blob, index), hb.face(hb_blob, index) + try: + actual, expected = hr.coverage(hr_face), hb.coverage(hb_face) + if actual == expected: + matched += 1 + else: + complete = hr.format13_coverage(hr_face) + assert complete is not None and actual == complete and expected <= actual, ( + path, index, len(actual), len(expected), sorted(actual ^ expected)[:10]) + print(f"Complete format-13 coverage: {path} face {index}; " + f"HarfRust {len(actual)} entries, HarfBuzz {len(expected)}") + corrected += 1 + finally: + hr.drop_face(hr_face) + hb.drop_face(hb_face) + hr.drop_blob(hr_blob) + hb.drop_blob(hb_blob) + print(f"Matched {matched} coverage sets; independently checked {corrected} " + "complete format-13 coverage sets (counts include a seeded existing entry).") + + +if __name__ == "__main__": + main() From b7331d4fe30634cedf1986a28d4c0d02d42a95fc Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 9 Oct 2026 14:18:45 -0600 Subject: [PATCH 2/2] capi: leave cmap format 0 iteration to Fontations Remove the format 0 fallback and its C API coverage cases. The generic iterator will cover that format once read-fonts includes Fontations #2265. --- harfrust_capi/src/face.rs | 54 ++++++++++++----------------- harfrust_capi/tests/skia_face.rs | 58 ++++++++++---------------------- 2 files changed, 39 insertions(+), 73 deletions(-) diff --git a/harfrust_capi/src/face.rs b/harfrust_capi/src/face.rs index 456de50e..027f5acd 100644 --- a/harfrust_capi/src/face.rs +++ b/harfrust_capi/src/face.rs @@ -594,41 +594,29 @@ pub unsafe extern "C" fn hr_face_collect_unicodes(face: *mut hr_face_t, out: *mu let Some(out) = (unsafe { object::as_mutable(out) }) else { return; }; - match subtable { - // Temporary workaround for read-fonts 0.45. Remove once we can use - // https://github.com/googlefonts/fontations/pull/2265. - CmapSubtable::Format0(table) => { - for (codepoint, &glyph) in table.glyph_id_array().iter().enumerate() { - if glyph != 0 { - out.add(codepoint as u32); - } - } - } - // A format 13 group maps an entire range to one glyph. Insert ranges - // directly, both to bound malformed glyph IDs and to avoid iterating - // over a million characters in a last-resort font. - CmapSubtable::Format13(table) => { - for group in table.groups() { - let first = group.start_char_code(); - let last = group.end_char_code().min(0x0010_FFFF); - let glyph = group.glyph_id(); - if first <= last && glyph != 0 && glyph < font.num_glyphs() { - out.values.insert_range(first..=last); - } + // A format 13 group maps an entire range to one glyph. Insert ranges + // directly, both to bound malformed glyph IDs and to avoid iterating + // over a million characters in a last-resort font. + if let CmapSubtable::Format13(table) = subtable { + for group in table.groups() { + let first = group.start_char_code(); + let last = group.end_char_code().min(0x0010_FFFF); + let glyph = group.glyph_id(); + if first <= last && glyph != 0 && glyph < font.num_glyphs() { + out.values.insert_range(first..=last); } } - _ => { - let limits = CmapIterLimits { - // The iterator's character limit is exclusive. - max_char: 0x0011_0000, - glyph_count: font.num_glyphs(), - }; - let is_format4 = matches!(subtable, CmapSubtable::Format4(_)); - for (codepoint, glyph) in subtable.iter_with_limits(limits) { - // Format 4's final U+FFFF segment is a sentinel. - if glyph.to_u32() != 0 && !(is_format4 && codepoint == 0xFFFF) { - out.add(codepoint); - } + } else { + let limits = CmapIterLimits { + // The iterator's character limit is exclusive. + max_char: 0x0011_0000, + glyph_count: font.num_glyphs(), + }; + let is_format4 = matches!(subtable, CmapSubtable::Format4(_)); + for (codepoint, glyph) in subtable.iter_with_limits(limits) { + // Format 4's final U+FFFF segment is a sentinel. + if glyph.to_u32() != 0 && !(is_format4 && codepoint == 0xFFFF) { + out.add(codepoint); } } } diff --git a/harfrust_capi/tests/skia_face.rs b/harfrust_capi/tests/skia_face.rs index 68399400..304145e6 100644 --- a/harfrust_capi/tests/skia_face.rs +++ b/harfrust_capi/tests/skia_face.rs @@ -184,46 +184,24 @@ fn unicode_collection_adds_coverage_without_clearing_the_set() { } #[test] -fn unicode_collection_covers_format0_and_constant_format13_ranges() { +fn unicode_collection_covers_constant_format13_ranges() { unsafe { - for (data, population, first, last) in [ - ( - &include_bytes!("../../harfrust/tests/fonts/aots/cmap0_font1.otf")[..], - 3, - 52, - 54, - ), - ( - &include_bytes!( - "../../harfrust/tests/fonts/text-rendering-tests/TestCMAPMacTurkish.ttf" - )[..], - 221, - 32, - 255, - ), - ( - &include_bytes!("../../harfrust/tests/fonts/in-house/AdobeBlank2.ttf")[..], - 1_111_998, - 0, - 0x0010_FFFD, - ), - ] { - let blob = hr_blob_create( - data.as_ptr().cast(), - data.len() as u32, - HR_MEMORY_MODE_READONLY, - ptr::null_mut(), - None, - ); - let face = hr_face_create(blob, 0); - hr_blob_destroy(blob); - let set = hr_set_create(); - hr_face_collect_unicodes(face, set); - assert_eq!(hr_set_get_population(set), population); - assert_eq!(hr_set_has(set, first), 1); - assert_eq!(hr_set_has(set, last), 1); - hr_set_destroy(set); - hr_face_destroy(face); - } + let data = include_bytes!("../../harfrust/tests/fonts/in-house/AdobeBlank2.ttf"); + let blob = hr_blob_create( + data.as_ptr().cast(), + data.len() as u32, + HR_MEMORY_MODE_READONLY, + ptr::null_mut(), + None, + ); + let face = hr_face_create(blob, 0); + hr_blob_destroy(blob); + let set = hr_set_create(); + hr_face_collect_unicodes(face, set); + assert_eq!(hr_set_get_population(set), 1_111_998); + assert_eq!(hr_set_has(set, 0), 1); + assert_eq!(hr_set_has(set, 0x0010_FFFD), 1); + hr_set_destroy(set); + hr_face_destroy(face); } }