Skip to content

Commit bbdbdab

Browse files
committed
LibPDF: Make setcurrentpoint in type 1 fonts not create a path entry
T1_SPEC.pdf describes `setcurrentpoint` like so: > sets the current point in the Type 1 font format BuildChar to > (x, y) in absolute character space coordinates without performing > a charstring moveto command. This establishes the current point > for a subsequent relative path building command. The > setcurrentpoint command is used only in conjunction with > results from OtherSubrs procedures. I think that's supposed to mean that it should only update the point in the path, so that later relative path commands use that as origin. `setcurrentpoint` is only used in type 1 fonts, not in type 2 charstrings. In type 1 fonts, it's used to implement the "flex" feature (type 2 has dedicated `*flex*` commands for that instead). "8.4 First Four Subrs Entries" says that subr 0 should always contain 3 0 callothersubr pop pop setcurrentpoint return and "8.3 Flex" in T1_SPEC.pdf says that ssubr 0 should be called with the flex height and final point's x/y on the stack. To me, that section pretty strongly suggests that the final point passed to othersubr 0 is supposed to be identical to the final point passed to othersubr 2. The `setcurrentpoint` is a no-op for us when the final point is identical to the last othersubr 2 point, due to how our `move_to()` lambda works. For the fonts in #25944 it isn't identical. It's not clear to me which of the two points we should push in the othersubr 0 EndFlex handler. If we push `flex[12]` / `flex[13]`, then the `setcurrentpoint` is always a no-op. If we push what's on the postscript stack, for the font in #25944, it makes a very slight visual difference. Maybe you're supposed to use one version if the flex is drawn as bezier curves, and the other when it's drawn as a line? Or maybe the font is just buggy and the numbers are supposed to be equal. Appendix 3 has PostScript code for the first four OtherSubr routines, but I can't read PostScript well enough yet to understand if that pushes the final point passed on the stack when it's called, or the final point passed to subr 2 (which calls othersubr 2, which adds a relative flex point). Fixes #25944 and addresses the type 1 font issue discussed in #21484 in a more satisfying way.
1 parent c6295c2 commit bbdbdab

File tree

1 file changed

+2
-12
lines changed

1 file changed

+2
-12
lines changed

Userland/Libraries/LibPDF/Fonts/Type1FontProgram.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,7 @@ ErrorOr<Type1FontProgram::Glyph> Type1FontProgram::parse_glyph(ReadonlyBytes con
497497

498498
auto& flex = state.flex_sequence;
499499

500+
// FIXME: This should probably call the flex() lambda.
500501
path.cubic_bezier_curve_to(
501502
{ flex[2], flex[3] },
502503
{ flex[4], flex[5] },
@@ -539,18 +540,7 @@ ErrorOr<Type1FontProgram::Glyph> Type1FontProgram::parse_glyph(ReadonlyBytes con
539540
case SetCurrentPoint: {
540541
auto y = pop();
541542
auto x = pop();
542-
543-
// FIXME: Gfx::Path behaves weirdly if a cubic_bezier_curve_to(a, b, c)
544-
// is followed by move(c). Figure out why, fix in Gfx::Path, then
545-
// remove this check here.
546-
// Run `Build/lagom/bin/pdf --render out.png Tests/LibPDF/type1.pdf`
547-
// as test -- if the output looks good, then all's good. At the moment,
548-
// the output looks broken without the if here.
549-
Gfx::FloatPoint new_point { x, y };
550-
if (state.point != new_point) {
551-
state.point = new_point;
552-
path.move_to(state.point);
553-
}
543+
state.point = { x, y };
554544
state.sp = 0;
555545
break;
556546
}

0 commit comments

Comments
 (0)