From b94a39d7f3d3864909b72bb8788bb373a29e96f6 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sun, 22 Sep 2024 08:23:34 -0600 Subject: [PATCH] Follow up to variation-selector-not-found glyph Addresses https://github.com/harfbuzz/harfbuzz/pull/4529#discussion_r1769638033 I'm not sure if this is an improvement. By leaving the var-selector as default-ignorable, ligatures can form around it, and the resulting cluster won't make it clear *which* base+var-selector could not be resolved... That doesn't quite help font fallback the way we want. Putting up for review. --- src/hb-buffer.hh | 1 + src/hb-ot-layout.hh | 2 +- src/hb-ot-shape-normalize.cc | 18 +++++++-------- src/hb-ot-shape.cc | 22 +++++++++++++++++++ src/hb-unicode.hh | 3 +++ .../in-house/tests/variation-selectors.tests | 2 +- 6 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/hb-buffer.hh b/src/hb-buffer.hh index abf87e7a6..2a6ad6128 100644 --- a/src/hb-buffer.hh +++ b/src/hb-buffer.hh @@ -52,6 +52,7 @@ enum hb_buffer_scratch_flags_t { HB_BUFFER_SCRATCH_FLAG_HAS_CGJ = 0x00000010u, HB_BUFFER_SCRATCH_FLAG_HAS_GLYPH_FLAGS = 0x00000020u, HB_BUFFER_SCRATCH_FLAG_HAS_BROKEN_SYLLABLE = 0x00000040u, + HB_BUFFER_SCRATCH_FLAG_HAS_VARIATION_SELECTOR_FALLBACK= 0x00000080u, /* Reserved for shapers' internal use. */ HB_BUFFER_SCRATCH_FLAG_SHAPER0 = 0x01000000u, diff --git a/src/hb-ot-layout.hh b/src/hb-ot-layout.hh index 2e78277bb..591f7a281 100644 --- a/src/hb-ot-layout.hh +++ b/src/hb-ot-layout.hh @@ -173,7 +173,7 @@ _hb_next_syllable (hb_buffer_t *buffer, unsigned int start) /* Design: * unicode_props() is a two-byte number. The low byte includes: - * - General_Category: 5 bits. + * - Extended General_Category: 5 bits. * - A bit each for: * * Is it Default_Ignorable(); we have a modified Default_Ignorable(). * * Whether it's one of the four Mongolian Free Variation Selectors, diff --git a/src/hb-ot-shape-normalize.cc b/src/hb-ot-shape-normalize.cc index 3bd58dac6..2354aeb21 100644 --- a/src/hb-ot-shape-normalize.cc +++ b/src/hb-ot-shape-normalize.cc @@ -220,16 +220,14 @@ handle_variation_selector_cluster (const hb_ot_shape_normalize_context_t *c, /* Just pass on the two characters separately, let GSUB do its magic. */ set_glyph (buffer->cur(), font); (void) buffer->next_glyph (); - if (buffer->not_found_variation_selector != HB_CODEPOINT_INVALID) - { - _hb_glyph_info_clear_default_ignorable (&buffer->cur()); - next_char (buffer, buffer->not_found_variation_selector); - } - else - { - set_glyph (buffer->cur(), font); - (void) buffer->next_glyph (); - } + + buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_VARIATION_SELECTOR_FALLBACK; + + _hb_glyph_info_set_general_category (&buffer->cur(), + _HB_UNICODE_GENERAL_CATEGORY_VARIATION_SELECTOR); + + set_glyph (buffer->cur(), font); + (void) buffer->next_glyph (); } /* Skip any further variation selectors. */ while (buffer->idx < end && diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index bbf07c1f3..74d958d09 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -837,6 +837,27 @@ hb_ot_zero_width_default_ignorables (const hb_buffer_t *buffer) pos[i].x_advance = pos[i].y_advance = pos[i].x_offset = pos[i].y_offset = 0; } +static void +hb_ot_deal_with_variation_selectors (hb_buffer_t *buffer) +{ + if (!(buffer->scratch_flags & HB_BUFFER_SCRATCH_FLAG_HAS_VARIATION_SELECTOR_FALLBACK) || + buffer->not_found_variation_selector == HB_CODEPOINT_INVALID) + return; + + unsigned int count = buffer->len; + hb_glyph_info_t *info = buffer->info; + + for (unsigned int i = 0; i < count; i++) + { + if (_hb_glyph_info_get_general_category (&info[i]) == + _HB_UNICODE_GENERAL_CATEGORY_VARIATION_SELECTOR) + { + _hb_glyph_info_clear_default_ignorable (&info[i]); + info[i].codepoint = buffer->not_found_variation_selector; + } + } +} + static void hb_ot_hide_default_ignorables (hb_buffer_t *buffer, hb_font_t *font) @@ -966,6 +987,7 @@ hb_ot_substitute_post (const hb_ot_shape_context_t *c) hb_aat_layout_remove_deleted_glyphs (c->buffer); #endif + hb_ot_deal_with_variation_selectors (c->buffer); hb_ot_hide_default_ignorables (c->buffer, c->font); if (c->plan->shaper->postprocess_glyphs && diff --git a/src/hb-unicode.hh b/src/hb-unicode.hh index 39aaee5ba..2b1921c28 100644 --- a/src/hb-unicode.hh +++ b/src/hb-unicode.hh @@ -34,6 +34,9 @@ #include "hb.hh" +// Hack. See: https://github.com/harfbuzz/harfbuzz/pull/4529#discussion_r1769638033 +#define _HB_UNICODE_GENERAL_CATEGORY_VARIATION_SELECTOR ((hb_unicode_general_category_t) 30) + extern HB_INTERNAL const uint8_t _hb_modified_combining_class[256]; /* diff --git a/test/shape/data/in-house/tests/variation-selectors.tests b/test/shape/data/in-house/tests/variation-selectors.tests index bcf656489..723e2cd5c 100644 --- a/test/shape/data/in-house/tests/variation-selectors.tests +++ b/test/shape/data/in-house/tests/variation-selectors.tests @@ -1,2 +1,2 @@ ../fonts/bbc24004e776f348a0f72287d24b0124867ee750.ttf;;U+0066,U+FE00,U+0069;[gid5=0+1134|gid1=0+0] -../fonts/bbc24004e776f348a0f72287d24b0124867ee750.ttf;--not-found-variation-selector-glyph=1000000;U+0066,U+FE00,U+0069;[gid2=0+711|gid1000000=0+0|gid3=2+497] +../fonts/bbc24004e776f348a0f72287d24b0124867ee750.ttf;--not-found-variation-selector-glyph=1000000;U+0066,U+FE00,U+0069;[gid5=0+1134|gid1000000=0+0]