------- Additional Comments From [EMAIL PROTECTED]  2007-05-28 02:07 -------
Wow, it took me a lot longer to thoroughly review this patch than I anticipated.
I'm almost ready to commit. I only have to test the PDFTrancoder and
PDFDocumentGraphics2D. Here are the issues I found and what I did about them:

- Javadocs: Still some issues there. I fixed the most important locations.
- FopCache: I did a double-check on that to see if I've done anything wrong, but
after applying the patch the cache file was always empty. The writing methods
were simply not referenced anywhere in the code. Therefore the font lookup
performance wasn't improved. I also wondered why it has to be done in such a
complicated way (writing and parsing XML using custom code) just for a cache
file. So since the code didn't work as expected I simply tore it out (Sorry,
Adrian) and used Java's Serializable mechanism. That has the elegant side-effect
that there's a lot less code to maintain and it's probably faster. And I renamed
FopCache to FontCache and moved it into the fonts package. I didn't like all
this cache stuff cluttering the apps package. Now on my machine the speed-up is
from 1200ms for the first run, to 70ms with the cache despite over 150 fonts
being registered. Another issue in that area was the use of the singleton
pattern for the FontCache. I have now attached the FontCache to the FopFactory
like every other cache we have. Furthermore, the default location of the cache
file is changed to <user-home>/.fop/fop-fonts.cache. I did this because the font
auto-detection also checks user directories so this made sense to me. I have not
yet added a configuration item to define the cache location and I removed the
command-line argument for the cache location as it had no counterpart for
embedded use. We can add that later when we see a need for it.
- TrueType Collections (*.ttc)and OpenType fonts with an ".otf" extension are
currently not found. I'll add that after applying the patch.
- TrueType Collections are not properly supported with the auto-detection
mechanism. ATM, if someone wants to use a TTC, he has to configure it using a
XML font metric file as before. We can improve that later.
- The same applies to fonts that should only be referenced but not embedded. For
certain environments you have all the fonts already installed on the production
printers and you may want to reference those. But I guess that's becoming less
important nowadays.
- The whole font configuration stuff got lost for PDFDocumentGraphics2D. I've
addressed that and took the opportunity to remove some of the Avalon
dependencies, too. This as a preparation for the move to Commons/Batik.
- I addressed a small problem when the font base URL is no file URL. The font
auto-detection will then be skipped for the font base URL.
- The RendererContextInfo stuff was quite confusing to me (especially the
somewhat abstract naming). I eventually managed to simplify it which resulted in
removing RendererContextInfo* and everything that's necessary is now in
XMLHanderConfigurator because RendererContextInfoConfigurator really configured
the XMLHandler, not the RendererContext.
- Another case where I thought I was doing something wrong: Type 1 fonts simply
didn't work. All fonts failed because the auto-detection code was accessing the
PFB file instead of the PFM. I fixed that.
- I made the font code a little less verbose on the log as with auto-detection
I'm not really interested in all the details. That's another case where dividing
developer feedback from user feedback will be helpful.

While working on this patch, I kept thinking that there's still some room (and
necessity) for improvement with the whole font handling (not directly related to
this patch). The big issue: The whole font configuration is currently triggered
during the renderer setup. That means it is done each time a new renderer is
created. Plus we still have a font config per renderer which is not beautiful. I
keep ending up with my "font source" idea I had back when I discussed the whole
font thing with Victor. I guess I will tackle that one day now that we've
decided not to work with FOrayFont...

Plus another few notes for the future:
- A facility for font subsitution is getting more important to have with
auto-detection since you don't specify the font names yourself anymore. Suppose
you have an FO file that uses "StandardGrotesk" but the actual font is
registered by auto-detection as "StandardGroteskBSK-Regular". But part of the
problem is our lack of parser for AFM files which contains font family
information in contrast to the PFM file. FOrayFont has an advantage here.
- Suppose you have a font set with Light, Regular, Medium, Bold, ExtraBold and
Super and they should actually be accessible under the same font family but with
the integer weights (...400, 500, 600...) from the FO spec. Similar to the point
above you will want to be able to specify a mapping which is probably very hard
to do automagically.

Anyway, thanks a lot, Adrian, for your work and your patience. There's no need
to improve anything on the patch for now. I've handled all issues that were
important to me. We can refine later. Commit coming up in the next hours.

Configure bugmail:
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

Reply via email to