From 6c017a1162265bee8c5d09cd89aef26ef25afe51 Mon Sep 17 00:00:00 2001 From: Qunxin Liu Date: Wed, 1 Nov 2023 09:54:46 -0700 Subject: [PATCH] [instancer] SinglePos/PairPos: do not strip_hints for partial instancing --- src/OT/Layout/GDEF/GDEF.hh | 1 + src/OT/Layout/GPOS/Common.hh | 2 +- src/OT/Layout/GPOS/PairPosFormat1.hh | 35 ++++++++++------ src/OT/Layout/GPOS/PairPosFormat2.hh | 37 ++++++++++------- src/OT/Layout/GPOS/SinglePos.hh | 13 +++--- src/OT/Layout/GPOS/SinglePosFormat1.hh | 26 +++++++++++- src/OT/Layout/GPOS/SinglePosFormat2.hh | 38 ++++++++++++++++- src/OT/Layout/GPOS/ValueFormat.hh | 57 +++++++++++--------------- src/hb-subset-plan.cc | 1 + src/hb-subset-plan.hh | 3 ++ src/hb-subset.cc | 2 +- 11 files changed, 142 insertions(+), 73 deletions(-) diff --git a/src/OT/Layout/GDEF/GDEF.hh b/src/OT/Layout/GDEF/GDEF.hh index feecc7892..d3611b98f 100644 --- a/src/OT/Layout/GDEF/GDEF.hh +++ b/src/OT/Layout/GDEF/GDEF.hh @@ -705,6 +705,7 @@ struct GDEFVersion1_2 if (subset_varstore) { out->version.minor = 3; + c->plan->has_gdef_varstore = true; } else if (subset_markglyphsetsdef) { out->version.minor = 2; c->serializer->revert (snapshot_version2); diff --git a/src/OT/Layout/GPOS/Common.hh b/src/OT/Layout/GPOS/Common.hh index 20d63b939..696d25d75 100644 --- a/src/OT/Layout/GPOS/Common.hh +++ b/src/OT/Layout/GPOS/Common.hh @@ -23,7 +23,7 @@ static void SinglePos_serialize (hb_serialize_context_t *c, const SrcLookup *src, Iterator it, const hb_hashmap_t> *layout_variation_idx_delta_map, - bool update_var_device_flags); + unsigned new_format); } diff --git a/src/OT/Layout/GPOS/PairPosFormat1.hh b/src/OT/Layout/GPOS/PairPosFormat1.hh index 74ab7d661..fa6460852 100644 --- a/src/OT/Layout/GPOS/PairPosFormat1.hh +++ b/src/OT/Layout/GPOS/PairPosFormat1.hh @@ -135,10 +135,26 @@ struct PairPosFormat1_3 hb_pair_t newFormats = hb_pair (valueFormat[0], valueFormat[1]); if (c->plan->normalized_coords) - newFormats = compute_effective_value_formats (glyphset, &c->plan->layout_variation_idx_delta_map); + { + /* all device flags will be dropped when full instancing, no need to strip + * hints, also do not strip emtpy cause we don't compute the new default + * value during stripping */ + newFormats = compute_effective_value_formats (glyphset, false, false, &c->plan->layout_variation_idx_delta_map); + } + /* do not strip hints for VF */ + else if (c->plan->flags & HB_SUBSET_FLAGS_NO_HINTING) + { + hb_blob_t* blob = hb_face_reference_table (c->plan->source, HB_TAG ('f','v','a','r')); + bool has_fvar = (blob != hb_blob_get_empty ()); + hb_blob_destroy (blob); - if (c->plan->flags & HB_SUBSET_FLAGS_NO_HINTING) - newFormats = compute_effective_value_formats (glyphset); + bool strip = !has_fvar; + /* special case: strip hints when a VF has no GDEF varstore after + * subsetting*/ + if (has_fvar && !c->plan->has_gdef_varstore) + strip = true; + newFormats = compute_effective_value_formats (glyphset, strip, true); + } out->valueFormat[0] = newFormats.first; out->valueFormat[1] = newFormats.second; @@ -173,6 +189,7 @@ struct PairPosFormat1_3 hb_pair_t compute_effective_value_formats (const hb_set_t& glyphset, + bool strip_hints, bool strip_empty, const hb_hashmap_t> *varidx_delta_map = nullptr) const { unsigned record_size = PairSet::get_size (valueFormat); @@ -193,16 +210,8 @@ struct PairPosFormat1_3 { if (record->intersects (glyphset)) { - if (!varidx_delta_map) - { - format1 = format1 | valueFormat[0].get_effective_format (record->get_values_1 ()); - format2 = format2 | valueFormat[1].get_effective_format (record->get_values_2 (valueFormat[0])); - } - else - { - format1 = format1 | valueFormat[0].update_var_device_table_flags (record->get_values_1 (), &set, varidx_delta_map); - format2 = format2 | valueFormat[1].update_var_device_table_flags (record->get_values_2 (valueFormat[0]), &set, varidx_delta_map); - } + format1 = format1 | valueFormat[0].get_effective_format (record->get_values_1 (), strip_hints, strip_empty, &set, varidx_delta_map); + format2 = format2 | valueFormat[1].get_effective_format (record->get_values_2 (valueFormat[0]), strip_hints, strip_empty, &set, varidx_delta_map); } record = &StructAtOffset (record, record_size); } diff --git a/src/OT/Layout/GPOS/PairPosFormat2.hh b/src/OT/Layout/GPOS/PairPosFormat2.hh index 2378677b9..dd02da887 100644 --- a/src/OT/Layout/GPOS/PairPosFormat2.hh +++ b/src/OT/Layout/GPOS/PairPosFormat2.hh @@ -287,11 +287,27 @@ struct PairPosFormat2_4 : ValueBase unsigned len2 = valueFormat2.get_len (); hb_pair_t newFormats = hb_pair (valueFormat1, valueFormat2); - if (c->plan->normalized_coords) - newFormats = compute_effective_value_formats (klass1_map, klass2_map, &c->plan->layout_variation_idx_delta_map); - if (c->plan->flags & HB_SUBSET_FLAGS_NO_HINTING) - newFormats = compute_effective_value_formats (klass1_map, klass2_map); + if (c->plan->normalized_coords) + { + /* in case of full instancing, all var device flags will be dropped so no + * need to strip hints here */ + newFormats = compute_effective_value_formats (klass1_map, klass2_map, false, false, &c->plan->layout_variation_idx_delta_map); + } + /* do not strip hints for VF */ + else if (c->plan->flags & HB_SUBSET_FLAGS_NO_HINTING) + { + hb_blob_t* blob = hb_face_reference_table (c->plan->source, HB_TAG ('f','v','a','r')); + bool has_fvar = (blob != hb_blob_get_empty ()); + hb_blob_destroy (blob); + + bool strip = !has_fvar; + /* special case: strip hints when a VF has no GDEF varstore after + * subsetting*/ + if (has_fvar && !c->plan->has_gdef_varstore) + strip = true; + newFormats = compute_effective_value_formats (klass1_map, klass2_map, strip, true); + } out->valueFormat1 = newFormats.first; out->valueFormat2 = newFormats.second; @@ -324,6 +340,7 @@ struct PairPosFormat2_4 : ValueBase hb_pair_t compute_effective_value_formats (const hb_map_t& klass1_map, const hb_map_t& klass2_map, + bool strip_hints, bool strip_empty, const hb_hashmap_t> *varidx_delta_map = nullptr) const { unsigned len1 = valueFormat1.get_len (); @@ -338,16 +355,8 @@ struct PairPosFormat2_4 : ValueBase for (unsigned class2_idx : + hb_range ((unsigned) class2Count) | hb_filter (klass2_map)) { unsigned idx = (class1_idx * (unsigned) class2Count + class2_idx) * record_size; - if (!varidx_delta_map) - { - format1 = format1 | valueFormat1.get_effective_format (&values[idx]); - format2 = format2 | valueFormat2.get_effective_format (&values[idx + len1]); - } - else - { - format1 = format1 | valueFormat1.update_var_device_table_flags (&values[idx], this, varidx_delta_map); - format2 = format2 | valueFormat2.update_var_device_table_flags (&values[idx + len1], this, varidx_delta_map); - } + format1 = format1 | valueFormat1.get_effective_format (&values[idx], strip_hints, strip_empty, this, varidx_delta_map); + format2 = format2 | valueFormat2.get_effective_format (&values[idx + len1], strip_hints, strip_empty, this, varidx_delta_map); } if (format1 == valueFormat1 && format2 == valueFormat2) diff --git a/src/OT/Layout/GPOS/SinglePos.hh b/src/OT/Layout/GPOS/SinglePos.hh index 7aeac91a4..a0243a218 100644 --- a/src/OT/Layout/GPOS/SinglePos.hh +++ b/src/OT/Layout/GPOS/SinglePos.hh @@ -39,15 +39,12 @@ struct SinglePos const SrcLookup* src, Iterator glyph_val_iter_pairs, const hb_hashmap_t> *layout_variation_idx_delta_map, - bool update_var_device_flags) + unsigned newFormat) { if (unlikely (!c->extend_min (u.format))) return; unsigned format = 2; - ValueFormat new_format = src->get_value_format (); - - if (update_var_device_flags) - new_format = new_format.update_var_device_table_flags (+ glyph_val_iter_pairs | hb_map (hb_second), - src, layout_variation_idx_delta_map); + ValueFormat new_format; + new_format = newFormat; if (glyph_val_iter_pairs) format = get_format (glyph_val_iter_pairs); @@ -90,8 +87,8 @@ SinglePos_serialize (hb_serialize_context_t *c, const SrcLookup *src, Iterator it, const hb_hashmap_t> *layout_variation_idx_delta_map, - bool update_var_device_flags) -{ c->start_embed ()->serialize (c, src, it, layout_variation_idx_delta_map, update_var_device_flags); } + unsigned new_format) +{ c->start_embed ()->serialize (c, src, it, layout_variation_idx_delta_map, new_format); } } diff --git a/src/OT/Layout/GPOS/SinglePosFormat1.hh b/src/OT/Layout/GPOS/SinglePosFormat1.hh index a04f15826..57cb826e6 100644 --- a/src/OT/Layout/GPOS/SinglePosFormat1.hh +++ b/src/OT/Layout/GPOS/SinglePosFormat1.hh @@ -146,6 +146,30 @@ struct SinglePosFormat1 : ValueBase hb_set_t intersection; (this+coverage).intersect_set (glyphset, intersection); + unsigned new_format = valueFormat; + + if (c->plan->normalized_coords) + { + new_format = valueFormat.get_effective_format (values.arrayZ, false, false, this, &c->plan->layout_variation_idx_delta_map); + } + /* do not strip hints for VF */ + else if (c->plan->flags & HB_SUBSET_FLAGS_NO_HINTING) + { + hb_blob_t* blob = hb_face_reference_table (c->plan->source, HB_TAG ('f','v','a','r')); + bool has_fvar = (blob != hb_blob_get_empty ()); + hb_blob_destroy (blob); + + bool strip = !has_fvar; + /* special case: strip hints when a VF has no GDEF varstore after + * subsetting*/ + if (has_fvar && !c->plan->has_gdef_varstore) + strip = true; + new_format = valueFormat.get_effective_format (values.arrayZ, + strip, /* strip hints */ + true, /* strip empty */ + this, nullptr); + } + auto it = + hb_iter (intersection) | hb_map_retains_sorting (glyph_map) @@ -153,7 +177,7 @@ struct SinglePosFormat1 : ValueBase ; bool ret = bool (it); - SinglePos_serialize (c->serializer, this, it, &c->plan->layout_variation_idx_delta_map, bool (c->plan->normalized_coords)); + SinglePos_serialize (c->serializer, this, it, &c->plan->layout_variation_idx_delta_map, new_format); return_trace (ret); } }; diff --git a/src/OT/Layout/GPOS/SinglePosFormat2.hh b/src/OT/Layout/GPOS/SinglePosFormat2.hh index 791c18a15..ae4a5ed75 100644 --- a/src/OT/Layout/GPOS/SinglePosFormat2.hh +++ b/src/OT/Layout/GPOS/SinglePosFormat2.hh @@ -143,6 +143,37 @@ struct SinglePosFormat2 : ValueBase coverage.serialize_serialize (c, glyphs); } + template + unsigned compute_effective_format (const hb_face_t *face, + Iterator it, + bool is_instancing, bool strip_hints, + bool has_gdef_varstore, + const hb_hashmap_t> *varidx_delta_map) const + { + hb_blob_t* blob = hb_face_reference_table (face, HB_TAG ('f','v','a','r')); + bool has_fvar = (blob != hb_blob_get_empty ()); + hb_blob_destroy (blob); + + unsigned new_format = 0; + if (is_instancing) + { + new_format = new_format | valueFormat.get_effective_format (+ it | hb_map (hb_second), false, false, this, varidx_delta_map); + } + /* do not strip hints for VF */ + else if (strip_hints) + { + bool strip = !has_fvar; + if (has_fvar && !has_gdef_varstore) + strip = true; + new_format = new_format | valueFormat.get_effective_format (+ it | hb_map (hb_second), strip, true, this, nullptr); + } + else + new_format = valueFormat; + + return new_format; + } + bool subset (hb_subset_context_t *c) const { TRACE_SUBSET (this); @@ -163,8 +194,13 @@ struct SinglePosFormat2 : ValueBase }) ; + unsigned new_format = compute_effective_format (c->plan->source, it, + bool (c->plan->normalized_coords), + bool (c->plan->flags & HB_SUBSET_FLAGS_NO_HINTING), + c->plan->has_gdef_varstore, + &c->plan->layout_variation_idx_delta_map); bool ret = bool (it); - SinglePos_serialize (c->serializer, this, it, &c->plan->layout_variation_idx_delta_map, bool (c->plan->normalized_coords)); + SinglePos_serialize (c->serializer, this, it, &c->plan->layout_variation_idx_delta_map, new_format); return_trace (ret); } }; diff --git a/src/OT/Layout/GPOS/ValueFormat.hh b/src/OT/Layout/GPOS/ValueFormat.hh index 5a3d16050..7a74e391c 100644 --- a/src/OT/Layout/GPOS/ValueFormat.hh +++ b/src/OT/Layout/GPOS/ValueFormat.hh @@ -144,11 +144,29 @@ struct ValueFormat : HBUINT16 return ret; } - unsigned int get_effective_format (const Value *values) const + unsigned int get_effective_format (const Value *values, bool strip_hints, bool strip_empty, const void *base, + const hb_hashmap_t> *varidx_delta_map) const { unsigned int format = *this; for (unsigned flag = xPlacement; flag <= yAdvDevice; flag = flag << 1) { - if (format & flag) should_drop (*values++, (Flags) flag, &format); + if (format & flag) + { + if (strip_hints && flag >= xPlaDevice) + { + format = format & ~flag; + values++; + continue; + } + if (varidx_delta_map && flag >= xPlaDevice) + { + update_var_flag (values++, (Flags) flag, &format, base, varidx_delta_map); + continue; + } + /* do not strip empty when instancing, cause we don't know whether the new + * default value is 0 or not */ + if (strip_empty) should_drop (*values, (Flags) flag, &format); + values++; + } } return format; @@ -156,11 +174,12 @@ struct ValueFormat : HBUINT16 template - unsigned int get_effective_format (Iterator it) const { + unsigned int get_effective_format (Iterator it, bool strip_hints, bool strip_empty, const void *base, + const hb_hashmap_t> *varidx_delta_map) const { unsigned int new_format = 0; for (const hb_array_t& values : it) - new_format = new_format | get_effective_format (&values); + new_format = new_format | get_effective_format (&values, strip_hints, strip_empty, base, varidx_delta_map); return new_format; } @@ -253,36 +272,6 @@ struct ValueFormat : HBUINT16 } } - unsigned update_var_device_table_flags (const Value *values, - const ValueBase *base, - const hb_hashmap_t> *varidx_delta_map) const - { - unsigned format = *this; - for (unsigned flag = xPlacement; flag <= yAdvDevice; flag = flag << 1) - { - if (format & flag) - { - if (flag >= xPlaDevice) update_var_flag (values, (Flags) flag, &format, base, varidx_delta_map); - values++; - } - } - - return format; - } - - template - unsigned update_var_device_table_flags (Iterator it, - const ValueBase* base, - const hb_hashmap_t> *varidx_delta_map) const - { - unsigned new_format = 0; - for (const hb_array_t& values : it) - new_format = new_format | update_var_device_table_flags (&values, base, varidx_delta_map); - - return new_format; - } - private: bool sanitize_value_devices (hb_sanitize_context_t *c, const ValueBase *base, const Value *values) const { diff --git a/src/hb-subset-plan.cc b/src/hb-subset-plan.cc index 471ed6a09..c6a5bfc68 100644 --- a/src/hb-subset-plan.cc +++ b/src/hb-subset-plan.cc @@ -1124,6 +1124,7 @@ hb_subset_plan_t::hb_subset_plan_t (hb_face_t *face, user_axes_location = input->axes_location; all_axes_pinned = false; pinned_at_default = true; + has_gdef_varstore = false; #ifdef HB_EXPERIMENTAL_API for (auto _ : input->name_table_overrides) diff --git a/src/hb-subset-plan.hh b/src/hb-subset-plan.hh index a05d1d1a6..1f19a58c1 100644 --- a/src/hb-subset-plan.hh +++ b/src/hb-subset-plan.hh @@ -147,6 +147,9 @@ struct hb_subset_plan_t bool gsub_insert_catch_all_feature_variation_rec; bool gpos_insert_catch_all_feature_variation_rec; + // whether GDEF VarStore is retained + mutable bool has_gdef_varstore; + #define HB_SUBSET_PLAN_MEMBER(Type, Name) Type Name; #include "hb-subset-plan-member-list.hh" #undef HB_SUBSET_PLAN_MEMBER diff --git a/src/hb-subset.cc b/src/hb-subset.cc index 229b1c308..de0fc590e 100644 --- a/src/hb-subset.cc +++ b/src/hb-subset.cc @@ -462,7 +462,7 @@ _dependencies_satisfied (hb_subset_plan_t *plan, hb_tag_t tag, case HB_OT_TAG_maxp: return !plan->normalized_coords || !pending_subset_tags.has (HB_OT_TAG_glyf); case HB_OT_TAG_GPOS: - return !plan->normalized_coords || plan->all_axes_pinned || !pending_subset_tags.has (HB_OT_TAG_GDEF); + return plan->all_axes_pinned || !pending_subset_tags.has (HB_OT_TAG_GDEF); default: return true; }