On 19.03.2005 02:09:46 Renaud Richardet wrote:
> Here's my work in progress for the renderers (AWT, Print, PNG and
> TIFF). Could you please give it a quick look at my patch [1], thanks.

Before I start with my comments, let me tell you that I'm happy to see
that you're getting the hang of it. I'm looking forward to applying a
revised patch adressing the issues outlined below. Thanks a lot for
helping out!

> There's a few points I would like to discuss.
> 
> I splitted the AWTRenderer from the Java2DRenderer and did some clean
> up. I also created the PrintRenderer, PNGRenderer and TIFFRenderer.
> Please see the class diagram at the wiki FopAndJava2D. I also
> documented my work there.

Uhm, there's a discrepancy: You have a png and tiff subpackage while I
would have preferred a bitmap subpackage as shown on the Wiki page.

Furthermore, I'd remove the JPSPrintRenderer as it might be cause for
confusion with the PrintRenderer. It's of not much use anyway. I'd
rather create examples in examples/embedding that show how to use the
PrintRenderer with JPS both with normal printers and StreamPrintServices.
I'd also rename Java2DPrintRenderer to PrintRenderer even though there's
a class with the same name one package higher up. I think that base
class should rather be renamed to something less misleading. But that'll
take some opinion-gathering first.

> I end up with a lazy rendering: Java2DRenderer only stores Viewports
> and no Images. No actual rendering is done at this point. Page images
> are actually rendered (with getPageImage()) when the concrete
> renderers need them (that is: when the user wants to see a page (AWT)
> or just before the page is encoded (PNG and TIFF) or printed. No
> BufferedImage is stored. This way, the memory print should be reduced.
> Is this approach OK? Could there be some thread issues?

Yes, it's ok, although for me it's not always clear that you can always
tell which approach takes more memory, especially if you're able to
release FO and area tree memory. It may even be necessary for bigger
documents to temporarily serialize the area tree.

I don't see any threading issues as (and when) the renderers all run in
the main thread.

> Renderer registration:
> I'm unsure if I've registered the 2 new Renderers (TIFF and PNG)
> correctly in the front-end. Could you give it a look, especially
> RendererFactory.createFOEventHandler(), thanks.

Looks ok.

> The PNG-Renderer outputs 1 picture per page (right?), so we end up
> with several files.
> My pragmatic approach is to let the user specify the first file name
> on the command line (eg. "image1.png"). FOP then creates the next
> images using the same directory and name pattern ("image2.png", ASO).
> For this, I had to register the outfile in FOUserAgent.
> We could offer more configuration possibilities, but I think it's
> sufficient ATM. I don't feel like changing the front end, which looks
> very clean and robust.

I agree it's ok for now. It can always be improved as need arises.

> For PNG, TIFF and Print, the quality of the output is _very_ poor. Am
> I using the right image type in
> Java2DRenderer.getPageImage(PageViewport pageViewport) ?
> Oleg used a TYPE_BYTE_BINARY, which seems to be the only type which
> works with the TIFFEncoder from Batik. See
> TIFFRenderer$LazyPageImagesIterator.next(). But the quality is poorer.
> Any hints?

Yes. Simply increase the bitmap size. Use
FOUserAgent.getPixelUnitToMillimeter() to calculate an enhanced
scaleFactor in Java2DRenderer.getPageImage() to support bigger
resolutions. Default resolution is 72dpi which explains the poor quality.
You should probably support an additional command line parameter that
enables the user to specify the resolution for the generated image.
Compare with [1]

[1] http://barcode4j.krysalis.org/cli.html

> AWTRenderer:
> I had to modify PageViewport.isResolved(), so that the flag "done" in
> RenderPageModel.addPage() gets a chance to be set to true. The Map
> unresolvedID is sometimes empty, but not set to null.
> This way, the user can see the first page while the layout engine is
> still rendering the other pages in background. Please let me know if
> there's a better way to define isResolved().

I'd have to invest more time to answer that. I'll see what I can do.
Maybe someone else has some input here.

> PrintRenderer works somehow (with awt.PrintJob). But what I would
> really like to do is passing the output of the PSRenderer to JPS
> (o.a.f.render.print.JPSPrintRenderer), but that doesn't seem to work
> on windowsXP. I'll check that on linux.

It works but only if you're printing to a PostScript-enabled printer. If
you have a PCL-only printer it doesn't work because Windows doesn't have
a PostScript interpreter to convert the PostScript to PCL. PrintRenderer
does exactly what it's supposed to. It has to provide a Pageable and
Printable interface and that's it. If someone want to pipe the PS output
from the PSRenderer through to a PostScript printer, he's welcome to do
so but this is not FOP's job. BTW in this case the PS file is really
just piped through 1:1 and that's exactly why the approach only works on
PostScript-enabled printers.

> I put some more ant tasks on build.xml for the examples tiff and png.
> the target examples-png is throwing a build.xml:1080:
> java.lang.NullPointerException, and I don't understand why??

These examples should go into the build.xml file in the examples/fo
directory. This shouldn't be in the root build.xml

Run "ant -d examples-png" and you'll get a stack trace that'll point you
to the problem.

> Jeremias (and anybody else), if some of my work collides with yours,
> then just let me know.

It doesn't appear to.

> And please put this mail on P5 and concentrate
> on the Page Breaking and XMLGraphics-commons instead ;) I won't work
> on FOP until mid next week anyway.
> 
> Regards, 
> Renaud
> 
> [1] http://issues.apache.org/bugzilla/show_bug.cgi?id=33760

One more thing: Why did you include the codecs in the patch? Can't you
just use the ones in batik.jar? It worked fine here. If you have to make
any modifications to the codec please create an additional patch that
will be sent to the Batik team.

Thanks for enduring my comments and keep it up!

Jeremias Maerki

Reply via email to