search

pcorless / icepdf

PDF Rendering and Viewing API in Java
Apache License 2.0
81 stars 21 forks source link

print method seems to be accidentelly semi dropped from the api #356

Closed tg-freigmbh closed 5 months ago

tg-freigmbh commented 5 months ago

Hello,

Our Project needs org.icepdf.ri.common.print.PrintHelperImpl#print(java.awt.Graphics, java.awt.print.PageFormat, int) In the current version of icepdf it seems like this method got somewhat hidden, PrintHelperImpl does implement Printable, but PrintHelper does not not implement Printable... So it works when you instantiate the PrintHelperImpl directly, but you cant call print(java.awt.Graphics, java.awt.print.PageFormat, int) if you are using the PrintHelperFactoryImpl#createPrintHelper. Not sure if thats intended, I was fairly confused by this. Maybe make PrintHelper implement Printable ?

pcorless commented 5 months ago

As work around you could cast PrintHelperFactoryImpl#createPrintHelper to (PrintHelperImpl). But to your point the print(java.awt.Graphics, java.awt.print.PageFormat, int) probably should be defined as abstract in the base class or the base class implements Printable as you suggested. I'll have to think about this a bit, at the moment it's not clear to me if one approach is better then the other.

Can you provide any more detail as to why you are calling print(java.awt.Graphics, java.awt.print.PageFormat, int) directly, I'm curious as this usually done by the DocPrintJob ?

tg-freigmbh commented 5 months ago

If I understand our spaghetti correctly: In "our" Printjob we put multiple printable things together (Plain-Text,Html, Graphics, and 1 or more pdfs . This gets done by passing the printgraphics around to each module/by using the Printable interface. Using DocPrintJob seems like the better solution if all you want to do is printout the pdf document. However In our case the pdf is only part of a bigger printout.

pcorless commented 5 months ago

Thanks for the information. I think it would be best to have PrintHelper extend Printable so that you have a proper instance. I'll ready a PR for you to review.

pcorless commented 5 months ago

Merged fix