From 926f512f354835f8323bb2c2e58789dd918a9b65 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sun, 25 Nov 2018 01:14:40 -0500 Subject: [PATCH] [aat.feat] Rework API and implementation Fixes https://github.com/harfbuzz/harfbuzz/pull/1346 --- docs/harfbuzz-docs.xml | 5 ++ docs/harfbuzz-sections.txt | 7 +-- src/hb-aat-layout-feat-table.hh | 89 ++++++++++++++++++--------------- src/hb-aat-layout.cc | 80 +++++++++++++---------------- src/hb-aat-layout.h | 32 +++++++----- test/api/test-aat-layout.c | 58 ++++++++++----------- 6 files changed, 141 insertions(+), 130 deletions(-) diff --git a/docs/harfbuzz-docs.xml b/docs/harfbuzz-docs.xml index 66a64d8f9..27353389d 100644 --- a/docs/harfbuzz-docs.xml +++ b/docs/harfbuzz-docs.xml @@ -112,6 +112,11 @@ + + Apple Advanced Typography API + + + Integration API diff --git a/docs/harfbuzz-sections.txt b/docs/harfbuzz-sections.txt index 0498a2790..5c45153c3 100644 --- a/docs/harfbuzz-sections.txt +++ b/docs/harfbuzz-sections.txt @@ -4,13 +4,14 @@ HB_OT_H_IN
-hb-aat +hb-aat-layout +HB_AAT_LAYOUT_NO_SELECTOR_INDEX hb_aat_layout_feature_type_t hb_aat_layout_get_feature_types hb_aat_layout_feature_type_get_name_id hb_aat_layout_feature_selector_t -hb_aat_layout_feature_type_get_selectors -hb_aat_layout_feature_selector_get_name_id +hb_aat_layout_feature_selector_info_t +hb_aat_layout_feature_type_get_selector_infos
diff --git a/src/hb-aat-layout-feat-table.hh b/src/hb-aat-layout-feat-table.hh index 98d628693..4e63ec8e8 100644 --- a/src/hb-aat-layout-feat-table.hh +++ b/src/hb-aat-layout-feat-table.hh @@ -39,13 +39,26 @@ namespace AAT { struct SettingName { + friend struct FeatureName; + int cmp (hb_aat_layout_feature_selector_t key) const { return (int) key - (int) setting; } - inline hb_aat_layout_feature_selector_t get_selector () const - { return (hb_aat_layout_feature_selector_t) (unsigned int) setting; } + inline hb_aat_layout_feature_selector_t get_selector (void) const + { return (hb_aat_layout_feature_selector_t) (unsigned) setting; } - inline hb_ot_name_id_t get_name_id () const { return nameIndex; } + inline void get_info (hb_aat_layout_feature_selector_info_t *s, + hb_aat_layout_feature_selector_t default_selector) const + { + s->name_id = nameIndex; + + s->enable = (hb_aat_layout_feature_selector_t) (unsigned int) setting; + s->disable = default_selector == HB_AAT_LAYOUT_FEATURE_SELECTOR_INVALID ? + (hb_aat_layout_feature_selector_t) (s->enable + 1) : + default_selector; + + s->reserved = 0; + } inline bool sanitize (hb_sanitize_context_t *c) const { @@ -82,29 +95,34 @@ struct FeatureName * as the default. */ }; - inline unsigned int get_selectors (const feat *feat, - hb_aat_layout_feature_selector_t *default_selector, - unsigned int start_offset, - unsigned int *count, - hb_aat_layout_feature_selector_t *selectors) const + inline unsigned int get_selector_infos (unsigned int start_offset, + unsigned int *selectors_count, /* IN/OUT. May be NULL. */ + hb_aat_layout_feature_selector_info_t *selectors, /* OUT. May be NULL. */ + unsigned int *pdefault_index, /* OUT. May be NULL. */ + const void *base) const { - const UnsizedArrayOf& settings_table = feat+settingTableZ; - unsigned int settings_count = nSettings; - if (count && *count) + hb_array_t< const SettingName> settings_table = (base+settingTableZ).as_array (nSettings); + + static_assert (Index::NOT_FOUND_INDEX == HB_AAT_LAYOUT_NO_SELECTOR_INDEX, ""); + + hb_aat_layout_feature_selector_t default_selector = HB_AAT_LAYOUT_FEATURE_SELECTOR_INVALID; + unsigned int default_index = Index::NOT_FOUND_INDEX; + if (featureFlags & Exclusive) { - unsigned int len = MIN (settings_count - start_offset, *count); - for (unsigned int i = 0; i < len; i++) - selectors[i] = settings_table[start_offset + i].get_selector (); - *count = len; + default_index = (featureFlags & NotDefault) ? featureFlags & IndexMask : 0; + default_selector = settings_table[default_index].get_selector (); } - if (default_selector) + if (pdefault_index) + *pdefault_index = default_index; + + if (selectors_count) { - unsigned int index = (featureFlags & NotDefault) ? featureFlags & IndexMask : 0; - *default_selector = ((featureFlags & Exclusive) && index < settings_count) - ? settings_table[index].get_selector () - : HB_AAT_LAYOUT_FEATURE_SELECTOR_INVALID; + hb_array_t arr = settings_table.sub_array (start_offset, selectors_count); + unsigned int count = arr.len; + for (unsigned int i = 0; i < count; i++) + settings_table[start_offset + i].get_info (&selectors[i], default_selector); } - return settings_count; + return settings_table.len; } inline hb_aat_layout_feature_type_t get_feature_type () const @@ -112,12 +130,6 @@ struct FeatureName inline hb_ot_name_id_t get_feature_name_id () const { return nameIndex; } - inline hb_ot_name_id_t get_feature_selector_name_id (const feat *feat, - hb_aat_layout_feature_selector_t key) const - { - return (feat+settingTableZ).lsearch (nSettings, key).get_name_id (); - } - inline bool sanitize (hb_sanitize_context_t *c, const void *base) const { TRACE_SANITIZE (this); @@ -162,32 +174,29 @@ struct feat return featureNameCount; } - inline const FeatureName& get_feature (hb_aat_layout_feature_type_t key) const + inline const FeatureName& get_feature (hb_aat_layout_feature_type_t feature_type) const { - return namesZ.bsearch (featureNameCount, key); + return namesZ.bsearch (featureNameCount, feature_type); } inline hb_ot_name_id_t get_feature_name_id (hb_aat_layout_feature_type_t feature) const { return get_feature (feature).get_feature_name_id (); } - inline hb_ot_name_id_t get_feature_selector_name_id (hb_aat_layout_feature_type_t feature, - hb_aat_layout_feature_selector_t selector) const - { return get_feature (feature).get_feature_selector_name_id (this, selector); } - - inline unsigned int get_selectors (hb_aat_layout_feature_type_t key, - hb_aat_layout_feature_selector_t *default_selector, - unsigned int start_offset, - unsigned int *count, - hb_aat_layout_feature_selector_t *selectors) const + inline unsigned int get_selector_infos (hb_aat_layout_feature_type_t feature_type, + unsigned int start_offset, + unsigned int *selectors_count, /* IN/OUT. May be NULL. */ + hb_aat_layout_feature_selector_info_t *selectors, /* OUT. May be NULL. */ + unsigned int *default_index /* OUT. May be NULL. */) const { - return get_feature (key).get_selectors (this, default_selector, start_offset, - count, selectors); + return get_feature (feature_type).get_selector_infos (start_offset, selectors_count, selectors, + default_index, this); } inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); return_trace (likely (c->check_struct (this) && + version.major == 1 && namesZ.sanitize (c, featureNameCount, this))); } diff --git a/src/hb-aat-layout.cc b/src/hb-aat-layout.cc index 75545adbc..727da42a8 100644 --- a/src/hb-aat-layout.cc +++ b/src/hb-aat-layout.cc @@ -39,6 +39,16 @@ #include "hb-aat-ltag-table.hh" +/** + * SECTION:hb-aat-layout + * @title: hb-aat-layout + * @short_description: Apple Advanced Typography Layout + * @include: hb-aat.h + * + * Functions for querying OpenType Layout features in the font face. + **/ + + /* Table data courtesy of Apple. Converted from mnemonics to integers * when moving to this file. */ static const hb_aat_feature_mapping_t feature_mappings[] = @@ -303,26 +313,26 @@ _hb_aat_language_get (hb_face_t *face, * hb_aat_layout_get_feature_types: * @face: a face object * @start_offset: iteration's start offset - * @count: (inout): buffer size as input, filled size as output - * @features: (out): features buffer + * @feature_count:(inout) (allow-none): buffer size as input, filled size as output + * @features: (out caller-allocates) (array length=feature_count): features buffer * - * Return value: Number of all available features + * Return value: Number of all available feature types. * * Since: REPLACEME */ unsigned int hb_aat_layout_get_feature_types (hb_face_t *face, unsigned int start_offset, - unsigned int *count, /* IN/OUT. May be NULL. */ - hb_aat_layout_feature_type_t *features /* OUT. May be NULL. */) + unsigned int *feature_count, /* IN/OUT. May be NULL. */ + hb_aat_layout_feature_type_t *features /* OUT. May be NULL. */) { - return face->table.feat->get_feature_types (start_offset, count, features); + return face->table.feat->get_feature_types (start_offset, feature_count, features); } /** * hb_aat_layout_feature_type_get_name_id: * @face: a face object - * @feature: feature id + * @feature_type: feature id * * Return value: Name ID index * @@ -330,55 +340,33 @@ hb_aat_layout_get_feature_types (hb_face_t *face, */ hb_ot_name_id_t hb_aat_layout_feature_type_get_name_id (hb_face_t *face, - hb_aat_layout_feature_type_t feature) -{ return face->table.feat->get_feature_name_id (feature); } + hb_aat_layout_feature_type_t feature_type) +{ return face->table.feat->get_feature_name_id (feature_type); } /** * hb_aat_layout_feature_type_get_selectors: * @face: a face object - * @feature: feature id - * @default_selector: (out): if is set, the feature is exclusive + * @feature_type: feature id * @start_offset: iteration's start offset - * @count: (inout): buffer size as input, filled size as output - * @settings: (out): settings buffer + * @selector_count: (inout) (allow-none): buffer size as input, filled size as output + * @selectors: (out caller-allocates) (array length=selector_count): settings buffer + * @default_index: (out) (allow-none): index of default selector if any * - * Per spec: - * For feature types that don't have exclusive settings, - * there will always be a pair of values. One value turns - * a selector on and a second value turns the selector off. - * The on setting must be even and the off setting must be one - * greater than the corresponding on setting. The off setting - * is therefore always odd. As a result, only the on setting - * should have an entry in the setting name array. + * If upon return, @default_index is set to #HB_AAT_LAYOUT_NO_SELECTOR_INDEX, then + * the feature type is non-exclusive. Otherwise, @default_index is the index of + * the selector that is selected by default. * - * Return value: Number of all available features + * Return value: Number of all available feature selectors. * * Since: REPLACEME */ unsigned int -hb_aat_layout_feature_type_get_selectors (hb_face_t *face, - hb_aat_layout_feature_type_t feature, - hb_aat_layout_feature_selector_t *default_selector, /* OUT. May be NULL. */ - unsigned int start_offset, - unsigned int *count, /* IN/OUT. May be NULL. */ - hb_aat_layout_feature_selector_t *selectors /* OUT. May be NULL. */) +hb_aat_layout_feature_type_get_selector_infos (hb_face_t *face, + hb_aat_layout_feature_type_t feature_type, + unsigned int start_offset, + unsigned int *selector_count, /* IN/OUT. May be NULL. */ + hb_aat_layout_feature_selector_info_t *selectors, /* OUT. May be NULL. */ + unsigned int *default_index /* OUT. May be NULL. */) { - return face->table.feat->get_selectors (feature, default_selector, - start_offset, count, selectors); + return face->table.feat->get_selector_infos (feature_type, start_offset, selector_count, selectors, default_index); } - -/** - * hb_aat_layout_feature_selector_get_name_id: - * @face: a face object - * @feature: feature id - * @selector: selector value - * - * Return value: Name ID index - * - * Since: REPLACEME - */ -hb_ot_name_id_t -hb_aat_layout_feature_selector_get_name_id (hb_face_t *face, - hb_aat_layout_feature_type_t feature, - hb_aat_layout_feature_selector_t selector) -{ return face->table.feat->get_feature_selector_name_id (feature, selector); } diff --git a/src/hb-aat-layout.h b/src/hb-aat-layout.h index 8bd4af27d..5912e012b 100644 --- a/src/hb-aat-layout.h +++ b/src/hb-aat-layout.h @@ -430,26 +430,32 @@ typedef enum HB_EXTERN unsigned int hb_aat_layout_get_feature_types (hb_face_t *face, unsigned int start_offset, - unsigned int *count, /* IN/OUT. May be NULL. */ - hb_aat_layout_feature_type_t *features /* OUT. May be NULL. */); + unsigned int *feature_count, /* IN/OUT. May be NULL. */ + hb_aat_layout_feature_type_t *features /* OUT. May be NULL. */); HB_EXTERN hb_ot_name_id_t hb_aat_layout_feature_type_get_name_id (hb_face_t *face, - hb_aat_layout_feature_type_t feature); + hb_aat_layout_feature_type_t feature_type); +typedef struct hb_aat_layout_feature_selector_info_t +{ + hb_ot_name_id_t name_id; + hb_aat_layout_feature_selector_t enable; + hb_aat_layout_feature_selector_t disable; + /*< private >*/ + unsigned int reserved; +} hb_aat_layout_feature_selector_info_t; + +#define HB_AAT_LAYOUT_NO_SELECTOR_INDEX 0xFFFFu HB_EXTERN unsigned int -hb_aat_layout_feature_type_get_selectors (hb_face_t *face, - hb_aat_layout_feature_type_t feature, - hb_aat_layout_feature_selector_t *default_selector, /* OUT. May be NULL. */ - unsigned int start_offset, - unsigned int *count, /* IN/OUT. May be NULL. */ - hb_aat_layout_feature_selector_t *settings /* OUT. May be NULL. */); +hb_aat_layout_feature_type_get_selector_infos (hb_face_t *face, + hb_aat_layout_feature_type_t feature_type, + unsigned int start_offset, + unsigned int *selector_count, /* IN/OUT. May be NULL. */ + hb_aat_layout_feature_selector_info_t *selectors, /* OUT. May be NULL. */ + unsigned int *default_index /* OUT. May be NULL. */); -HB_EXTERN hb_ot_name_id_t -hb_aat_layout_feature_selector_get_name_id (hb_face_t *face, - hb_aat_layout_feature_type_t feature, - hb_aat_layout_feature_selector_t selector); HB_END_DECLS diff --git a/test/api/test-aat-layout.c b/test/api/test-aat-layout.c index 516b46191..358fac879 100644 --- a/test/api/test-aat-layout.c +++ b/test/api/test-aat-layout.c @@ -52,51 +52,53 @@ test_aat_get_feature_types (void) static void test_aat_get_feature_selectors (void) { - hb_aat_layout_feature_selector_t default_selector; - hb_aat_layout_feature_selector_t settings[3]; + unsigned int default_index; + hb_aat_layout_feature_selector_info_t settings[3]; unsigned int count = 3; - g_assert_cmpuint (4, ==, hb_aat_layout_feature_type_get_selectors (face, HB_AAT_LAYOUT_FEATURE_TYPE_DESIGN_COMPLEXITY_TYPE, - &default_selector, 0, &count, settings)); + g_assert_cmpuint (4, ==, hb_aat_layout_feature_type_get_selector_infos (face, + HB_AAT_LAYOUT_FEATURE_TYPE_DESIGN_COMPLEXITY_TYPE, + 0, &count, settings, + &default_index)); g_assert_cmpuint (3, ==, count); - g_assert_cmpuint (0, ==, default_selector); + g_assert_cmpuint (0, ==, default_index); - g_assert_cmpuint (0, ==, settings[0]); - g_assert_cmpuint (294, ==, hb_aat_layout_feature_selector_get_name_id (face, HB_AAT_LAYOUT_FEATURE_TYPE_DESIGN_COMPLEXITY_TYPE, settings[0])); + g_assert_cmpuint (0, ==, settings[0].enable); + g_assert_cmpuint (294, ==, settings[0].name_id); - g_assert_cmpuint (1, ==, settings[1]); - g_assert_cmpuint (295, ==, hb_aat_layout_feature_selector_get_name_id (face, HB_AAT_LAYOUT_FEATURE_TYPE_DESIGN_COMPLEXITY_TYPE, settings[1])); + g_assert_cmpuint (1, ==, settings[1].enable); + g_assert_cmpuint (295, ==, settings[1].name_id); - g_assert_cmpuint (2, ==, settings[2]); - g_assert_cmpuint (296, ==, hb_aat_layout_feature_selector_get_name_id (face, HB_AAT_LAYOUT_FEATURE_TYPE_DESIGN_COMPLEXITY_TYPE, settings[2])); - - g_assert_cmpuint (HB_OT_NAME_ID_INVALID, ==, hb_aat_layout_feature_selector_get_name_id (face, HB_AAT_LAYOUT_FEATURE_TYPE_DESIGN_COMPLEXITY_TYPE, HB_AAT_LAYOUT_FEATURE_SELECTOR_INVALID)); + g_assert_cmpuint (2, ==, settings[2].enable); + g_assert_cmpuint (296, ==, settings[2].name_id); count = 3; - g_assert_cmpuint (4, ==, hb_aat_layout_feature_type_get_selectors (face, HB_AAT_LAYOUT_FEATURE_TYPE_DESIGN_COMPLEXITY_TYPE, - &default_selector, 3, &count, settings)); + g_assert_cmpuint (4, ==, hb_aat_layout_feature_type_get_selector_infos (face, + HB_AAT_LAYOUT_FEATURE_TYPE_DESIGN_COMPLEXITY_TYPE, + 3, &count, settings, + &default_index)); g_assert_cmpuint (1, ==, count); - g_assert_cmpuint (0, ==, default_selector); + g_assert_cmpuint (0, ==, default_index); - g_assert_cmpuint (3, ==, settings[0]); - g_assert_cmpuint (297, ==, hb_aat_layout_feature_selector_get_name_id (face, HB_AAT_LAYOUT_FEATURE_TYPE_DESIGN_COMPLEXITY_TYPE, settings[0])); + g_assert_cmpuint (3, ==, settings[0].enable); + g_assert_cmpuint (297, ==, settings[0].name_id); count = 1; - g_assert_cmpuint (1, ==, hb_aat_layout_feature_type_get_selectors (face, HB_AAT_LAYOUT_FEATURE_TYPE_TYPOGRAPHIC_EXTRAS, - &default_selector, 0, &count, settings)); + g_assert_cmpuint (1, ==, hb_aat_layout_feature_type_get_selector_infos (face, + HB_AAT_LAYOUT_FEATURE_TYPE_TYPOGRAPHIC_EXTRAS, + 0, &count, settings, + &default_index)); g_assert_cmpuint (1, ==, count); - g_assert_cmpuint (HB_AAT_LAYOUT_FEATURE_TYPE_INVALID, ==, default_selector); + g_assert_cmpuint (HB_AAT_LAYOUT_NO_SELECTOR_INDEX, ==, default_index); - g_assert_cmpuint (8, ==, settings[0]); - g_assert_cmpuint (308, ==, hb_aat_layout_feature_selector_get_name_id (face, HB_AAT_LAYOUT_FEATURE_TYPE_TYPOGRAPHIC_EXTRAS, settings[0])); + g_assert_cmpuint (8, ==, settings[0].enable); + g_assert_cmpuint (308, ==, settings[0].name_id); count = 100; - g_assert_cmpuint (0, ==, hb_aat_layout_feature_type_get_selectors (face, HB_AAT_LAYOUT_FEATURE_TYPE_INVALID, - NULL, 0, &count, settings)); + g_assert_cmpuint (0, ==, hb_aat_layout_feature_type_get_selector_infos (face, HB_AAT_LAYOUT_FEATURE_TYPE_INVALID, + 0, &count, settings, + NULL)); g_assert_cmpuint (0, ==, count); - - g_assert_cmpuint (HB_OT_NAME_ID_INVALID, ==, hb_aat_layout_feature_selector_get_name_id (sbix, HB_AAT_LAYOUT_FEATURE_TYPE_INVALID, - (hb_aat_layout_feature_selector_t) 0)); } int