From 007d7cdd5382241b45b01e505924aaab217e981a Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis@chromium.org>
Date: Sun, 15 Oct 2023 09:44:40 -0400
Subject: [PATCH] LibPDF: Fix sign (and fixed point) in glyph decoding opcode
 24

Two bugs:

1. We decoded a u32, not an i32 as the spec wants
2. (minor) Our fixed-point divisor was off by one

Fixes text rendering in Bakke2010a.pdf in pdffiles, and rendering of
other fonts with negative width adjustments from optcode 255.
That PDF was produced by "Apple pstopdf" and uses font SFBX1200,
which is apparently a variant of Computer Modern. So maybe this
helps with lots of PDFs produced from TeX files, but I haven't
checked that.
---
 .../LibPDF/Fonts/Type1FontProgram.cpp         | 22 +++++++++++--------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/Userland/Libraries/LibPDF/Fonts/Type1FontProgram.cpp b/Userland/Libraries/LibPDF/Fonts/Type1FontProgram.cpp
index 0ee99a015e..3bf879e552 100644
--- a/Userland/Libraries/LibPDF/Fonts/Type1FontProgram.cpp
+++ b/Userland/Libraries/LibPDF/Fonts/Type1FontProgram.cpp
@@ -135,6 +135,9 @@ void Type1FontProgram::consolidate_glyphs()
 
 PDFErrorOr<Type1FontProgram::Glyph> Type1FontProgram::parse_glyph(ReadonlyBytes const& data, Vector<ByteBuffer> const& subroutines, GlyphParserState& state, bool is_type2)
 {
+    // Type 1 Font Format: https://adobe-type-tools.github.io/font-tech-notes/pdfs/T1_SPEC.pdf (Chapter 6: CharStrings dictionary)
+    // Type 2 Charstring Format: https://adobe-type-tools.github.io/font-tech-notes/pdfs/5177.Type2.pdf
+
     auto push = [&](float value) -> PDFErrorOr<void> {
         if (state.sp >= state.stack.size())
             return error("Operand stack overflow");
@@ -250,16 +253,17 @@ PDFErrorOr<Type1FontProgram::Glyph> Type1FontProgram::parse_glyph(ReadonlyBytes
         int v = data[i];
         if (v == 255) {
             TRY(require(4));
-            int a = data[++i];
-            int b = data[++i];
-            int c = data[++i];
-            int d = data[++i];
+            // Both Type 1 and Type 2 spec:
+            // "If the charstring byte contains the value 255, the next four bytes indicate a two’s complement signed number.
+            //  The first of these four bytes contains the highest order bits [...]"
+            i32 a = static_cast<i32>((data[i + 1] << 24) | (data[i + 2] << 16) | (data[i + 3] << 8) | data[i + 4]);
+            i += 4;
             if (is_type2) {
-                auto integer = float((a << 8) | b);
-                auto fraction = float((c << 8) | d) / AK::NumericLimits<u16>::max();
-                TRY(push(integer + fraction));
-            } else
-                TRY(push((a << 24) + (b << 16) + (c << 8) + d));
+                // Just in the Type 2 spec: "This number is interpreted as a Fixed; that is, a signed number with 16 bits of fraction."
+                TRY(push(a / (float)0x1'0000));
+            } else {
+                TRY(push(a));
+            }
         } else if (v >= 251) {
             TRY(require(1));
             auto w = data[++i];