Jeremias- Excellent, thanks.
No biggie on the time...figured you had other priorities. In any event, I'll get the paperwork in. This will allow me to remove the ugly hack version from my own head branch. I'll check on the CCLA -- I'm authorized to sign for the company on stuff like this, so it's no problem. Thanks! - Patrick Patrick K. Jaromin VP Technology 312.943.1600 * fax 312.640.8100 JGSullivan Interactive, Inc. http://JGSullivan.com -----Original Message----- From: Jeremias Maerki [mailto:[EMAIL PROTECTED] Sent: Friday, December 07, 2007 5:03 AM To: fop-dev@xmlgraphics.apache.org Subject: Re: Custom Fonts in Java2DRenderer & Hardcoded ColorModel There were a couple of small problems to fix in the PCLRenderer but it seems to work fine now, even with custom fonts. So from my POV the patch is ready to be applied. I'll just have to wait for the ICLA. On 07.12.2007 11:41:07 Jeremias Maerki wrote: > 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 > Jeremias Maerki
