I filed it (http://issues.apache.org/bugzilla/show_bug.cgi?id=40729)
Thanks for your patience and support. Would not have worked without it. Next is svg I guess....but I am not sure that will be immediately. I have only a day or so left for now. Peter > -----Original Message----- > From: Jeremias Maerki [mailto:[EMAIL PROTECTED] > Sent: Wednesday, October 11, 2006 6:49 PM > To: fop-dev@xmlgraphics.apache.org > Subject: Re: Including an sRGB color profile? > > Without seeing the code, no objections here. It's good to be on the safe > side by using a synchronized Map. I'll look into the "base" parameter > for getColorSpace() when I see the code. > > When you submit the first patch, please don't expect a fast turn-around, > since right now I can't tell when I will be able to process it. I'm > currently struggling to juggle my items on my task list and all the > other FOP stuff (like this) in the little time I have available for FOP > nowadays. > > On 11.10.2006 17:35:34 Peter wrote: > > So I think I have it working but not without substantial changes > > > > A summary > > > > 1)FopFactory > > ============ > > *) Added a synchronized Map, colorSpaceMap to cache ...ColorSpace > > objects(life can be easy). Don't know whether this has to be > synchronized. > > Have not looked into the details of FOP MT features. > > > > *) Added a public method > > public ColorSpace getColorSpace( > > String base, > > String iccProfileSrc) > > > > Kind of awkward to have to pass in the base but the only way I could see > to > > work around that is to have the cache in the user agent iso the factory > > > > 2) AreaTreeParser.Handler#setTraits > > =================================== > > *) Change invocation of ColorUtil.parseColorString (2x). It now needs a > > first FOUserAgent argument > > *) Change invocation of BorderProps.valueOf. Expects a first FOUserAgent > > argument > > > > 3) NCNameProperty#getColor > > ========================== > > *) New first FOUserAgent argument > > *) Invoke ColorUtil.parseColorString with the FOUserAgent argument > > > > 4) NumericProperty#getColor > > ============================ > > *) New first FOUserAgent argument > > > > 5) PropertyParser#parsePrimaryExpr > > ================================== > > *) ColorProperty constructor takes new first FOUserAgent argument > > > > 6) SystemColorFunction#eval > > =========================== > > *) ColorProperty constructor takes new first FOUserAgent argument > > > > > > 7) ColorProperty > > ================ > > *) Constructor new first FOUserAgent argument > > *) getColor - new first FOUserAgent argument > > > > 8) CommonBorderPaddingBackground > > ================================ > > *) getColor - new first FOUserAgent argument > > > > 8) NumberProperty > > ================= > > *) getColor - new first FOUserAgent argument > > > > 9) Property > > =========== > > *) getColor - new first FOUserAgent argument > > > > 10) ColorUtil has been changed to take "advantage" of these changes > > > > > > I have not added a lot of tests to check the arguments which are > accessed to > > get hold of the FOUserAgent and the unit tests gave one NPE because one > of > > the arguments to one of the functions was null. I am trying to decide > > whether more defensive checks for null arguments would make sense. > > > > Anyone any thoughts? > > > > Thanks, > > > > Peter > > > > > > > > > -----Original Message----- > > > From: Jeremias Maerki [mailto:[EMAIL PROTECTED] > > > Sent: Wednesday, October 11, 2006 2:29 PM > > > To: fop-dev@xmlgraphics.apache.org > > > Subject: Re: Including an sRGB color profile? > > > > > > ColorUtil, as the name implies, is a utility class and it doesn't feel > > > right to put an instance of it in FOUserAgent. I would extract the > > > profile caching into a separate class and put that there (or rather in > > > the FopFactory so multiple rendering runs can use the cache). The > > > ColorUtil methods should then usually receive an instance of the > profile > > > cache to work with. > > > > > > You will easily see from the unit tests if anything breaks. I will > also > > > do extensive tests when reviewing your patch. Since it probably needs > > > some additional work afterwards, we will probably put it in a branch > > > until it's ready to be merged back into Trunk. Don't be too afraid to > > > change some method signatures. What needs to be done, needs to be > done. > > > > > > On 11.10.2006 14:07:51 Peter wrote: > > > > Some more information....found during the last few hours....and > finished > > > > typing in just before your reply came in. > > > > > > > > I have been trying to figure out whether it would help to add an > > > instance of > > > > ColorUtil to the FOUserAgent (and changing some of its methods and > > > members > > > > to instance level) > > > > > > > > Skipping to the conclusion first - in theory it seems possible to > make > > > > ColorUtil#parseColorString an instance method and add a ColorUtil > > > instance > > > > to the FOUserAgent. In theory, because API wise an FOUserAgent can > > > always be > > > > made available (given enough API changes) but I have no idea whether > it > > > will > > > > actually be there at runtime (could be null). > > > > > > > > Actually there might a problem with one or more static function of > one > > > of > > > > the calling classes/methods I just realize. > > > > > > > > Anyway, changes are rather extensive and I am not sure whether they > will > > > > work or break anything I overlooked. > > > > > > > > Please advice - I am willing to give it a try but need to decide > asap (I > > > am > > > > running out of my time slot that was made available for fop dev work > for > > > the > > > > coming weeks). > > > > > > > > Thanks, > > > > > > > > Peter > > > > > > > > > > > > What I found out, ColorUtil is referenced from > > > > > > > > 1/ colorEqualTest > > > > ==> Not an issue I guess (test) > > > > > > > > 2/ AreaTreeParser.Handler#setTraits > > > > ==> Not an issue as an FOUserAgent is available > > > > > > > > 3/ Trait.Background#toString ==> FOUserAgent not needed > > > > ==> Not an issue as an FOUserAgent is not needed (only colorToString > is > > > > called) > > > > > > > > 4/ NCNameProperty#getColor > > > > ==> NO FOUserAgent ==> ISSUE > > > > > > > > 5/ ColorProperty#ColorProperty > > > > ==> NO FOUserAgent ==> ISSUE > > > > > > > > 6/ ColorProperty#toString ==> FOUserAgent not needed > > > > ==> Not an issue as an FOUserAgent is not needed (only colorToString > is > > > > called) > > > > > > > > 7/ XMLRenderer#addTraitAttributes ==> FOUserAgent not needed > > > > ==> Not an issue as an FOUserAgent is not needed (only colorToString > is > > > > called) > > > > > > > > 8/ BorderProps#toString ==> FOUserAgent not needed > > > > ==> Not an issue as an FOUserAgent is not needed (only colorToString > is > > > > called) > > > > > > > > 9/ BorderProps#valueOf > > > > ==> NO FOUserAgent ==> ISSUE > > > > > > > > To summarize three invocations of ColorUtil#parseColorString give > > > problems > > > > > > > > I went a level further > > > > > > > > 4/ NCNameProperty#getColor > > > > ==> NO FOUserAgent ==> But all callers of getColor seem to have > access > > > to > > > > something that in theory should be able to return the FOUserAgent > > > > ==> No longer an issue > > > > > > > > > > > > 5/ ColorProperty#ColorProperty > > > > ==> NO FOUserAgent ==> But all callers of ColorProperty constructor > seem > > > to > > > > have access to something that in theory should be able to return the > > > > FOUserAgent ==> No longer an issue > > > > > > > > 9/ BorderProps#valueOf > > > > ==> NO FOUserAgent ==> ==> But all callers of valueOf (one :)) seem > to > > > have > > > > access to something that in theory should be able to return the > > > FOUserAgent > > > > ==> No longer an issue > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Jeremias Maerki [mailto:[EMAIL PROTECTED] > > > > > Sent: Wednesday, October 11, 2006 2:05 PM > > > > > To: fop-dev@xmlgraphics.apache.org > > > > > Subject: Re: Including an sRGB color profile? > > > > > > > > > > Ok for me, as long as we track the outstanding issues. > > > > > > > > > > On 11.10.2006 12:01:19 Peter wrote: > > > > > > > > > > > > > > Would you please try to run URI resolution through > > > > > > > FOUserAgent.resolveURI()? That way we'll have a uniform URI > > > resolution > > > > > > > everywhere. > > > > > > > > > > > > I understand your concern. I will look into it but the issue is > that > > > > > > ColorUtil is really a bunch of static methods that at this stage > > > have no > > > > > > access to any instance objects. I am sure that can be changed > but I > > > am > > > > > > concerned this is going to ripple down to a lot of files and I > would > > > > > rather > > > > > > start out by leaving as much code as possible untouched. > > > > > > > > > > > > > > > > > > > Uhm, that's not so good. We were trying to avoid statics where > > > > > possible. > > > > > > > I'd prefer to keep that Map in FOUserAgent (per rendering > run), or > > > if > > > > > > > you want to cache the profiles, we should make a cache class > with > > > > > > > SoftReferences similar to what is implemented in ImageFactory > and > > > > > attach > > > > > > > that to the FopFactory. But I can look into this. I don't want > to > > > put > > > > > > > too much on your shoulders if you don't want to. It's cool > enough > > > that > > > > > > > you're putting so much energy into this. > > > > > > > > > > > > Basically the same issue here. ColorUtil is static....and as a > > > (lousy) > > > > > > excuse, I mainly just duplicated the colorMap cache > "idea"....but I > > > > > agree > > > > > > one should avoid those static beasts. Not caching the > ColorProfiles > > > on > > > > > the > > > > > > other hand seems not a good idea either, from a memory > consumption > > > > > > perspective. > > > > > > > > > > > > Again (sorry for repeating myself) I would rather see this > included > > > and > > > > > > continue from there...building up fop-dev experience step by > step. > > > > > > > > > > > > > > > > > > > > Jeremias Maerki > > > > > > > > > > > > Jeremias Maerki > > > > Jeremias Maerki
