DO NOT REPLY [Bug 41831] [PATCH] Refactored configuration, font detection and caching, url resolution
https://issues.apache.org/bugzilla/show_bug.cgi?id=41831 Glenn Adams gl...@skynav.com changed: What|Removed |Added Status|RESOLVED|CLOSED --- Comment #35 from Glenn Adams gl...@skynav.com 2012-04-01 06:38:19 UTC --- batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- 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
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 [EMAIL PROTECTED] changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED --- Additional Comments From [EMAIL PROTECTED] 2007-05-28 07:33 --- Patch applied: http://svn.apache.org/viewvc?view=revrev=542237 -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 [EMAIL PROTECTED] changed: What|Removed |Added Status|NEW |ASSIGNED --- Additional Comments From [EMAIL PROTECTED] 2007-05-23 08:19 --- I will take this on, since Andreas has mysteriously vanished. Thanks, Adrian, for revising the patch. The new one is an significant improvement over the previous. I'm in the process of reviewing the patch in detail. I will be able to commit it on Friday. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #19756|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2007-05-22 09:52 --- Created an attachment (id=20242) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=20242action=view) Patch file -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #19712|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2007-05-22 09:57 --- Created an attachment (id=20243) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=20243action=view) List of updated/new files -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-05-22 10:08 --- This patch contains many design and style changes and improvements including (but not limited to) :- * Small URI related JDK1.4 dependencies removed from FOURIResolver. * DirectoryWalker from commons-io-1.3.1.jar is now used for font detection. * cache-fonts/ has been renamed to use-cache/ so it may cover other general caching uses in the future. Its a large patch so I may have missed something, so please try the patch out. Feedback welcome :-) Adrian. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-05-18 10:09 --- Andreas, if you come back online again please do not commit the patch just yet as I have made some significant improvements to it which will be attached soon. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-05-11 02:08 --- Andreas, are you still working on this patch? -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-04-23 08:09 --- *** Bug 41514 has been marked as a duplicate of this bug. *** -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-04-09 01:49 --- Hi Andreas, I've just checked the zip file and it does contain a NativeFontDirFinder.java file. What I submitted seemed to work ok for Keith.. maybe the problem lies in the merge after your ammendments? I think I renamed and changed the type of fontPaths to fontDirs (it now uses an entity inner class called FontDirInfo defined in NativeFontDirFinder to hold font paths). Hope you get time to commit the patch, wishing you a nice easter holiday period :-) Adrian. (In reply to comment #24) (In reply to comment #20) Hi Adrian, Andreas, this patch is getting a little out of date with the trunk now - if you need me to create a fresh patch from the latest trunk code let me know. Just a little nit: the last attachment does not include an updated NativeFontDirFinder, while at least the addition of a 'fontPaths' member seems to have changed...? Is it possible this was changed, or did I miss something? Cheers, Andreas -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-04-05 11:21 --- (In reply to comment #21) Hi Keith / Adrian, ... I wonder whether it is worth changing this log from info to debug after the first run (FontInfoFinder line 151). If you have a lot of fonts, it can produce several pages of messages (mainly from /usr/share/fonts/type1/gsfonts on my system). Indeed, I tend to agree with Keith here. Adrian, what do you think? Of course, the issue is rendered moot once we're talking about servlet environments. Still, we do have CLI users to take into account... Cheers, Andreas -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-04-05 11:33 --- (In reply to comment #20) Hi Adrian, Andreas, this patch is getting a little out of date with the trunk now - if you need me to create a fresh patch from the latest trunk code let me know. Just a little nit: the last attachment does not include an updated NativeFontDirFinder, while at least the addition of a 'fontPaths' member seems to have changed...? Is it possible this was changed, or did I miss something? Cheers, Andreas -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-04-03 00:00 --- Hi Adrian, Just a little FYI: I did not get around to applying the patch during the weekend (because of a small disagreement with my ISP over the price of my connection... I thought it should be free ;-) As a result they cut me off... Checking my e-mail via Webinterface now) Anyway, this will all be settled later this week, so it will take a bit more time than I had anticipated, but just wanted to let you know that I haven't forgotten about this one. Locally, I've done about all I can. Just need to get to uploading. Cheers, Andreas -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=41831.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=41831
--- Additional Comments From [EMAIL PROTECTED] 2007-04-02 02:40 ---
Hi Keith,
Many thanks for trying out the patch and taking the time to provide me with
feedback. My comments are below.
(In reply to comment #17)
I tried out this patch on Ubuntu Linux and found that I needed a few changes
to
get it to work properly. Since this hasn't been checked in, I'm not sure what
to
send a patch against, so I'll just describe them for now.
org.apache.fop.fonts.autodetect.UnixFontDirFinder
It is probably worth adding the .fonts directory in a user's home directory:
e.g.
UNIX_FONT_PATHS = {
System.getProperty(user.home) + /.fonts,
Ok this is a nice easy addition. Andreas, if you are reading this
I believe you have made some small changes. Before I make any of the changes
that Keith suggests, it might be a good idea for you to attach an additional
patch file with your changes.
org.apache.fop.fonts.autodetect.FontFileFinder.find should have the recursive
argument set to true when called from
org.apache.fop.fonts.autodetect.NativeFontFileFinder.find, because the fonts
are
found several layers deep under /usr/share/fonts.
I am thinking that I may make the NativeFontFileFinder class a little more
flexible so that implementing classes are able to specify whether each
individual search file path should be searched recursively or not.
org.apache.fop.fonts.autodetect.FontInfoFinder
If you use
embedUrl = fontFile.toURI().toURL().toExternalForm();
then it seems to do escaping of spaces, otherwise font filenames containing
spaces cause problems.
This is also a quick easy change.
It is probably worth handling UnsupportedOperationException within the
org.apache.fop.fonts.autodetect.FontInfoFinder.find() method to prevent one
bad
font breaking everything. I find OpenType fonts with CFF data are not
supported, yet errors thrown by org.apache.fop.fonts.truetype.TTFFontLoader
break the caching otherwise.
Yes this is a case I did not cater for. I will change this.
I'm not convinced that the cache is working properly for failed fonts,
because I
always seem to get log messages about them, not just on the first run.
However,
I haven't had time to investigate that properly.
I believe this to be working fine. Bad/failed fonts are also recorded in the
cache. The error is still printed to remind the user of the problem but the
font is not parsed again unless the file has been changed.
Thanks again Keith for taking time to try out the patch and provide feedback
:-).
Cheers,
Adrian Cumiskey.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-04-02 06:37 --- Created an attachment (id=19890) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=19890action=view) updated files from org.apache.fop.fonts.autodetect package Following feedback from Andreas and Keith I have updated the autodetect package. I hope this does not cause you too many merge problems Andreas. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #19890|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2007-04-02 06:55 --- Created an attachment (id=19891) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=19891action=view) updated files from org.apache.fop.fonts.autodetect package Whoops.. This is what I should have submitted. Please ignore the last attached archive file. Andreas, this patch is getting a little out of date with the trunk now - if you need me to create a fresh patch from the latest trunk code let me know. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-04-02 10:22 --- Hi Adrian, Thanks for making those changes, they seem to work for me. (In reply to comment #18) I believe this to be working fine. Bad/failed fonts are also recorded in the cache. The error is still printed to remind the user of the problem but the font is not parsed again unless the file has been changed. Sorry, yes, the failed fonts are recorded fine. However, I wonder whether it is worth changing this log from info to debug after the first run (FontInfoFinder line 151). If you have a lot of fonts, it can produce several pages of messages (mainly from /usr/share/fonts/type1/gsfonts on my system). thanks for a very useful patch. Keith Stribley -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=41831.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=41831
--- Additional Comments From [EMAIL PROTECTED] 2007-03-30 16:42 ---
I tried out this patch on Ubuntu Linux and found that I needed a few changes to
get it to work properly. Since this hasn't been checked in, I'm not sure what to
send a patch against, so I'll just describe them for now.
org.apache.fop.fonts.autodetect.UnixFontDirFinder
It is probably worth adding the .fonts directory in a user's home directory:
e.g.
UNIX_FONT_PATHS = {
System.getProperty(user.home) + /.fonts,
org.apache.fop.fonts.autodetect.FontFileFinder.find should have the recursive
argument set to true when called from
org.apache.fop.fonts.autodetect.NativeFontFileFinder.find, because the fonts are
found several layers deep under /usr/share/fonts.
org.apache.fop.fonts.autodetect.FontInfoFinder
If you use
embedUrl = fontFile.toURI().toURL().toExternalForm();
then it seems to do escaping of spaces, otherwise font filenames containing
spaces cause problems.
It is probably worth handling UnsupportedOperationException within the
org.apache.fop.fonts.autodetect.FontInfoFinder.find() method to prevent one bad
font breaking everything. I find OpenType fonts with CFF data are not
supported, yet errors thrown by org.apache.fop.fonts.truetype.TTFFontLoader
break the caching otherwise.
I'm not convinced that the cache is working properly for failed fonts, because I
always seem to get log messages about them, not just on the first run. However,
I haven't had time to investigate that properly.
cheers,
Keith
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-03-26 14:09 --- Adrian, Just to keep you up to date: I'm almost done here. Apart from the nits mentioned earlier, everything looks A-OK to me. I'll wrap things up later this week, and will then first attach a slightly revised patch here. If it meets with your approval, then it will most likely get committed by the weekend. Thanks again for this fine contribution! Andreas -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-03-26 16:09 --- Hi Andreas, Many thanks for reviewing and taking care of commiting my patch. I realise you guys are very busy and I really appreciate you finding the time to thoroughly review my large patch and provide me with feedback :-). I don't think I will be making any more minor tweaks to this patch at this time as I am busy working on resolving some issues with the postscript renderer. Of course if anything crops up after the commit I will be more than happy to look at it. I very much look forward to seeing the new features I've added being part of the FOP trunk :-). All the best, Adrian Cumiskey. (In reply to comment #15) Adrian, Just to keep you up to date: I'm almost done here. Apart from the nits mentioned earlier, everything looks A-OK to me. I'll wrap things up later this week, and will then first attach a slightly revised patch here. If it meets with your approval, then it will most likely get committed by the weekend. Thanks again for this fine contribution! Andreas -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-03-24 04:07 --- Hi Adrian, Had some more time to look into this. I'm currently on a more detailed stroll through your code. Just like to know what I commit, nothing personal. ;-) The change to MacFontFinder is working as it should. In the meantime, if you make any more changes, try to post incremental patches only containing the classes you know for certain were altered since the last diff. I'll do my best to keep up. Here's already a few suggestions for change, but there's no immediate necessity to create a new patch. I've already corrected some things locally, mainly style issues. For the javadocs, we like to have the class-level doc right before the class declaration, after the import header. As you have it, Eclipse collapses them with the license header. :( In WindowsFontFinder, there's that hacky fallback you're commenting on: if shelling out is necessary, then note that 'set windir' will immediately return only the line containing 'windir=', so there's no need to iterate over all environment variables. Still looking further, but these are some things I've already stumbled upon. Stay tuned. Cheers Andreas -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #19713|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2007-03-20 09:25 --- Created an attachment (id=19756) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=19756action=view) Patch file Following feedback from Andreas - thanks v much for taking the time to look at this :-), I have made a very minor modification to the MacFontDirFinder. It should hopefully now be able to detect fonts that have been installed under user home directory (i.e. ~/Library/Fonts). Please try this patch out on your system Andreas. I would encourage, maybe even urge ;-) someone to commit this patch soon as it is a large patch that touches upon a number of files (see file list) and is therefore likely to become out of date quickly with commits to the repository. Kind regards, Adrian. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-03-16 13:57 --- Something else just occurred to me (In reply to comment #7) ... only, I can't find a TTF file on my system for which I get no complaints (about ascender/descender conflicts, or absent unicode cmaps) Must be something Mac-related (? or did you test on that platform yet?) I would very much like to know what the results are on an Intel Mac. I'm still on the PowerPC. As I was browsing through the font handling code, there's a lot of byte-reading and bit-shifting done... Well, maybe if I find the time, I'll study the TTF-spec, step through the code and have a look at how the bytes are interpreted over here... -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-03-15 03:13 --- Hi Andreas, Thanks for trying out the patch :-). (In reply to comment #7) Adrian, After applying the changes in the patch, it /seems/ to work... only, I can't find a TTF file on my system for which I get no complaints (about ascender/descender conflicts, or absent unicode cmaps) Must be something Mac-related (? or did you test on that platform yet?) It is good that you are a Mac user as I was unable to test on the Mac. The Mac native font finder should examine the following folders (not recursively) in order to search for available system fonts :- ~/Library/Fonts/, // user /Library/Fonts/, // local /Network/Library/Fonts/, // network /System/Library/Fonts/, // system /System Folder/Fonts/ // classic If I could also have some feedback from unix users that would be great. As for the fine-tuning: I get a few SEVERE errors when the MacFontInfoFinder wants to access directories that don't exist. Those need to be suppressed. I have now suppressed this behaviour and will add it to the next patch submission. I'm also not too sure I like the idea of automatically /loading/ all the present fonts... Take care of what FontLoader does. It reads the whole TTF-file into a byte array, and some Unicode fonts are rather large beasts to read into memory entirely. If you have a few of those installed...? I'd try to limit it to the fonts that will actually be used, i.e. keep the possibly used font-info cached, but only trigger loading the font during property-resolving when the corresponding font-family is encountered. The fonts should only be loaded the first time you run FOP in autodetect and cache mode. After this first run, all the font info should be stored in the fop.cache (should be created in fop/conf/fop.cache) and subsequent FOP executions should make use of the cache. Even though this detection is only expensive at the first time of execution, I take your point on board and am considering lightening the font loading for this autodetect purpose - maybe a call along the lines of FontLoader.loadLightFont(fontFile, resolver) - I may add this in the next patch update. As you indicated, a bit of tweaking and tuning necessary, but all in all 'Nice Job!'. Yes I think there will be some more tweaking, the more people try this patch out the better - I am sure there will be some things I haven't considered. Cheers, Adrian. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #19702|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2007-03-15 08:23 --- Created an attachment (id=19712) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=19712action=view) List of updated/new files -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #19703|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2007-03-15 08:31 --- Created an attachment (id=19713) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=19713action=view) Patch file Now suppress error messages on failed native font finding as requested by Andreas. Also tidied up the FontLoader so the read() method does the work on calls to load() method and the implementing constructors are nice and dumb as they should be :-). -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-03-15 10:28 --- (In reply to comment #8) It is good that you are a Mac user as I was unable to test on the Mac. The Mac native font finder should examine the following folders (not recursively) in order to search for available system fonts :- ~/Library/Fonts/, // user /Library/Fonts/, // local /Network/Library/Fonts/, // network /System/Library/Fonts/, // system /System Folder/Fonts/ // classic Only local and system worked with me. FOP claimed the other three didn't exist, but: - user: the ~ does not get resolved correctly; /~/Library/Fonts indeed does not exist, since I have no directory /~, that is cd /~ doesn't work. I think if you could somehow avoid the slash from being prepended, then it would work. - network: the Library alias exists with me, but the corresponding device [/automount/Library] is not mounted... - classic: indeed does not exist, but I think we don't even need to consider supporting Classic anymore. The latest available JVM for that platform was 1.2, IIRC, so if we consider 1.4 as minimal target JVM for the next release, FOP wouldn't run on it anyway. The fonts should only be loaded the first time you run FOP in autodetect and cache mode. After this first run, all the font info should be stored in the fop.cache (should be created in fop/conf/fop.cache) and subsequent FOP executions should make use of the cache. Good point. I was only thinking about a recent post on fop-users@ where Abel Braaksma pointed out that loading a 23MB font-file into memory just to use one or two characters was slight overkill. I agree with his point, and I would certainly want to avoid blindly loading three or four of these fonts into memory if they aren't used at all. Besides that, it's not even during the property-resolving stage that the font needs to be loaded. All the FO tree needs to know is whether the supplied font exists in the specified style/weight. It's only during layout that more info about the font is needed (metrics, kerning pairs...). If the FO isn't valid, then the font never needs to be loaded. I agree with the point that FOP only needs to determine the first time whether a font /exists/ and store that info in a cache, but if no document ever uses that font, it should never be loaded completely, which is exactly what fop.fonts.truetype.FontFileReader does behind the scenes, when you call loadFont (). Maybe fop.fonts.LazyFont can be used for this. It seems to fit the purpose, no? -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 [EMAIL PROTECTED] changed: What|Removed |Added CC|fop-| |[EMAIL PROTECTED] | AssignedTo|[EMAIL PROTECTED] |fop- |tech.com|[EMAIL PROTECTED] --- Additional Comments From [EMAIL PROTECTED] 2007-03-14 02:59 --- Hi Adrian, Wow, looks like a big patch! A few quick comments: - when you submit bugs, please don't assign them to one of the developers, but rather to fop-dev, so that everyone can follow them - given the size of the patch, and if you've not already done so I'll ask you to send a signed ICLA (and a CCLA, if necessary) to the ASF's secretary. See here for details: http://www.apache.org/licenses/#clas I'll try to have a look ASAP, but I'm quite busy currently. If anyone beats me to it, all the better. Thanks! Vincent -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee. You are on the CC list for the bug, or are watching someone who is.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-03-14 03:08 --- Hi Vincent, I CC'd fop-dev in on this patch so everyone should have been able to follow the patch and provide feedback - but I will assign it directly in future. I have sent a signed ICLA to Apache so everything should be fine, but I have not received any confirmation from them of its receipt. Please everyone, try the patch out. Cheers, Adrian. (In reply to comment #3) Hi Adrian, Wow, looks like a big patch! A few quick comments: - when you submit bugs, please don't assign them to one of the developers, but rather to fop-dev, so that everyone can follow them - given the size of the patch, and if you've not already done so I'll ask you to send a signed ICLA (and a CCLA, if necessary) to the ASF's secretary. See here for details: http://www.apache.org/licenses/#clas I'll try to have a look ASAP, but I'm quite busy currently. If anyone beats me to it, all the better. Thanks! Vincent -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-03-14 13:35 --- (In reply to comment #5) Adrian, I had a quick look at the code (haven't tried it out /yet/ ;-)) Very impressive piece of work! Auto-discovery of fonts is a very long-awaited feature. Yep, that's going to be a good step forward in user-friendliness. I can't comment on the patch, though, as I haven't had time, yet. Thanks for working on this, Adrian. Considering the ICLA: if you can, fax it to Apache, just to be certain. I didn't even send mine by snail- mail, since I have lost hope in that some time ago... Hmm, I'm wondering how long it will be before Apache enables potential committers to sign and send an ICLA /digitally/. Actually, that's possible. You can send a scanned ICLA by mail to [EMAIL PROTECTED] AND [EMAIL PROTECTED] Ideally, the mail is PGP signed. Just as an aside: No, Adrian's ICLA hasn't popped up in the secretary's records. I'll notify everyone as soon as that has happened. snip/ -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 --- Additional Comments From [EMAIL PROTECTED] 2007-03-14 16:49 --- Adrian, After applying the changes in the patch, it /seems/ to work... only, I can't find a TTF file on my system for which I get no complaints (about ascender/descender conflicts, or absent unicode cmaps) Must be something Mac-related (? or did you test on that platform yet?) As for the fine-tuning: I get a few SEVERE errors when the MacFontInfoFinder wants to access directories that don't exist. Those need to be suppressed. I'm also not too sure I like the idea of automatically /loading/ all the present fonts... Take care of what FontLoader does. It reads the whole TTF-file into a byte array, and some Unicode fonts are rather large beasts to read into memory entirely. If you have a few of those installed...? I'd try to limit it to the fonts that will actually be used, i.e. keep the possibly used font-info cached, but only trigger loading the font during property-resolving when the corresponding font-family is encountered. As you indicated, a bit of tweaking and tuning necessary, but all in all 'Nice Job!'. Cheers, Andreas -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 41831] - [PATCH] Refactored configuration, font detection and caching, url resolution
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=41831. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=41831 [EMAIL PROTECTED] changed: What|Removed |Added CC||fop- ||[EMAIL PROTECTED] -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug, or are watching someone who is.
