From b6cfacfd9f54335638cccdd4e4ddca5dcef15b94 Mon Sep 17 00:00:00 2001
From: LukasACH <git@lukasach.dev>
Date: Tue, 21 Mar 2023 14:39:58 +0100
Subject: [PATCH] LibGfx/OpenType: Get size of ValueRecord from PairPos
 valueFormat field

The stored ValueRecord in the font file only contains the fields
specified in the valueFormat field of the PairPosFormat1 table.
This means we need to construct the ValueRecord dynamically at runtime
and cannot bit_cast it to a struct.
---
 .../Libraries/LibGfx/Font/OpenType/Font.cpp   | 38 ++++++++++++++-----
 .../Libraries/LibGfx/Font/OpenType/Tables.h   | 14 +------
 2 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/Userland/Libraries/LibGfx/Font/OpenType/Font.cpp b/Userland/Libraries/LibGfx/Font/OpenType/Font.cpp
index 6b070580f7..03dced6323 100644
--- a/Userland/Libraries/LibGfx/Font/OpenType/Font.cpp
+++ b/Userland/Libraries/LibGfx/Font/OpenType/Font.cpp
@@ -1085,9 +1085,6 @@ Optional<i16> GPOS::glyph_kerning(u16 left_glyph_id, u16 right_glyph_id) const
                 dbgln_if(OPENTYPE_GPOS_DEBUG, "   valueFormat2: {}", pair_pos_format1.value_format2);
                 dbgln_if(OPENTYPE_GPOS_DEBUG, "   pairSetCount: {}", pair_pos_format1.pair_set_count);
 
-                if (pair_pos_format1.value_format1 == 0)
-                    continue;
-
                 auto get_coverage_index = [&](u16 glyph_id, Offset16 coverage_format_offset) -> Optional<u16> {
                     auto coverage_format_slice = pair_pos_format_slice.slice(coverage_format_offset);
                     auto const& coverage_format = *bit_cast<BigEndian<u16> const*>(coverage_format_slice.data());
@@ -1128,18 +1125,41 @@ Optional<i16> GPOS::glyph_kerning(u16 left_glyph_id, u16 right_glyph_id) const
                     return {};
                 }
 
+                size_t value1_size = popcount(static_cast<u32>(pair_pos_format1.value_format1 & 0xff)) * sizeof(u16);
+                size_t value2_size = popcount(static_cast<u32>(pair_pos_format1.value_format2 & 0xff)) * sizeof(u16);
+                dbgln_if(OPENTYPE_GPOS_DEBUG, "ValueSizes: {}, {}", value1_size, value2_size);
+
+                // Manually iterate over the PairSet table, as the size of each PairValueRecord is not known at compile time.
                 auto pair_set_offset = pair_pos_format1.pair_set_offsets[coverage_index.value()];
                 auto pair_set_slice = pair_pos_format_slice.slice(pair_set_offset);
 
-                auto const& pair_set = *bit_cast<GPOS::PairSet const*>(pair_set_slice.data());
+                FixedMemoryStream stream(pair_set_slice);
 
-                for (size_t k = 0; k < pair_set.pair_value_count; ++k) {
-                    auto pair_value_record = pair_set.pair_value_records[k];
-                    if (right_glyph_id == pair_value_record.second_glyph) {
-                        dbgln_if(OPENTYPE_GPOS_DEBUG, "Returning x advance {}", pair_value_record.value_record1.x_advance);
-                        return pair_value_record.value_record1.x_advance;
+                auto pair_value_count = stream.read_value<BigEndian<u16>>().release_value_but_fixme_should_propagate_errors();
+
+                bool found_matching_glyph = false;
+                for (size_t k = 0; k < pair_value_count; ++k) {
+                    auto second_glyph = stream.read_value<BigEndian<u16>>().release_value_but_fixme_should_propagate_errors();
+
+                    if (right_glyph_id == second_glyph) {
+                        dbgln_if(OPENTYPE_GPOS_DEBUG, "Found matching second glyph {}", second_glyph);
+                        found_matching_glyph = true;
+                        break;
                     }
+
+                    (void)stream.discard(value1_size + value2_size).release_value_but_fixme_should_propagate_errors();
                 }
+
+                if (!found_matching_glyph) {
+                    dbgln_if(OPENTYPE_GPOS_DEBUG, "Did not find second glyph matching {}", right_glyph_id);
+                    continue;
+                }
+
+                [[maybe_unused]] auto value_record1 = read_value_record(pair_pos_format1.value_format1, stream);
+                [[maybe_unused]] auto value_record2 = read_value_record(pair_pos_format1.value_format2, stream);
+
+                dbgln_if(OPENTYPE_GPOS_DEBUG, "Returning x advance {}", value_record1.x_advance);
+                return value_record1.x_advance;
             }
 
             else if (pair_pos_format == 2) {
diff --git a/Userland/Libraries/LibGfx/Font/OpenType/Tables.h b/Userland/Libraries/LibGfx/Font/OpenType/Tables.h
index 0106d57a41..0009e2e09e 100644
--- a/Userland/Libraries/LibGfx/Font/OpenType/Tables.h
+++ b/Userland/Libraries/LibGfx/Font/OpenType/Tables.h
@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2020, Srimanta Barua <srimanta.barua1@gmail.com>
  * Copyright (c) 2022, Jelle Raaijmakers <jelle@gmta.nl>
+ * Copyright (c) 2023, Lukas Affolter <git@lukasach.dev>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -637,19 +638,6 @@ public:
         Offset16 y_advance_device_offset;
     };
 
-    // https://learn.microsoft.com/en-us/typography/opentype/spec/gpos#pair-adjustment-positioning-format-1-adjustments-for-glyph-pairs
-    struct PairValueRecord {
-        BigEndian<u16> second_glyph;
-        ValueRecord value_record1;
-        ValueRecord value_record2;
-    };
-
-    // https://learn.microsoft.com/en-us/typography/opentype/spec/gpos#pair-adjustment-positioning-format-1-adjustments-for-glyph-pairs
-    struct PairSet {
-        BigEndian<u16> pair_value_count;
-        PairValueRecord pair_value_records[];
-    };
-
     // https://learn.microsoft.com/en-us/typography/opentype/spec/gpos#pair-adjustment-positioning-format-2-class-pair-adjustment
     struct PairPosFormat2 {
         BigEndian<u16> pos_format;