Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for Apache PDFBox TilingPaint. #25

Merged
merged 2 commits into from May 29, 2020

Conversation

p1xel
Copy link
Contributor

@p1xel p1xel commented May 27, 2020

We encountered an issue where a TilingPaint object may be passed to the applyPaint method of PdfBoxGraphics2DPaintApplier, but is not currently handled.

Our solution reads the private TexturePaint field of the TilingPaint object and passes it to the existing applyTexturePaint method.

Copy link
Owner

@rototor rototor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PullRequest.

Please provide a sample PDF, that triggers this code path. And a line in PdfRerenderTest.rerenderPDF() to rerender this sample. I will then merge this and try to get a patch upstream to PDFBox to correctly access the TilingPaint internals.

catch (NoSuchFieldException ignored)
{
}
throw new NullPointerException("Field " + fieldName + " not found!");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not rethrowing the underlying NoSuchFieldException seems strange. The method getPropertyValue() you likely copied here did loop, and ignored that error because of that. When accessing a private field you in theory also need to loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it should throw and I've changed it back to use a loop as you suggest. The theory was that a private field is only accessible from the class that contains it, but we've already broken encapsulation.

private void applyTilingPaint(Paint paint, PaintApplierState state) throws IOException
{
TexturePaint texturePaint = getPrivateFieldValue(paint, "paint");
applyTexturePaint(texturePaint, state);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casting this to TexturePaint is likely calling for trouble. Why did you not simply call applyPaint() here with this paint?
Also you ignore the patternMatrix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - that is a better way to handle it. Also added some exception handling to catch any issues with private field access.

I think the patternMatrix should be accounted for already. In the TilingPaint class, the TexturePaint field is initialised from getImage(...) which applies the anchorRect and patternMatrix. The source and converted PDFs look the same.

try
{
Field f = c.getDeclaredField(fieldName);
f.setAccessible(true);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wont work in JDK9+ :( There are hacks (Unsafe&Friends) to workaround that, but I am not sure that I want to import them here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully we can get the patch for PDFBox to make this unnecessary.

@p1xel
Copy link
Contributor Author

p1xel commented May 29, 2020

Thanks for the PullRequest.

Please provide a sample PDF, that triggers this code path. And a line in PdfRerenderTest.rerenderPDF() to rerender this sample. I will then merge this and try to get a patch upstream to PDFBox to correctly access the TilingPaint internals.

Unfortunately, we've only seen this case once so far, and the source is proprietary, so I'm not able to share it. I'm also not sure how to generate a PDF with this type of content at the moment, but will try to get a sample if possible.

@p1xel p1xel requested a review from rototor May 29, 2020 07:15
@rototor
Copy link
Owner

rototor commented May 29, 2020

Ok, I'll merge that now and

  • try to build / find some sample PDF with a tiled pattern
  • add JDK9 private field access workaround/hack
  • try to get some kind of interface / accessibility to the private property in PDFBox upstream.

@rototor rototor merged commit 60a90c9 into rototor:master May 29, 2020
rototor added a commit that referenced this pull request May 29, 2020
found that the matrix has to be applied somehow.
@rototor
Copy link
Owner

rototor commented May 29, 2020

I found a test case, as I already use the TilingPattern for some special cases when I need some pattern/gradient as a stroke fill. E.g. when using to draw a stroke. And in my test case the matrix seems to be needed. Thats something I still need to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants