Thanks for the hint. That made things easier. I've committed the
necessary changes but without the potential performance optimizations we
discussed.

On 07.12.2008 17:02:34 thomas.deweese wrote:
> Hi Jeremias,
> 
> Jeremias Maerki <[EMAIL PROTECTED]> wrote on 12/07/2008 09:04:57 AM:
> 
> > Ok, I've created a Bugzilla issue to track this issue in FOP:
> > https://issues.apache.org/bugzilla/show_bug.cgi?id=46360
> 
>   Ok, Thanks.
> 
> > A short test shows that I can't easily clone the document if it has a
> > document type. The following is a patch the would fix the exception.
> [...]
> > And I don't even know if the patch here is even plausible.
> 
>    I strongly suspect that it's intentional that the document will not
> create a DocumentType Node as this should be provided when the
> document is created.
> 
> > But I guess I'll need to work around that outside of Batik as we'd have
> > to wait for a Batik release before we can release a new FOP with the fix
> > applied.  So I'm planning to do this:
> 
>   I suggest using:
>         batik.dom.util.DOMUtilities.deepCloneDocument
>                 (Document doc, DOMImplementation impl);
> 
>   This has been in Batik for quite a while and has code to
> work around this exact problem.
> 
> > On 07.12.2008 14:44:53 thomas.deweese wrote:
> > > Hi Jeremias,
> > > 
> > > Jeremias Maerki <[EMAIL PROTECTED]> wrote on 12/07/2008 08:09:48 
> AM:
> > > 
> > > > Thanks a lot, Thomas. So the safest way would be to clone the 
> (cached) 
> > > SVG
> > > > document.
> > > 
> > >    Correct.  This is what we do when we cache documents - mostly 
> because 
> > > we
> > > do deal with dynamic documents.
> > > 
> > > > As an optimization step one could investigate if some sort of
> > > > caching the GVT tree is doable (see my other post just a few minutes 
> 
> > > ago).
> > > > I assume the second option would take quite a bit of effort 
> (including
> > > > benchmarks beforehand to determine the optimization potential).
> > > 
> > >    So the work would be knowing that you can reuse the GVT tree 
> because
> > > the destination format matches (so the 'added' bridges would match).
> > > The other potential issue would be the viewBox fitting (the affine for
> > > that lives in the 'CanvasGraphicsNode').  You would probably want to
> > > handle that outside of the GVT tree.
> > > 
> > >    As far as the boost this would give you, the actual construction
> > > of the GVT tree is pretty quick, but some of the stuff that is done
> > > the first time through (calculating BBoxes, text layout, etc) can be
> > > fairly time consuming.
> > > 
> > > > On 07.12.2008 13:53:12 thomas.deweese wrote:
> > > > > Hi Jeremias,
> > > > > 
> > > > > Jeremias Maerki <[EMAIL PROTECTED]> wrote on 12/07/2008 
> 06:53:04 
> > > AM:
> > > > > 
> > > > > > Is that a sure thing that the DOM is not modified? 
> > > > > 
> > > > >    It is 100% certain that you can't have one document with
> > > > > two attached GVT tree's - which I think you would need to do
> > > > > with your current approach.  The actual DOM infoset shouldn't
> > > > > be modified but internally we attach additional structures to
> > > > > handle the CSS & SVG DOMs.  Also some items (particularly in
> > > > > the SVG DOM) may be lazily constructed (so the two threads might
> > > > > collide when 'reading' those items).
> > > > > 
> > > > > > If yes, we'd have to ask the question whether attaching the CSS 
> > > engine 
> > > > > > to the DOM is the right thing to do. And is the CSS Engine as 
> such 
> > > not 
> > > > > > thread-safe or is only the setting of the CSS engine not thread 
> safe 
> > > 
> > > > > (method
> > > > > > BridgeContext.initializeDocument(Document))? 
> > > > > 
> > > > >    Once again no attempt was made to make the CSS Engine thread 
> safe, 
> > > it
> > > > > might be thread safe for some set of operations but I can't give 
> any 
> > > kind
> > > > > of guidelines.  Of course you must attach the CSS Engine otherwise 
> the 
> > > 
> > > > > CSS DOM (which we use to render the document) won't be available.
> > > > > 
> > > > >    In Batik it is 100% required that people access DOM/CSS/GVT 
> through 
> > > 
> > > > > one thread.
> > > > > 
> > > > > > Maybe an idea could be to attach the CSS engine to the thread 
> > > (thread 
> > > > > > local) instead of to the DOM if only one thread uses the CSS 
> engine 
> > > in 
> > > > > > a single rendering run and the CS engine is not thread-safe. 
> > > > > > Just brain-storming....
> > > > > 
> > > > >    The only thing that might work would be to attach the CSSEngine 
> 
> > > once
> > > > > and build the GVT tree once and keep that in the cache.  But even 
> with
> > > > > that the GVT tree may well not support multiple concurrent 
> renderings
> > > > > (although for a static document it should come close).
> > > > > 
> > > > > > On 07.12.2008 12:32:43 Peter Coppens wrote:
> > > > > > > As in another post mentioned - I have been debugging the code. 
> The 
> > > dom 
> > > > > is
> > > > > > > not modified. The dom caches a Parser object which is not 
> thread 
> > > safe.
> > > > > > > 
> > > > > > > 
> > > > > > > > From: Jeremias Maerki <[EMAIL PROTECTED]>
> > > > > > > > Reply-To: <[email protected]>
> > > > > > > > Date: Sun, 07 Dec 2008 12:17:48 +0100
> > > > > > > > To: <[email protected]>
> > > > > > > > Subject: Re: Batik exception when using fop with svg images 
> in 
> > > > > threaded
> > > > > > > > environment
> > > > > > > > 
> > > > > > > > Thanks for your input, Thomas!
> > > > > > > > 
> > > > > > > > On 06.12.2008 20:00:01 thomas.deweese wrote:
> > > > > > > >> Hi Jeremias,
> > > > > > > >> 
> > > > > > > >> Jeremias Maerki <[EMAIL PROTECTED]> wrote on 
> 12/05/2008 
> > > > > 03:55:42 AM:
> > > > > > > >> 
> > > > > > > >>> I can reproduce the issue with this test case. It really 
> only 
> > > > > happens in
> > > > > > > >>> the multi-threading case (running against Batik Trunk). As 
> 
> > > soon as 
> > > > > you
> > > > > > > >>> place break-points before the exception 
> > > (NumberFormatException) 
> > > > > can
> > > > > > > >>> happen, it won't. I'm not sure where to start looking for 
> the 
> > > > > problem.
> > > > > > > >>> Maybe we can do something in FOP to avoid this. Help/hints 
> 
> > > would 
> > > > > be
> > > > > > > >>> appreciated.
> > > > > > > >> 
> > > > > > > >>    I don't know how FOP loads and rasterizes SVG documents. 
>  If 
> > > 
> > > > > there
> > > > > > > >> is a global cache of Documents that have been read so the 
> same 
> > > > > Document
> > > > > > > >> Object ends up being used in both threads that would cause 
> this 
> > > 
> > > > > problem.
> > > > > > > > 
> > > > > > > > Hmm, I wonder why we've never had that before (to my 
> knowledge). 
> > > 
> > > > > After
> > > > > > > > all we have an image cache which caches and reuses the SVG 
> DOM 
> > > for 
> > > > > years.
> > > > > > > > Even FOP versions before 0.20.5 did that.
> > > > > > > > 
> > > > > > > > I mean I can understand that SVGs with scripts or animation 
> > > might 
> > > > > modify
> > > > > > > > the DOM but static SVGs with no script, no animation, no CSS 
> 
> > > parts?
> > > > > > > > 
> > > > > > > > Anyway, FOP does have (0.94 or earlier) an image cache or 
> uses 
> > > the 
> > > > > one
> > > > > > > > in the image loading framework from XML Graphics Commons 
> (since 
> > > FOP 
> > > > > 0.95).
> > > > > > > > In the case of SVG documents, the DOM is reused with the 
> > > intention 
> > > > > of
> > > > > > > > improving performance.
> > > > > > > > 
> > > > > > > >>    Checking the source it appears that 
> fop.image.ImageFactory 
> > > does 
> > > > > this.
> > > > > > > > 
> > > > > > > > You're looking at old source code (before 0.95). This has 
> been 
> > > > > replaced
> > > > > > > > by the XG Commons image loading framework.
> > > > > > > > http://xmlgraphics.apache.org/commons/image-loader.html
> > > > > > > > 
> > > > > > > >> There seem to be some options for sharing (or not) of 
> images 
> > > > > between
> > > > > > > >> contexts but it's not obvious to me how that works.
> > > > > > > > 
> > > > > > > > Images are cached with their original URI as the key. When 
> the 
> > > same 
> > > > > URI
> > > > > > > > is requested the cached image (with a reference to the SVG 
> DOM) 
> > > is
> > > > > > > > returned if it hasn't already been claimed by the garbage 
> > > collector.
> > > > > > > > Image subclasses indicate if they are cacheable or not:
> > > > > > > > https://svn.eu.apache.
> > > > > > org/viewvc/xmlgraphics/commons/trunk/src/java/org/apache
> > > > > > > > /xmlgraphics/image/loader/Image.java?view=markup
> > > > > > > > 
> > > > > > > >>    Another option would be to deep clone the Document the 
> > > second 
> > > > > time
> > > > > > > >> (deciding that it's the second time is left as an exercise 
> for 
> > > the
> > > > > > > >> user of Batik ;).
> > > > > > > > 
> > > > > > > > That depends. If you're saying that the DOM can actually be 
> > > > > manipulated,
> > > > > > > > even in case of a "static" SVG, then that might not always 
> be a 
> > > good
> > > > > > > > idea. To be on the safe side, I think I'll always have to 
> clone 
> > > the 
> > > > > SVG
> > > > > > > > document.
> > > > > > > > 
> > > > > > > > I hope the performance penalty for the cloning is not too 
> high.
> > > > > > > > 
> > > > > > > > Do you think it is feasible to modify Batik to treat the SVG 
> 
> > > > > document as
> > > > > > > > read-only? If yes, where would we have to dive in?
> > > > > > > > 
> > > > > > > >>> Peter, there's one problem in your test case: You reuse 
> the 
> > > > > FOUserAgent
> > > > > > > >>> for all documents. That's not how it's supposed to be. You 
> 
> > > have to
> > > > > > > >>> create a new FOUserAgent for each processing run. Too bad, 
> 
> > > that 
> > > > > doesn't
> > > > > > > >>> already fix the problem. ;-)
> > > > > > > >> 
> > > > > > > >>    This might let the 'context' stuff I mentioned above 
> kick 
> > > in.
> > > > > > > > 
> > > > > > > > No, the image cache is attached to the FopFactory, not the 
> > > > > FOUserAgent.
> > > > > > > > The old image cache used to create a hard reference on an 
> image 
> > > when 
> > > > > it
> > > > > > > > was preloaded during layout to block garbage collection. 
> That 
> > > hard
> > > > > > > > reference was released (leaving the soft reference in the 
> image 
> > > > > cache)
> > > > > > > > when the image was rendered. I didn't reimplement that 
> feature 
> > > to 
> > > > > lower
> > > > > > > > the memory requirements for images that have to be fully 
> loaded 
> > > at
> > > > > > > > "pre-loading time".
> > > > > > > > 
> > > > > > > >> 
> > > > > > > >>> 
> > > > > > > >>> On 04.12.2008 23:58:57 Peter Coppens wrote:
> > > > > > > >>>> 
> > > > > > > >>>> We have been able to reproduce with the following files
> > > > > > > >>>> 
> > > > > > > >>>> http://www.nabble.com/file/p20844449/TestPijltje.java 
> > > > > TestPijltje.java
> > > > > > > >> 
> > > > > > > >>>> http://www.nabble.com/file/p20844449/pols.xml pols.xml
> > > > > > > >>>> http://www.nabble.com/file/p20844449/pols.xslt pols.xslt
> > > > > > > >>>> http://www.nabble.com/file/p20844449/pijltje.svg 
> pijltje.svg
> > > > > > > >>>> 
> > > > > > > >>>> TestPijltje.java is the test program. It starts two 
> threads 
> > > each 
> > > > > of
> > > > > > > >> which
> > > > > > > >>>> will generate a pdf based on the pols.xslt stylesheet, 
> > > pols.xml 
> > > > > en
> > > > > > > >>>> pijltje.svg input file.
> > > > > > > >>>> 
> > > > > > > >>>> If changing the code to only start one thread it always 
> works 
> > > 
> > > > > fine.
> > > > > > > >> With two
> > > > > > > >>>> threads regular exceptions are thrown
> > > > > > > >>>> 
> > > > > > > >>>> Stack trace is always something like
> > > > > > > >>>> 
> > > > > > > >>>> Dec 4, 2008 11:54:33 PM org.apache.fop.svg.SVGUserAgent 
> > > > > displayError
> > > > > > > >>>> SEVERE: SVG 
> > > Errorfile:/Users/pc/Downloads/fop-0.95/pijltje.svg:
> > > > > > > >>>> The attribute "enable-background" represents an invalid 
> CSS 
> > > value
> > > > > > > >> ("new 0 0
> > > > > > > >>>> 47.403 26.361").
> > > > > > > >>>> Original message:
> > > > > > > >>>> Invalid number.
> > > > > > > >>>> org.w3c.dom.DOMException:
> > > > > > > >> file:/Users/pc/Downloads/fop-0.95/pijltje.svg:
> > > > > > > >>>> The attribute "enable-background" represents an invalid 
> CSS 
> > > value
> > > > > > > >> ("new 0 0
> > > > > > > >>>> 47.403 26.361").
> > > > > > > >>>> Original message:
> > > > > > > >>>> Invalid number.
> > > > > > > >>>>         at 
> > > > > > > >> 
> > > org.apache.batik.css.engine.CSSEngine.getCascadedStyleMap(Unknown
> > > > > > > >>>> Source)
> > > > > > > >>>>         at 
> > > > > > > >> 
> org.apache.batik.css.engine.CSSEngine.getComputedStyle(Unknown
> > > > > > > >>>> Source)
> > > > > > > >>>>         at 
> > > > > > > >> 
> org.apache.batik.bridge.CSSUtilities.getComputedStyle(Unknown
> > > > > > > >>>> Source)
> > > > > > > >>>>         at 
> > > > > > > >> 
> org.apache.batik.bridge.CSSUtilities.convertVisibility(Unknown
> > > > > > > >>>> Source)
> > > > > > > >>>>         at
> > > > > > > >>>> 
> > > > > 
> org.apache.batik.bridge.SVGSVGElementBridge.createGraphicsNode(Unknown
> > > > > > > >>>> Source)
> > > > > > > >>>>         at 
> org.apache.batik.bridge.GVTBuilder.build(Unknown 
> > > > > Source)
> > > > > > > >>>>         at
> > > > > > > >>>> org.apache.fop.render.pdf.PDFSVGHandler.
> > > > > > > >>> renderSVGDocument(PDFSVGHandler.java:188)
> > > > > > > >>>> 
> > > > > > > >>>> 
> > > > > > > >>>> Any help warmly welcomed!
> > > > > > > >>>> 
> > > > > > > >>>> Thanks
> > > > > > > >>>> 
> > > > > > > >>>> Peter
> > > > > > > >>>> 
> > > > > > > >>>> 
> > > > > > > >>>> -- 
> > > > > > > >>>> View this message in context: 
> http://www.nabble.com/Batik-
> > > > > > > >>> 
> > > exception-when-using-fop-with-svg-images-in-threaded-environment-
> > > > > > > >>> tp20809049p20844449.html
> > > > > > > >>>> Sent from the Batik - Users mailing list archive at 
> > > Nabble.com.
> > > > > > > >>>> 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > Jeremias Maerki
> > > > > > 
> > > > 
> > > > 
> > 
> > 
> > 
> > 
> > Jeremias Maerki
> > 
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > For additional commands, e-mail: [EMAIL PROTECTED]
> > 




Jeremias Maerki


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to