LadybirdBrowser/ladybird
1
1
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2024-09-21 02:08:12 +03:00
Code Issues Packages Projects Releases Wiki Activity

LibPDF: Move type1 subr 0 handling into othersubr handler

Browse Source 
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 0:
3 0 callothersubr pop pop setcurrentpoint return
"""

othersubr handler 0 gets three arguments:
* The flex height (the distance after which the bezier splines
  are replaced with just straight lines)
* The current position after the flex

It pushes that position on the postscript stack, where predefined subr
handler number 0 then pops it from. It then passes it to
setcurrentpoint.

In theory, we now correctly do that setcurrentpoint call, which we
previously weren't.

In practice, that setcurrentpoint call always receives the last point of
the flex -- and our path api apparently gets confused when move_to() is
called on it when the current point is already at that same location.

So tweak the SetCurrentPoint handler to not set the current point on
the path if it's already the path's current point, with a FIXME to
figure out what exactly is happening in Gfx::Path.

No big behavior change if flex is used, but this is 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.)
This commit is contained in:
Nico Weber 2023-10-31 12:42:46 -04:00 committed by Tim Flynn
parent 0bb8249780
commit 3e707efdfa
Notes: sideshowbarker 2024-07-17 07:09:53 +09:00 
Author: https://github.com/nico
Commit: https://github.com/SerenityOS/serenity/commit/3e707efdfa
Pull-request: https://github.com/SerenityOS/serenity/pull/21484
1 changed files with 52 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
Userland/Libraries/LibPDF/Fonts/Type1FontProgram.cpp
View File

@ -397,32 +397,6 @@ PDFErrorOr<Type1FontProgram::Glyph> Type1FontProgram::parse_glyph(ReadonlyBytes
if (static_cast<size_t>(subr_number) >= subroutines.size())
return error("Subroutine index out of range");
if (!is_type2) {
// 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) {
if (state.flex_index != 14)
break;
auto& flex = state.flex_sequence;
path.cubic_bezier_curve_to(
{ flex[2], flex[3] },
{ flex[4], flex[5] },
{ flex[6], flex[7] });
path.cubic_bezier_curve_to(
{ flex[8], flex[9] },
{ flex[10], flex[11] },
{ flex[12], flex[13] });
state.flex_feature = false;
state.sp = 0;
break;
}
}
auto const& subr = subroutines[subr_number];
if (subr.is_empty())
return error("Empty subroutine");
@ -471,7 +445,19 @@ PDFErrorOr<Type1FontProgram::Glyph> Type1FontProgram::parse_glyph(ReadonlyBytes
// 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."
// [...]
// Type 1 spec, 8.4 First Four Subrs Entries:
// "Subrs entry number 0: 3 0 callothersubr pop pop setcurrentpoint return
// Subrs entry number 1: 0 1 callothersubr return
// Subrs entry number 2: 0 2 callothersubr return
// Subrs entry number 3: return
// [...]
// Subrs entry 0 passes the final three arguments in the Flex mechanism into OtherSubrs entry 0.
// That procedure puts the new current point on top of the Post- Script interpreter operand stack.
// Subrs entry 0 then transfers these two coordinate values to the Type 1 BuildChar operand stack
// with two pop commands and makes them the current point coordinates with a setcurrentpoint command."
enum OtherSubrCommand {
EndFlex = 0,
StartFlex = 1,
AddFlexPoint = 2,
};
@ -480,6 +466,35 @@ PDFErrorOr<Type1FontProgram::Glyph> Type1FontProgram::parse_glyph(ReadonlyBytes
auto n = static_cast<int>(pop());
switch ((OtherSubrCommand)othersubr_number) {
case EndFlex: {
if (n != 3)
return error("Unexpected argument code for othersubr 0");
auto y = pop();
auto x = pop();
[[maybe_unused]] auto flex_height = pop();
state.postscript_stack[state.postscript_sp++] = y;
state.postscript_stack[state.postscript_sp++] = x;
if (state.flex_index != 14)
break;
auto& flex = state.flex_sequence;
path.cubic_bezier_curve_to(
{ flex[2], flex[3] },
{ flex[4], flex[5] },
{ flex[6], flex[7] });
path.cubic_bezier_curve_to(
{ flex[8], flex[9] },
{ flex[10], flex[11] },
{ flex[12], flex[13] });
state.flex_feature = false;
state.sp = 0;
break;
}
case StartFlex:
if (n != 0)
return error("Unexpected argument code for othersubr 1");
@ -510,8 +525,17 @@ PDFErrorOr<Type1FontProgram::Glyph> Type1FontProgram::parse_glyph(ReadonlyBytes
auto y = pop();
auto x = pop();
state.point = { x, y };
path.move_to(state.point);
// FIXME: Gfx::Path behaves weirdly if a cubic_bezier_curve_to(a, b, c)
// is followed by move(c). Figure out why, fix in Gfx::Path, then
// remove this check here.
// Run `Build/lagom/bin/pdf --render out.png Tests/LibPDF/type1.pdf`
// as test -- if the output looks good, then all's good. At the moment,
// the output looks broken without the if here.
Gfx::FloatPoint new_point { x, y };
if (state.point != new_point) {
state.point = new_point;
path.move_to(state.point);
}
state.sp = 0;
break;
}