From 0bb824978079307a36842aae8d57026498beb4c9 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 31 Oct 2023 08:57:25 -0400 Subject: [PATCH] LibPDF: Move type1 subr 1 and 2 handling into othersubr handler https://adobe-type-tools.github.io/font-tech-notes/pdfs/T1_SPEC.pdf, 8.4 First Four Subrs Entries: """If Flex or hint replacement is used in a Type 1 font program, the first four entries in the Subrs array in the Private dictionary must be assigned charstrings that correspond to the following code sequences. If neither Flex nor hint replacement is used in the font program, then this requirement is removed, and the first Subrs entry may be a normal charstring subroutine sequence. The first four Subrs entries contain: [...] Subrs entry number 1: 0 1 callothersubr return Subrs entry number 2: 0 2 callothersubr return """ So subr entry numbers 1 and 2 just call othersubr 1 and and 2, which means we can just move the handling code over. No behavior change if flex is used, but more correct if it isn't. (This only works because our `return` handler is empty, else we would have to make the callothersubr handler start a call frame.) --- .../LibPDF/Fonts/Type1FontProgram.cpp | 53 +++++++++++++------ 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/Userland/Libraries/LibPDF/Fonts/Type1FontProgram.cpp b/Userland/Libraries/LibPDF/Fonts/Type1FontProgram.cpp index b777a4a1d6..2c513b5adb 100644 --- a/Userland/Libraries/LibPDF/Fonts/Type1FontProgram.cpp +++ b/Userland/Libraries/LibPDF/Fonts/Type1FontProgram.cpp @@ -398,9 +398,9 @@ PDFErrorOr Type1FontProgram::parse_glyph(ReadonlyBytes return error("Subroutine index out of range"); if (!is_type2) { - // FIXME: Hardcoding subrs 0-2 here is incorrect, since some fonts don't use the flex feature. - // For the ones that do, subrs 0-2 have fixed contents that have callothersubr instructions. - // The right thing to do is to implement callothersubr for subrs 0-3 and remove the hardcoding here. + // FIXME: Hardcoding subr 0 here is incorrect, since some fonts don't use the flex feature. + // For the ones that do, subrs 0 has fixed contents that have callothersubr instructions. + // The right thing to do is to implement callothersubr for subrs 0 and remove the hardcoding here. // Subroutines 0-2 handle the flex feature. if (subr_number == 0) { @@ -422,16 +422,6 @@ PDFErrorOr Type1FontProgram::parse_glyph(ReadonlyBytes state.sp = 0; break; } - if (subr_number == 1) { - state.flex_feature = true; - state.flex_index = 0; - state.sp = 0; - break; - } - if (subr_number == 2) { - state.sp = 0; - break; - } } auto const& subr = subroutines[subr_number]; if (subr.is_empty()) @@ -474,10 +464,41 @@ PDFErrorOr Type1FontProgram::parse_glyph(ReadonlyBytes } case CallOtherSubr: { - [[maybe_unused]] auto othersubr_number = pop(); + // Type 1 spec, 8.3 Flex: + // "5. Insert at the beginning of the sequence the coordinate of the reference point relative to the starting point. + // There are now seven coordinate values (14 numbers) in the sequence. + // 6. Place a call to Subrs entry 1 at the beginning of this sequence of coordinates, and place an rmoveto command + // and a call to Subrs entry 2 after each of the seven coordinate pairs in the sequence. + // 7. Place the Flex height control parameter and the final coordinate expressed in absolute terms (in character space) + // followed by a call to Subrs entry 0 at the end." + enum OtherSubrCommand { + StartFlex = 1, + AddFlexPoint = 2, + }; + auto othersubr_number = (OtherSubrCommand)pop(); + auto n = static_cast(pop()); - for (int i = 0; i < n; ++i) - state.postscript_stack[state.postscript_sp++] = pop(); + + switch ((OtherSubrCommand)othersubr_number) { + case StartFlex: + if (n != 0) + return error("Unexpected argument code for othersubr 1"); + state.flex_feature = true; + state.flex_index = 0; + state.sp = 0; + break; + case AddFlexPoint: + if (n != 0) + return error("Unexpected argument code for othersubr 2"); + // We do this directly in move_to(). + state.sp = 0; + break; + default: + for (int i = 0; i < n; ++i) + state.postscript_stack[state.postscript_sp++] = pop(); + break; + } + break; }