Patrick, sorry for the delay. I've taken a first look at your patch. Looks good. The only thing I have changed is moving FontMetricMapper back to render.java2d since no other class in the fonts package deals with AWT fonts, yet. And FontMetricMapper would have been the first.
Since your patch contains new files, would you please fill out and submit an ICLA to the Apache Software Foundation? You can find the form here: http://www.apache.org/licenses/#clas Please also check if your company might need to submit a CCLA, too. As soon as I've received notification from the ASF's secretary, I'll commit your patch. In the meantime, I'll do some more testing. More inline below... On 02.12.2007 22:45:13 Patrick Jaromin wrote: > Jeremias- > > Alright. Between the kid's naps, I've managed to create a patch to allow > for user-configured fonts in Java2DRenderers. I've tested it with the > PNG and TIFF renderers. > > A couple things... > > 1) I've renamed the original "FontMetricMapper" to > "SystemFontMetricMapper" and created a new interface > "org.apache.fop.fonts.FontMetricMapper". Then the new implementation is > called "CustomFontMetricMapper," which represents the fonts *not* > installed on the system but described in the fop config. I looked at > maintaining the original naming but just didn't like any of the names I > could think of. If this won't fly perhaps someone can suggest an > alternative naming strategy. It's fine IMO. > 2) I changed the signature of the FontInfo.setup method (the java2d > version of that class). It actually now more closely resembles its > counterpart. Since the PCLRenderer uses this same class I changed the > call there as well. However, I don't know how to test that renderer > properly and am just guessing that it would be fine. If not, it would be > simple to overload the setup method with the former signature and revert > PCLRenderer. I'll do the testing there. > 3) I don't really know how to write tests for these changes and so, > consequently, I didn't. I used the fop script to output PNG and TIFF > images with various combinations of settings in my userconfig.xml and it > ultimately worked as expected. I'm a pretty big proponent of test-driven > development but it wasn't obvious to me how to add new tests -- I'm not > familiar with what's setup here and don't have a ton of free time to > research it. Let me know if there's something I could read that would > help me resolve that quickly. I know, renderer testing is tricky to do in a way that testing is stable and useful. Therefore we don't do that, yet. We used to have binary comparison for PDF files but that too unstable and people didn't use it. For PDF, PS and Java2D (and related) I can create bitmaps which I can compare visually by hand. For the other formats, this is much more tedious. There just isn't a one solution that fits all cases. > 4) A minor thing...I had originally implemented > "CustomFontMetricsMapper.mapChar()" method to simply return the > character provided (as the original FontMetricsMapper did). However, > there is a side-effect in the underlying Typeface implementation > delegate - MultiByteFont - of calling it's mapChar() method. If you > *don'* call it, then "embeded fonts" (which these are considered since > they pass the test (MultiBytFont.isEmbedable()) ) won't work...NPE on > getWidth(). I didn't spend much time looking for a solution to this -- I > simply delegated the mapChar method to the internal typeface, which > solved the problem. I don't think this is wrong, just don't like to > leave these side-effect. I could see somehow re-working the definition > of "embeded font" and come up with some way to make it so that these > fonts aren't classed "embeded"...though it's probably far more trouble > than it's worth...but thought I'd mention it. I'll look into it. > I've attached the patch to this message...I apologize if this is > problematic but I wanted to check primarily on the FontMetricMapper > rename before doing anything else with it, like sumbmitting to Bugzilla. Not strictly necessary now that I've already patched a local working copy of FOP. Generally, it is a good idea, though. A patch is less likely to be forgotten though that still occasionally happens. :-( > Let me know. > > Thanks! > > - Patrick <snip/> Jeremias Maerki
