PDFBOX-3293 truetype hinting

[pcorless]

PDFBOX-3293

• cuts in font hinting into FontBox and PDFBox
• combined my background on this issue with help from Claude to get some of the finer points worked out
• some more cleanup is needed but the basic structure is in place.

[THausherr]

Thank you, very interesting. From having a quick look I didn't see any breaking changes. I will set up some testing to see how the rendering is going.

You don't need to include the liberation font, it's already there, and is used in existing tests:

getResourceAsStream("/org/apache/pdfbox/resources/ttf/LiberationSans-Regular.ttf"));

[pcorless]

I'll get that font path touched up.

[THausherr]

I did a fast (human) visual compare by scrolling through the diff files. Besides the one chinese file that the issue is about, many were slightly different but the few where I had a closer look didn't seem to have any problem, one was slightly better, the horizontal bar in an "e" was more in the middle than without hinting. The rendering with hinting was faster (which is surprising, we'll see if this keeps happening) One difference has less quality:
mozilla/pdf.js#1732

file: PDFJS-1732.pdf

new rendering at 96 dpi (from the third notes line):

old rendering:

another file also caught my attention:
genko_oc_shiryo1.pdf
the new rendering looks partly bold.

new:

old:

Another small thing: IMHO the enabling of the hinting feature should be in PDFRenderer, not in TrueType. This can be done like with setSubsamplingAllowed(), and passed to PageDrawerParameters, then to PageDrawer.

[THausherr]

Copilot comment:

High-risk issues I found

1. GraphicsState.round() / round-state behavior may not match TrueType precisely

The round-state implementation is central to many opcodes. I’d specifically re-check:

ROUND_UP_TO_GRID
ROUND_DOWN_TO_GRID
ROUND_TO_DOUBLE_GRID
setSuperRound()

Why it’s risky:

These behaviors feed directly into MDRP, MIRP, MDAP, and MIAP.
A small rounding mismatch can shift stems by a pixel.

This one isn’t obviously broken, but it is critical enough that I’d want more proof against FreeType for edge values and negative distances.
2) TrueTypeInterpreter.installLogicalOps() likely has semantic issues

These lines stand out:

ODD / EVEN use Fixed.round(ctx.pop()) >> 6
AND / OR use Java truthiness semantics
NOT checks == 0

The likely issue is the exact definition of ODD/EVEN in TT bytecode. They depend on the rounded distance and parity at the 1/64 pixel level, and I’d want to verify the exact threshold behavior.

This may already be covered by tests, but I’d still flag it as worth a deeper look.
3) ROLL looks suspicious

Current implementation:

pops c, b, a
pushes b, c, a

That is not obviously the standard TrueType ROLL behavior unless the stack order matches a specific convention.

I’d double-check against the spec. ROLL is one of those stack-manipulation ops that can be silently wrong and then corrupt later execution.
4) MINDEX appears fragile

The implementation pops k, then pops k values, then pushes them back in a rearranged order.

That logic is a common place for off-by-one or order bugs. I would explicitly test:

k=1
k=2
larger values
interaction with the exact stack ordering expected by the spec

I don’t think this should block the PR on its own, but it deserves scrutiny.
5) TrueTypeInterpreter.jump() / control flow is a likely source of subtle bugs

This is one of the most important areas.

Concern points:

Jumps are relative to instructionStart()
instructionStart() is set just before opcode fetch
skipToElseOrEif() / skipToEif() manually skip over nested control flow and push operands

Potential issues:

whether relative offsets are measured from the right byte
whether ELSE/EIF skipping preserves correct nesting for all nested cases
whether JMPR, JROT, JROF interact correctly with instructionStart

This is exactly the sort of logic that can work for tested fonts but fail on odd control-flow constructs.
6) FDEF / IDEF / function body handling needs special scrutiny

The function-definition model stores:

the original program bytes
entry point offset

Then later replays from the stored program using callBody().

Things I’d verify carefully:

whether nested function definitions inside a function body are handled correctly
whether ENDF return semantics behave correctly when called recursively
whether ctx.setReturnFromFunction(false) is reset safely for nested calls
whether skipFunctionBody() properly handles push opcodes and nested IFs inside function bodies

This area is conceptually sound, but it’s one of the easiest places for “it works for Liberation Sans” but not for other fonts.
7) ExecutionContext.movePoint() backward-compatibility behavior is very heuristic-heavy

This is the riskiest logic in the whole patch.

The method now suppresses movement in certain cases when:

backward compatibility is enabled
IUP has run on both axes
y movement should freeze

That may be correct for the intended render mode, but it is a policy layer mixed into the VM’s primitive point-move operation.

Risks:

it can affect multiple opcodes unexpectedly
it changes behavior based on timing of IUP, which is hard to reason about
it may make the interpreter less spec-faithful than intended

If this is intended to emulate FreeType’s grayscale compatibility mode, I’d strongly suggest isolating it more explicitly so the core point move semantics stay clean.
8) hintingPpem() in PageDrawer looks plausible but needs proof for transforms

The code computes ppem from the transform’s vertical basis vector magnitude.

That’s clever, but because hinting is sensitive to device resolution and transform composition, I’d want:

explicit tests for rotation
explicit tests for anisotropic scaling
explicit tests for fractional transforms

You added some tests, which is good, but this is still a place where “looks right” can hide bugs.
Most concerning likely bug class

If I had to prioritize, I’d focus on:

control flow / stack ops
rounding semantics
function-definition replay
backward-compatibility movement suppression

Those are the areas most likely to produce subtle regressions.

[THausherr]

I can't comment on the copilot comments because I didn't look at the new code yet (e.g. because I have no knowledge whatsoever about hinting, the only thing I know is that it adjusts the points to a grid). I mostly looked at changes in the existing code and it looked like the new feature peacefully coexist, which is nice.

[pcorless]

I'll start working through those copilot comments.

Performance is an interesting one. On my test suite I also see no penalty for having hinting enabled. The caching works really well for most fonts.

I referenced pdf.js on my initial research as I thought it would be a good reference. As I dug in though it turned out their hinting was coming from a native system library not from JavaScript.

The two sample files you posted both fall into the "tricky" fonts category. FreeType has identified a list of fonts that need to be handled as special case. Differently being they need the bytecode to assembled/scale the glpyhs on every size. FreeTypes license is pretty flexible so I suspect it can be tied back in but it will take some legal work to properly update the LICENSE and NOTICE files.

Pushed changes for your two comments.

[THausherr]

Thank you; please merge the latest changes from the trunk, and enable line 708 in PagePane.java, the call renderer.setHintingEnabled(...) this way you (and me) can see the difference in PDFDebugger.

[THausherr]

Re Freetype license, it is not mentioned on
https://www.apache.org/legal/resolved.html
so we may have to ask, but they accept BSD and the freetype license is said to be "BSD like".
Re license, we need an ICLA from you
https://www.apache.org/licenses/contributor-agreements.html

[pcorless]

I've pushed the hinting hookup in the debugger.

I think I was was wrong on the "tricky" fonts on those two samples, will continue to investigate. I have some notes on the issue and need to revisit them.

I'll get you those contributor agreements soon.