You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pdfbox.apache.org by "pcorless (via GitHub)" <gi...@apache.org> on 2026/09/16 02:50:05 UTC
[PR] PDFBOX-3293 truetype hinting [pdfbox]
pcorless opened a new pull request, #532:
URL: https://github.com/apache/pdfbox/pull/532
[PDFBOX-3293](https://issues.apache.org/jira/browse/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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5904376753
OK, I think I addressed that last round of review comment. Also touched up coverage in a few related areas.
The https://github.com/apache/pdfbox/pull/532/commits/b63f3f107d74aaf42fa4f6191043c72af31591d9 commit does some interesting probability work to guess the tricky fonts by how points are moved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5709043055
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.](https://github.com/pcorless/pdfbox/blob/PDFBOX-3293.truetype.hinting.submit/fontbox/src/main/java/org/apache/fontbox/ttf/GlyphHinter.java#L368) 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5988237365
I'm pretty happy with the implementation. My QA file set renders well, and I don't have any concerns about rendering quality.
I've spent the last few nights making sure the branch contains no code from FreeType or any other library. This is an unfortunate side effect of agent-assisted coding that I hadn't anticipated. Yesterday I pushed a change (7438ac05d6) that rewrites three functions which, in earlier commits, followed FreeType's code more closely than I would have liked. I'm happy to leave the branch history as it is, but when it's merged it's important that it's squash-merged, so the earlier versions of this code don't make it into trunk's history.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5904416434
I have not been able to find a solution to PDFJS-1732.pdf at low zoom numbers, its a side effect of the grid fitting. It does clear up as the zoom level increases. This one will have to be a known issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5717896875
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.
<img width="663" height="972" alt="grafik" src="https://github.com/user-attachments/assets/f4ba1918-1118-4ba2-af5e-718ff212ee75" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5755297403
Push changes to address the review comment for github and fix for genko_oc_shiryo1.pdf bolding. PDFJS-1732.pdf is a different beast, hinting shouldn't be applied on this one, still looking for a why.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5694803537
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:
https://github.com/mozilla/pdf.js/issues/1732
file: [PDFJS-1732.pdf](https://github.com/user-attachments/files/32279279/PDFJS-1732.pdf)
new rendering at 96 dpi (third line):
<img width="254" height="44" alt="grafik" src="https://github.com/user-attachments/assets/1edbc029-75f8-4541-8cba-4e59636f85d1" />
old rendering:
<img width="252" height="49" alt="grafik" src="https://github.com/user-attachments/assets/fb8a628c-5c47-4348-8fac-bf704b37e284" />
another file also caught my attention:
[genko_oc_shiryo1.pdf](https://github.com/user-attachments/files/32279678/genko_oc_shiryo1.pdf)
the new rendering looks partly bold.
Another small thing: IMHO the setting should be in PDFRenderer, not in TrueType. This can be done like `setSubsamplingAllowed`, and passed to `PageDrawerParameters`, then to `PageDrawer`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5724695194
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5695480713
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5968993689
Thank you, this fixes the problems I mentioned. Is there anything you still want to do before this gets committed?
Do I understand correctly that there is no freetype code included in the PR, but that you've been using freetype output to verify your own logic, i.e. there are no open copyright questions at this time?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5694862824
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5846165822
Copilot writes this:
fontbox/src/main/java/org/apache/fontbox/ttf/Fixed.java:83-85 — ceil() is incorrect for negative non-integral values. The implementation (value + 63) & ~63 only behaves like a ceiling for some negative inputs. For example, ceil(-65) returns -128, but the mathematical ceiling in F26Dot6 should be -64. Since the interpreter exposes this through the TrueType CEILING opcode, fonts using CEILING with negative distances will receive a value one pixel too small, potentially shifting hinted points and causing incorrect outlines. Please implement ceiling explicitly for signed values and add negative-value tests, e.g. ceil(-1) == 0, ceil(-63) == 0, ceil(-65) == -64, and ceil(-128) == -128.
I have no idea if this is realistic, i.e. if such values can occur (I tried with the regression tests and found nothing). Also this part of the code was there before so I wonder why copilot didn't complain until now.
Btw fontbox has not only some more ttf fonts in the repository, it also loads some ttf fonts during the tests. There's a dejavu font you also did, plus Keyboard.ttf, NotoEmoji-Regular.ttf, NotoMono-Regular.ttf, ipag.ttf. They are in the directory target/fonts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5890728284
Thank you; please change GlyphHinter line 232 from " catch (IOException | RuntimeException e)" to "catch (IOException | HintingException e)", we want to know the hard way if something really nasty happens.
I have looked at the test coverage, there are parts of TrueTypeInterpreter that aren't covered so I ran my visual tests to see if any of the missing parts were hit. Only two were, flipRange and NPUSHW, and only with microsoft fonts, e.g. with this code:
```
@Test
void testArial() throws IOException
{
TrueTypeFont ttf = new TTFParser().parse(new RandomAccessReadBufferedFile("c:/windows/fonts/arial.ttf"));
ttf.getHintedPath(81, 13); // flipRange + NPUSHW
}
```
So we can't really test this on the CI, but locally with people who have windows (me). Could you add some code like this that checks that the result is what is expected?
```
@Test
void testArial() throws IOException
{
File file = new File("c:/windows/fonts/arial.ttf");
assumeTrue(file.exists());
TrueTypeFont ttf = new TTFParser().parse(new RandomAccessReadBufferedFile(file));
GeneralPath hintedPath = ttf.getHintedPath(81, 13); // hits flipRange + NPUSHW
// .... todo
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5789147429
OK thanks and I'll start taking a look at those comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "asf-gitbox-commits (via GitHub)" <gi...@apache.org>.
asf-gitbox-commits closed pull request #532: PDFBOX-3293 truetype hinting
URL: https://github.com/apache/pdfbox/pull/532
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5718087724
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5965798764
Added more protection against "bold" glyphs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5779082294
`not derivatives of FreeType (see `hinting_plan.md` "Oracle licensing").`
Is this referring to a file in the FreeType distribution?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5789077472
I've touched up that wording. FreeType uses "oracle" quite a bit when comparing to a golden set. I've renamed it "reference" to make it more clear. I've also pulled out references to my local notes I used to build the plan for this.
I've been testing withe the sneaky changes on/off and I think it will be worth while figuring out how to handle the licensing as it's a real value add against my test set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5917151010
Thanks, I think the sample file looks better than before the day you wrote about it, before the change it had some "thick" areas that looked unbalanced.
However here's a problem:
<img width="252" height="181" alt="grafik" src="https://github.com/user-attachments/assets/7b1ec4b6-7448-4bc7-9a0b-984132de20a9" />
This is from https://issues.apache.org/jira/browse/PDFBOX-4081
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5789110372
Please merge the latest changes from the trunk, I have removed the trailing spaces so that the actual change is better visible.
Here's the latest copilot comments:
Blocking review comments
fontbox/src/main/java/org/apache/fontbox/ttf/BytecodeStream.java, around nextByte() and nextWord() — setEnd() establishes a function-body limit, and seek() honors that limit, but both read methods only check code.length. Consequently, a PUSHB/PUSHW operand at the end of a function body can consume bytes beyond the matching ENDF, including instructions from the surrounding program. This defeats the function-boundary protection and can produce incorrect execution or allow malformed font bytecode to escape its function body. Both methods should validate against limit rather than code.length, and nextWord() should verify that both bytes remain within the limit.
BytecodeStream.java
@@
public int nextByte()
{
- if (ip >= code.length)
+ if (ip >= limit)
{
Other important findings
fontbox/src/main/java/org/apache/fontbox/ttf/TrueTypeInterpreter.java, jump/function handling — Add an explicit regression test for a PUSHW or PUSHB whose operand bytes straddle the function limit. The current tests cover jumps out of a function body, but not operand reads crossing the limit, so they would not catch the defect above.
TrueTypeInterpreterTest.java
@@
+@Test
+void testFunctionBodyCannotReadOperandsPastEndf()
+{
+ // The PUSHW is positioned so its second operand would be ENDF or code after ENDF.
+ // The interpreter must reject the read rather than consume bytes outside the body.
fontbox/src/test/resources/ttf/hinting/generate_golden.py and README.md — The documentation describes inconsistent FreeType targets: the generator uses FT_LOAD_TARGET_NORMAL, while the README describes monochrome/full-pixel tracing in places and the tests claim grayscale/v40 behavior. The golden-test contract should state one exact FreeType load target and use the same target in all scripts and documentation; otherwise regenerated reference files may silently validate a different interpreter mode.
fontbox/src/main/java/org/apache/fontbox/ttf/GlyphHinter.java — The implementation silently returns null for all composites when the component assembly or hinting path fails, causing the renderer to fall back to the raw outline. That is a reasonable resilience policy, but the tests should distinguish “unsupported composite hinting” from “successful composite hinting” and assert that representative composites actually receive instructions and produce the intended hinted result.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5924778189
Added more refinement to the full control logic
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5691580365
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"));
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5691599249
I'll get that font path touched up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] PDFBOX-3293 truetype hinting [pdfbox]
Posted by "pcorless (via GitHub)" <gi...@apache.org>.
pcorless commented on PR #532:
URL: https://github.com/apache/pdfbox/pull/532#issuecomment-5883764204
I've been testing with sample.pdf from the PDFBOX-3293 ticket and it's a good example of "tricky" vs. "No tricky". Render fine when enabled and skips hinting without it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org