From 0dee94ef402bd908d62439813a49a1a2d3f8f377 Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis@chromium.org>
Date: Tue, 20 Feb 2024 11:42:59 -0500
Subject: [PATCH] LibPDF+LibGfx: Do not try to read "hmtx" table for PDFs

It is sometimes truncated in fonts embedded in PDFs, and the data
is not needed to render PDFs. 26 of my 1000 test files complained
"Could not load Hmtx: Not enough data" before.

Increases number of PDFs that render without diagnostics from
743 to 764.
---
 Userland/Libraries/LibGfx/Font/OpenType/Font.cpp | 13 ++++++++-----
 Userland/Libraries/LibGfx/Font/OpenType/Font.h   |  7 +++++--
 Userland/Libraries/LibPDF/Fonts/PDFFont.h        |  2 +-
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/Userland/Libraries/LibGfx/Font/OpenType/Font.cpp b/Userland/Libraries/LibGfx/Font/OpenType/Font.cpp
index 23d89cb498..51c231b5f2 100644
--- a/Userland/Libraries/LibGfx/Font/OpenType/Font.cpp
+++ b/Userland/Libraries/LibGfx/Font/OpenType/Font.cpp
@@ -271,9 +271,12 @@ ErrorOr<NonnullRefPtr<Font>> Font::try_load_from_offset(ReadonlyBytes buffer, u3
         return Error::from_string_literal("Font is missing Maxp");
     auto maxp = TRY(Maxp::from_slice(opt_maxp_slice.value()));
 
-    if (!opt_hmtx_slice.has_value())
-        return Error::from_string_literal("Font is missing Hmtx");
-    auto hmtx = TRY(Hmtx::from_slice(opt_hmtx_slice.value(), maxp.num_glyphs(), hhea.number_of_h_metrics()));
+    Optional<Hmtx> hmtx;
+    if (!(options.skip_tables & Options::SkipTables::Hmtx)) {
+        if (!opt_hmtx_slice.has_value())
+            return Error::from_string_literal("Font is missing Hmtx");
+        hmtx = TRY(Hmtx::from_slice(opt_hmtx_slice.value(), maxp.num_glyphs(), hhea.number_of_h_metrics()));
+    }
 
     NonnullOwnPtr<CharCodeToGlyphIndex> cmap = options.external_cmap ? options.external_cmap.release_nonnull() : TRY(CmapCharCodeToGlyphIndex::from_slice(opt_cmap_slice.value()));
 
@@ -412,14 +415,14 @@ Gfx::ScaledGlyphMetrics Font::glyph_metrics(u32 glyph_id, float x_scale, float y
         return embedded_bitmap_metrics.release_value();
     }
 
-    if (!m_loca.has_value() || !m_glyf.has_value()) {
+    if (!m_loca.has_value() || !m_glyf.has_value() || !m_hmtx.has_value()) {
         return Gfx::ScaledGlyphMetrics {};
     }
 
     if (glyph_id >= glyph_count()) {
         glyph_id = 0;
     }
-    auto horizontal_metrics = m_hmtx.get_glyph_horizontal_metrics(glyph_id);
+    auto horizontal_metrics = m_hmtx->get_glyph_horizontal_metrics(glyph_id);
     auto glyph_offset = m_loca->get_glyph_offset(glyph_id);
     auto glyph = m_glyf->glyph(glyph_offset);
     return Gfx::ScaledGlyphMetrics {
diff --git a/Userland/Libraries/LibGfx/Font/OpenType/Font.h b/Userland/Libraries/LibGfx/Font/OpenType/Font.h
index 16b5c61ba5..823d01b342 100644
--- a/Userland/Libraries/LibGfx/Font/OpenType/Font.h
+++ b/Userland/Libraries/LibGfx/Font/OpenType/Font.h
@@ -34,6 +34,9 @@ struct FontOptions {
     enum SkipTables {
         // If set, do not try to read the 'name' table. family() and variant() will return empty strings.
         Name = 1 << 0,
+
+        // If set, do not try to read the 'hmtx' table. This will make glyph_metrics() return 0 for everyting and is_fixed_width() return true.
+        Hmtx = 1 << 1,
     };
     u32 skip_tables { 0 };
 };
@@ -94,7 +97,7 @@ private:
         Optional<Name>&& name,
         Hhea&& hhea,
         Maxp&& maxp,
-        Hmtx&& hmtx,
+        Optional<Hmtx>&& hmtx,
         NonnullOwnPtr<CharCodeToGlyphIndex> cmap,
         Optional<Loca>&& loca,
         Optional<Glyf>&& glyf,
@@ -130,7 +133,7 @@ private:
     Optional<Name> m_name;
     Hhea m_hhea;
     Maxp m_maxp;
-    Hmtx m_hmtx;
+    Optional<Hmtx> m_hmtx;
     Optional<Loca> m_loca;
     Optional<Glyf> m_glyf;
     NonnullOwnPtr<CharCodeToGlyphIndex> m_cmap;
diff --git a/Userland/Libraries/LibPDF/Fonts/PDFFont.h b/Userland/Libraries/LibPDF/Fonts/PDFFont.h
index cc3b33bac0..8688bfdb18 100644
--- a/Userland/Libraries/LibPDF/Fonts/PDFFont.h
+++ b/Userland/Libraries/LibPDF/Fonts/PDFFont.h
@@ -19,7 +19,7 @@ class Renderer;
 
 // PDF files don't need most of the data in OpenType fonts, and even contain invalid data for
 // these tables in some cases. Skip reading these tables.
-constexpr u32 pdf_skipped_opentype_tables = OpenType::FontOptions::SkipTables::Name;
+constexpr u32 pdf_skipped_opentype_tables = OpenType::FontOptions::SkipTables::Name | OpenType::FontOptions::SkipTables::Hmtx;
 
 class PDFFont : public RefCounted<PDFFont> {
 public: