From b1677e76aa111e57b5c8b37d646ccf19a95973ff Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sun, 9 Feb 2025 01:56:03 +0000 Subject: [PATCH 1/3] [trak] Handle "out-of-range" values better If requested size < min-size, use the value for min-size. If requested size > max-size, use the value for max-size. This is the only way that makes sense. Extrapolating as we were doing, is just wrong... This also seems to match what CoreText does. Adjacent to fixing https://github.com/harfbuzz/harfbuzz/issues/5054 --- src/hb-aat-layout-trak-table.hh | 21 +++++++++++++------ test/shape/data/in-house/tests/aat-trak.tests | 5 ++--- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index 17cb93658..da29ab9f0 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -119,17 +119,26 @@ struct TrackData /* * Choose size. */ - unsigned int sizes = nSizes; - if (!sizes) return 0; - if (sizes == 1) return trackTableEntry->get_value (base, 0, sizes); + count = nSizes; + if (!count) return 0; + if (count == 1) return trackTableEntry->get_value (base, 0, count); - hb_array_t size_table ((base+sizeTable).arrayZ, sizes); + // At least two entries. + + hb_array_t size_table ((base+sizeTable).arrayZ, count); unsigned int size_index; - for (size_index = 0; size_index < sizes - 1; size_index++) + for (size_index = 0; size_index < count; size_index++) if (size_table[size_index].to_float () >= ptem) break; - return roundf (interpolate_at (size_index ? size_index - 1 : 0, ptem, + if (size_index == 0) + return trackTableEntry->get_value (base, 0, count); + if (size_index == count) + return trackTableEntry->get_value (base, count - 1, count); + + // Interpolate. + + return roundf (interpolate_at (size_index - 1, ptem, *trackTableEntry, base)); } diff --git a/test/shape/data/in-house/tests/aat-trak.tests b/test/shape/data/in-house/tests/aat-trak.tests index 75c87049c..63ca941a4 100644 --- a/test/shape/data/in-house/tests/aat-trak.tests +++ b/test/shape/data/in-house/tests/aat-trak.tests @@ -5,7 +5,6 @@ ../fonts/TRAK.ttf;--font-ptem=9;U+0041,U+0042,U+0043;[A.alt=0@30,0+1060|B=1@30,0+1060|C.alt=2@30,0+1060] ../fonts/TRAK.ttf;--font-ptem=24;U+0041,U+0042,U+0043;[A.alt=0@-7,0+986|B=1@-7,0+986|C.alt=2@-7,0+986] ../fonts/TRAK.ttf;--font-ptem=72;U+0041,U+0042,U+0043;[A.alt=0@-35,0+929|B=1@-35,0+929|C.alt=2@-35,0+929] -../fonts/TRAK.ttf;--font-ptem=144;U+0041,U+0042,U+0043;[A.alt=0@-78,0+843|B=1@-78,0+843|C.alt=2@-78,0+843] -../fonts/TRAK.ttf;--font-ptem=144;U+0041,U+0042,U+0043;[A.alt=0@-78,0+843|B=1@-78,0+843|C.alt=2@-78,0+843] +../fonts/TRAK.ttf;--font-ptem=144;U+0041,U+0042,U+0043;[A.alt=0@-50,0+900|B=1@-50,0+900|C.alt=2@-50,0+900] ../fonts/TRAK.ttf;--font-ptem=144 --features=-trak;U+0041,U+0042,U+0043;[A.alt=0+1000|B=1+1000|C.alt=2+1000] -../fonts/TRAK.ttf;--font-ptem=144 --features=-trak[1:3];U+0041,U+0042,U+0043,U+0041,U+0042,U+0043;[A.alt=0@-78,0+843|B=1+1000|C.alt=2+1000|A.alt=3@-78,0+843|B=4@-78,0+843|C.alt=5@-78,0+843] +../fonts/TRAK.ttf;--font-ptem=144 --features=-trak[1:3];U+0041,U+0042,U+0043,U+0041,U+0042,U+0043;[A.alt=0@-50,0+900|B=1+1000|C.alt=2+1000|A.alt=3@-50,0+900|B=4@-50,0+900|C.alt=5@-50,0+900] From 1bf0a5bc17f2d1040a144ceafbd97205eb9976f0 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sun, 9 Feb 2025 02:43:09 +0000 Subject: [PATCH 2/3] [trak] Streamline a bit Also use float math. --- src/hb-aat-layout-trak-table.hh | 112 ++++++++++-------- test/shape/data/in-house/tests/aat-trak.tests | 2 +- 2 files changed, 65 insertions(+), 49 deletions(-) diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index da29ab9f0..93ad558c5 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -48,17 +48,69 @@ struct TrackTableEntry float get_track_value () const { return track.to_float (); } - int get_value (const void *base, unsigned int index, - unsigned int table_size) const - { return (base+valuesZ).as_array (table_size)[index]; } + float interpolate_at (unsigned int idx, + float ptem, + const void *base, + hb_array_t size_table) const + { + const FWORD *values = (base+valuesZ).arrayZ; + + float s0 = size_table[idx].to_float (); + float s1 = size_table[idx + 1].to_float (); + int v0 = values[idx]; + int v1 = values[idx + 1]; + + // Deal with font bugs. + if (unlikely (s1 < s0)) + { hb_swap (s0, s1); hb_swap (v0, v1); } + if (unlikely (ptem < s0)) return v0; + if (unlikely (ptem > s1)) return v1; + if (unlikely (s0 == s1)) return (v0 + v1) * 0.5f; + + float t = (ptem - s0) / (s1 - s0); + return v0 + t * (v1 - v0); + } + + float get_value (float ptem, + const void *base, + hb_array_t size_table) const + { + const FWORD *values = (base+valuesZ).arrayZ; + + unsigned int n_sizes = size_table.length; + + /* + * Choose size. + */ + if (!n_sizes) return 0.f; + if (n_sizes == 1) return values[0]; + + // At least two entries. + + unsigned i; + for (i = 0; i < n_sizes; i++) + if (size_table[i].to_float () >= ptem) + break; + + // Boundary conditions. + if (i == 0) return values[0]; + if (i == n_sizes) return values[n_sizes - 1]; + + // Exact match. + if (size_table[i].to_float () == ptem) return values[i]; + + // Interpolate. + return interpolate_at (i - 1, ptem, base, size_table); + } public: - bool sanitize (hb_sanitize_context_t *c, const void *base, - unsigned int table_size) const + bool sanitize (hb_sanitize_context_t *c, + const void *base, + unsigned int n_sizes) const { TRACE_SANITIZE (this); return_trace (likely (c->check_struct (this) && - (valuesZ.sanitize (c, base, table_size)))); + (valuesZ.sanitize (c, base, n_sizes)))); } protected: @@ -76,22 +128,7 @@ struct TrackTableEntry struct TrackData { - float interpolate_at (unsigned int idx, - float target_size, - const TrackTableEntry &trackTableEntry, - const void *base) const - { - unsigned int sizes = nSizes; - hb_array_t size_table ((base+sizeTable).arrayZ, sizes); - - float s0 = size_table[idx].to_float (); - float s1 = size_table[idx + 1].to_float (); - float t = unlikely (s0 == s1) ? 0.f : (target_size - s0) / (s1 - s0); - return t * trackTableEntry.get_value (base, idx + 1, sizes) + - (1.f - t) * trackTableEntry.get_value (base, idx, sizes); - } - - int get_tracking (const void *base, float ptem) const + float get_tracking (const void *base, float ptem) const { /* * Choose track. @@ -116,30 +153,9 @@ struct TrackData } if (!trackTableEntry) return 0; - /* - * Choose size. - */ - count = nSizes; - if (!count) return 0; - if (count == 1) return trackTableEntry->get_value (base, 0, count); - - // At least two entries. - - hb_array_t size_table ((base+sizeTable).arrayZ, count); - unsigned int size_index; - for (size_index = 0; size_index < count; size_index++) - if (size_table[size_index].to_float () >= ptem) - break; - - if (size_index == 0) - return trackTableEntry->get_value (base, 0, count); - if (size_index == count) - return trackTableEntry->get_value (base, count - 1, count); - - // Interpolate. - - return roundf (interpolate_at (size_index - 1, ptem, - *trackTableEntry, base)); + return trackTableEntry->get_value (ptem, + base, + (base+sizeTable).as_array (nSizes)); } bool sanitize (hb_sanitize_context_t *c, const void *base) const @@ -187,7 +203,7 @@ struct trak if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) { const TrackData &trackData = this+horizData; - int tracking = trackData.get_tracking (this, ptem); + float tracking = trackData.get_tracking (this, ptem); hb_position_t offset_to_add = c->font->em_scalef_x (tracking / 2); hb_position_t advance_to_add = c->font->em_scalef_x (tracking); foreach_grapheme (buffer, start, end) @@ -200,7 +216,7 @@ struct trak else { const TrackData &trackData = this+vertData; - int tracking = trackData.get_tracking (this, ptem); + float tracking = trackData.get_tracking (this, ptem); hb_position_t offset_to_add = c->font->em_scalef_y (tracking / 2); hb_position_t advance_to_add = c->font->em_scalef_y (tracking); foreach_grapheme (buffer, start, end) diff --git a/test/shape/data/in-house/tests/aat-trak.tests b/test/shape/data/in-house/tests/aat-trak.tests index 63ca941a4..b293fbc2e 100644 --- a/test/shape/data/in-house/tests/aat-trak.tests +++ b/test/shape/data/in-house/tests/aat-trak.tests @@ -4,7 +4,7 @@ ../fonts/TRAK.ttf;--font-ptem=2;U+0041,U+0042,U+0043;[A.alt=0@100,0+1200|B=1@100,0+1200|C.alt=2@100,0+1200] ../fonts/TRAK.ttf;--font-ptem=9;U+0041,U+0042,U+0043;[A.alt=0@30,0+1060|B=1@30,0+1060|C.alt=2@30,0+1060] ../fonts/TRAK.ttf;--font-ptem=24;U+0041,U+0042,U+0043;[A.alt=0@-7,0+986|B=1@-7,0+986|C.alt=2@-7,0+986] -../fonts/TRAK.ttf;--font-ptem=72;U+0041,U+0042,U+0043;[A.alt=0@-35,0+929|B=1@-35,0+929|C.alt=2@-35,0+929] +../fonts/TRAK.ttf;--font-ptem=72;U+0041,U+0042,U+0043;[A.alt=0@-36,0+929|B=1@-36,0+929|C.alt=2@-36,0+929] ../fonts/TRAK.ttf;--font-ptem=144;U+0041,U+0042,U+0043;[A.alt=0@-50,0+900|B=1@-50,0+900|C.alt=2@-50,0+900] ../fonts/TRAK.ttf;--font-ptem=144 --features=-trak;U+0041,U+0042,U+0043;[A.alt=0+1000|B=1+1000|C.alt=2+1000] ../fonts/TRAK.ttf;--font-ptem=144 --features=-trak[1:3];U+0041,U+0042,U+0043,U+0041,U+0042,U+0043;[A.alt=0@-50,0+900|B=1+1000|C.alt=2+1000|A.alt=3@-50,0+900|B=4@-50,0+900|C.alt=5@-50,0+900] From a70a30ddfdfde9d47cba9bb2828e52dafd5ecb20 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sun, 9 Feb 2025 02:55:48 +0000 Subject: [PATCH 3/3] [trak] Interpolate between tracks Fixes https://github.com/harfbuzz/harfbuzz/issues/5054 According to Ned, this is what CoreText does. Should add tests some time... --- src/hb-aat-layout-trak-table.hh | 52 ++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index 93ad558c5..ee80c5150 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -128,34 +128,38 @@ struct TrackTableEntry struct TrackData { - float get_tracking (const void *base, float ptem) const + float get_tracking (const void *base, float ptem, float track = 0.f) const { - /* - * Choose track. - */ - const TrackTableEntry *trackTableEntry = nullptr; - unsigned int count = nTracks; - float last_trak = 1e5; - for (unsigned int i = 0; i < count; i++) - { - /* Note: Seems like the track entries are sorted by values. But the - * spec doesn't explicitly say that. It just mentions it in the example. */ + unsigned count = nTracks; + hb_array_t size_table = (base+sizeTable).as_array (nSizes); - /* Not sure what CoreText does, but it looks to apply a trak=1.0 by default - * if there is no 0.0 trak. So, just pick the one closest to 0.0. */ + if (!count) return 0.f; + if (count == 1) return trackTable[0].get_value (ptem, base, size_table); - float trak = trackTable[i].get_track_value (); - if (fabsf (trak) < fabsf (last_trak)) - { - trackTableEntry = &trackTable[i]; - break; - } - } - if (!trackTableEntry) return 0; + // At least two entries. - return trackTableEntry->get_value (ptem, - base, - (base+sizeTable).as_array (nSizes)); + unsigned i = 0; + unsigned j = count - 1; + + // Find the two entries that track is between. + while (i + 1 < count && trackTable[i + 1].get_track_value () < track) + i++; + while (j > 0 && trackTable[j - 1].get_track_value () > track) + j--; + + // Exact match. + if (i == j) return trackTable[i].get_value (ptem, base, size_table); + + // Interpolate. + + float t0 = trackTable[i].get_track_value (); + float t1 = trackTable[j].get_track_value (); + + float t = (track - t0) / (t1 - t0); + + float a = trackTable[i].get_value (ptem, base, size_table); + float b = trackTable[j].get_value (ptem, base, size_table); + return a + t * (b - a); } bool sanitize (hb_sanitize_context_t *c, const void *base) const