On 10.10.2006 21:31:49 Peter Coppens wrote:
> 
> I finished(?) working on the somewhat modified items (1), (2) and (4) of
> Jerememias' list.
> 
> I plan to clean up to code a bit and then try to submit a patch (there
> should be a first time for everything I guess :))

It's not that difficult. :-)

> Here is an overview of what has been done - probably belongs in the patch as
> well, but if someone, despite the length of this post, decides to go through
> it anyway, an upfront red flag can be waved if needed, keeping me from going
> through the patch process.....
> 
> 
> >From the users point of view.
> 
> The rgb-icc function is accepted as input for color properties of fo
> elements and the generated PDF will include and refer to those color
> profiles. Other renderers will use profile based converted sRGB values. The
> ICC profiles are loaded using the src attribute from the color-profile
> element. The uri resolution relies on how the Java VM resolves color
> profiles.

Would you please try to run URI resolution through
FOUserAgent.resolveURI()? That way we'll have a uniform URI resolution
everywhere.

> A cmyk function accepting 4 arguments (values ranging from 0%-100% or
> 0.0-1.0) is also available. The PDF renderer will generate DeviceCMYK colors
> based on the provided values, the other renderers will use a "standard"
> cmyk-rgb conversion and generated rgb colors.
> 
> 
> Implementation wise the following changes were done
> 
> 
> org.apache.fop.fo.expr.PropertyParser
> 
> 1/
> FunctionTable has two new functions, cmyk and rgb-icc linked to two new
> classes CMYKcolorFunction and ICCcolorFunction
> 
> 2/
> parsePrimaryExpr recognizes functions that return a negative number from the
> nbArgs as variable argument functions. Those are parsed with a new method,
> parseVarArgs

Clever.

> org.apache.fop.fo.expr.CMYKColorFunction
> 
> New function class for the cmyk function
> 
> org.apache.fop.fo.expr.ICCColorFunction
> 
> New function class for the rgb-icc function. It is a somewhat peculiar
> function class for two reasons. 
> 
> First, it returns an negative number for nbArgs (flagging a variable number
> of arguments function). 
> 
> Second its eval method returns a string that contains a fop-rgb-icc function
> call. The arguments for this new function are the same as the one of rgb-icc
> with the exception of the fifth, which is an extra argument that contains
> the src attribute for the color profile (from
> /root/declarations/color-profile/@src)
> 
> An alternative suggested by Jeremias was to introduce a color context
> tracking the mapping between ICC profile name and src. Certainly a more
> elegant approach but it would require more classes to be touched something I
> was reluctant to do (this being my first attempt to a fop contribution)

I like your idea. Quite creative!

> 
> org.apache.fop.fo.pagination.Declarations
> 
> Added a new method 
>       public ColorProfile getColorProfile(String cpName)
> 
> org.apache.fop.fo.pagination.ColorProfile
> 
> Added a new method 
>       public String getSrc()
> 
> 
> org.apache.fop.util.ColorUtil
> 
> 1/
> Added a static member referring to a/the Log object
> 
> 2/
> Added a static (synchronized) Map, colorSpaceMap, mapping color profile
> src's to ICC_ColorSpace instances.

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.

> 3/
> parseColorString
> 
> Now also recognizes fop-rgb-icc and cmyk functions invoking parseAsFopRgbIcc
> or parseAsCMYK.
> 
> 4/
> Removed cloning of the Color object before returning from parseColorString
> (has a TODO in the current code)
> 
> 5/
> Added 
>       private static Color parseAsFopRgbIcc(String value) 
>               throws PropertyException
> 
> If the color profile src (5th argument) is not available in the
> colorSpaceMap cache, an attempt is made to create an ICC_Profile and
> associated ICC_ColorSpace based on the src.  If successful, the ColorSpace
> is added to the cache, if not the fallback rgb values are used to create an
> rgb based color. If an ICC_ColorSpace is available an RgbIccColor (new
> class) object is created.
> 
> 6/ 
> Added
>       private static Color parseAsCMYK(String value) 
>         throws PropertyException
> 
> Parses cmyk arguments and creates a Color from a CMYKColorSpace color space.
> 
> 
> 7/
> colorTOsRGBString
> 
> Changed to recognize and handle cmyk and rgb-icc colors. Perhaps the name of
> the function should be changed?

Please do so. This stuff is only used internally so we don't have to
worry about backwards-compatibility.

> 
> 
> org.apache.fop.util.CMYKColorSpace
> 
> Implemented 
>       public float[] toRGB(float[] colorvalue)
> 
> 
> org.apache.fop.util.RgbIccColor
> 
> New Color class keeping track of the rgb-icc arguments and color profile src
> information.
> 
> 
> 
> org.apache.fop.pdf.PDFColor
> 
> 1/
> 
> Added a private RgbIccColor member. This is probably the least elegant
> change in this patch. 
> 
> There is something I do not understand with the way PDFColorSpace,
> PDFDeviceColorSpace and PDFPathPaint are used. PDFPathPaint has a
> PDFDeviceColorSpace member while for me and at least at first sight it would
> make more sense to use a PDFColorSpace member in stead. Changing that would
> however result in quite a few changes lots of which would occur in code I
> have no clue when it would be called. 
> 
> The alternative approach I took is to add an RgbIccColor member which is
> only filled in when the PDFColor is created from an RgbIccColor source in
> PDFColor(java.awt.Color col). 
> 
> Eventually, perhaps when svg rgb-icc and cmyk support is added, this will
> have to be dealt with. 
> 
> Bottom line is I would prefer to see these first set of changes rolled in
> before starting to add changes to code not well understood.

I think that approach is ok. The color space handling in the PDF library
is indeed a little strange. It was mostly because of inability of the
Java classlib to map some of the cases (like DeviceCMYK).

> 2/
>       public PDFColor(java.awt.Color col)
> 
> Recognize Color instances based on CMYK or RgbIccColor and store relevant
> information
> 
> 3/
>       public String getColorSpaceOut(boolean fillNotStroke)
> 
> Recognize RgbIccColor based PDFColor instances and act accordingly
> 
> 
> 
> org.apache.fop.render.pdf.PDFRenderer
> 
>       protected void setColor(Color col, boolean fill, StringBuffer pdf)
> 
> When color is an RgbIccColor instance the ICC profile is added to the pdf
> when needed.
>
>
> Peter


I'm impressed! Looks like I had the right feeling about you. Thank you
very much for your detailed information. It helps a lot to understand
what you're doing and I think it is going exactly in the right direction.

Sorry that I can't help more but it looks like you have everything under
control.

<snip/>


Jeremias Maerki

Reply via email to