Fop
  1. Fop
  2. FOP-1261

[PATCH] Add support for rgb-icc and proprietary cmyk color functions

    Details

    • Type: Bug Bug
    • Status: Closed
    • Resolution: Fixed
    • Affects Version/s: trunk
    • Fix Version/s: None
    • Component/s: pdf
    • Labels:
      None
    • Environment:
      Operating System: All
      Platform: All
    • External issue ID:
      40729

      Description

      [PATCH]
      A patch is attached that adds support for the standard (color) rgb-icc function
      and a proprietary cmyk function.

      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 and resolved using the FOP URI resolver.

      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.

      1. fop-icc-cmyk-patch4.diff
        69 kB
        Peter Coppens
      2. fop-icc-cmyk-patch-2.diff
        79 kB
        Peter Coppens
      3. color-sandbox.fo
        2 kB
        Jeremias Maerki

        Activity

        Hide
        Peter Coppens added a comment -

        Attachment fop-icc-cmyk-patch4.diff has been added with description: Patch file

        Show
        Peter Coppens added a comment - Attachment fop-icc-cmyk-patch4.diff has been added with description: Patch file
        Hide
        Peter Coppens added a comment -

        This is a new version of the previous patch. A new version is needed in order
        to make the equivalent changes to batik easier.

        Show
        Peter Coppens added a comment - This is a new version of the previous patch. A new version is needed in order to make the equivalent changes to batik easier.
        Hide
        Peter Coppens added a comment -

        Attachment fop-icc-cmyk-patch-2.diff has been added with description: cmyk and rgb-icc support

        Show
        Peter Coppens added a comment - Attachment fop-icc-cmyk-patch-2.diff has been added with description: cmyk and rgb-icc support
        Hide
        Jeremias Maerki added a comment -

        I finally managed to look into this. The patch looks good after styling the code
        to our code conventions. I had to fix an NPE when there were no color-profiles
        specified and the rgb-icc() function was used. Furthermore, the fop-rgb-icc()
        function did not parse the fallback color values correctly. I now have this
        patch ready to be committed in a separate checkout locally. As soon as the ICLA
        is registered I'll commit.

        What we have to look into when the patch is applied:

        • When the PDF/A-1b profile is active, FOP does not check for CMYK colors when
          the output intent is sRGB (which is the default). I guess I'll have to look into
          this a little deeper. There may be additional things that need to be changed.
        • The PSRenderer will need to learn to handle the DeviceCMYK colors correctly.

        So, Peter, thanks a lot for the hard work so far. This looks really good. I bet
        many people will be extremely happy with the new functionality. Please have a
        little patience until the ICLA pops up.

        Show
        Jeremias Maerki added a comment - I finally managed to look into this. The patch looks good after styling the code to our code conventions. I had to fix an NPE when there were no color-profiles specified and the rgb-icc() function was used. Furthermore, the fop-rgb-icc() function did not parse the fallback color values correctly. I now have this patch ready to be committed in a separate checkout locally. As soon as the ICLA is registered I'll commit. What we have to look into when the patch is applied: When the PDF/A-1b profile is active, FOP does not check for CMYK colors when the output intent is sRGB (which is the default). I guess I'll have to look into this a little deeper. There may be additional things that need to be changed. The PSRenderer will need to learn to handle the DeviceCMYK colors correctly. So, Peter, thanks a lot for the hard work so far. This looks really good. I bet many people will be extremely happy with the new functionality. Please have a little patience until the ICLA pops up.
        Hide
        Peter Coppens added a comment -

        Apologies for the NPE's. Should have added some unit tests I guess.

        Concerning the open issues listed. Perhaps I can find some time to look at some
        of them (e.g. the PSRenderer) once this patch ends up in the code tree (and I
        get the Batik related changes in a state I can post).

        Peter

        PS As mentioned I faxed the ICLA so I hope it will make it through the process
        (whatever that is)

        Show
        Peter Coppens added a comment - Apologies for the NPE's. Should have added some unit tests I guess. Concerning the open issues listed. Perhaps I can find some time to look at some of them (e.g. the PSRenderer) once this patch ends up in the code tree (and I get the Batik related changes in a state I can post). Peter PS As mentioned I faxed the ICLA so I hope it will make it through the process (whatever that is)
        Hide
        Jeremias Maerki added a comment -

        (Trying this channel as Peter hasn't replied through direct mail, yet).

        Peter, the ASF's secretary has recorded a new ICAL entry as below. Is this not
        your entry or is it a mistake/typo? In Bugzilla it says your name is "Peter
        Coppens". The mail addresses also don't match.

        notinavail:Peter Coppans:Peter Coppans:pgp.coppans.at.gmail.com:Signed CLA

        Show
        Jeremias Maerki added a comment - (Trying this channel as Peter hasn't replied through direct mail, yet). Peter, the ASF's secretary has recorded a new ICAL entry as below. Is this not your entry or is it a mistake/typo? In Bugzilla it says your name is "Peter Coppens". The mail addresses also don't match. notinavail:Peter Coppans:Peter Coppans:pgp.coppans.at.gmail.com:Signed CLA
        Hide
        Peter Coppens added a comment -

        I guess my handwriting is not so readable

        Name is Peter Coppens, email is pgp.coppens@gmail.com

        Do I have to resend the document?

        (In reply to comment #5)
        > (Trying this channel as Peter hasn't replied through direct mail, yet).
        >
        > Peter, the ASF's secretary has recorded a new ICAL entry as below. Is this not
        > your entry or is it a mistake/typo? In Bugzilla it says your name is "Peter
        > Coppens". The mail addresses also don't match.
        >
        > notinavail:Peter Coppans:Peter Coppans:pgp.coppans.at.gmail.com:Signed CLA
        >
        >

        Show
        Peter Coppens added a comment - I guess my handwriting is not so readable Name is Peter Coppens, email is pgp.coppens@gmail.com Do I have to resend the document? (In reply to comment #5) > (Trying this channel as Peter hasn't replied through direct mail, yet). > > Peter, the ASF's secretary has recorded a new ICAL entry as below. Is this not > your entry or is it a mistake/typo? In Bugzilla it says your name is "Peter > Coppens". The mail addresses also don't match. > > notinavail:Peter Coppans:Peter Coppans:pgp.coppans.at.gmail.com:Signed CLA > >
        Hide
        Jeremias Maerki added a comment -

        No, that's not necessary. I just needed confirmation that it's a typo. Thanks.
        I'll handle the patch tomorrow.

        Show
        Jeremias Maerki added a comment - No, that's not necessary. I just needed confirmation that it's a typo. Thanks. I'll handle the patch tomorrow.
        Hide
        Jeremias Maerki added a comment -

        Patch applied! Finally. Thanks a lot for your work and patience, Peter.
        http://svn.apache.org/viewvc?view=rev&rev=474225

        While testing I found that CMYK color referencing an CMYK color profile are
        generated in the DeviceCMYK color space and the ICC profile is not embedded. I
        have yet to take a closer look at that one.

        Show
        Jeremias Maerki added a comment - Patch applied! Finally. Thanks a lot for your work and patience, Peter. http://svn.apache.org/viewvc?view=rev&rev=474225 While testing I found that CMYK color referencing an CMYK color profile are generated in the DeviceCMYK color space and the ICC profile is not embedded. I have yet to take a closer look at that one.
        Hide
        Peter Coppens added a comment -

        > While testing I found that CMYK color referencing an CMYK color profile are
        > generated in the DeviceCMYK color space and the ICC profile is not embedded. I
        > have yet to take a closer look at that one.

        Could you provide the test. I would be happy to take a look at this later this week.

        Show
        Peter Coppens added a comment - > While testing I found that CMYK color referencing an CMYK color profile are > generated in the DeviceCMYK color space and the ICC profile is not embedded. I > have yet to take a closer look at that one. Could you provide the test. I would be happy to take a look at this later this week.
        Hide
        Jeremias Maerki added a comment -

        You can get the color profile for color-sandbox.fo here:
        http://www.eci.org/eci/en/060_downloads.php
        (download ECI_Offset_2004.zip)

        Show
        Jeremias Maerki added a comment - You can get the color profile for color-sandbox.fo here: http://www.eci.org/eci/en/060_downloads.php (download ECI_Offset_2004.zip)
        Hide
        Jeremias Maerki added a comment -

        Attachment color-sandbox.fo has been added with description: Example using CMYK colors with a CMYK color profile

        Show
        Jeremias Maerki added a comment - Attachment color-sandbox.fo has been added with description: Example using CMYK colors with a CMYK color profile
        Hide
        Peter Coppens added a comment -

        The problem is in the PDFColor constructor

        Replacing the line
        if(cs!=null && cs.getType()==ColorSpace.TYPE_CMYK) {
        with
        if(cs!=null && cs instanceof CMYKColorSpace) {

        fixes the issue I think.

        Not entirely sure this is the most elegant/performant/full proof solution.

        Btw, sorry for the low quality of this patch (not having any real sponsors made
        me take shortcuts, like not making any unit tests). Always wrong.

        Anyway, should I submit a "real" fop patch for this or is this entry sufficient?

        Show
        Peter Coppens added a comment - The problem is in the PDFColor constructor Replacing the line if(cs!=null && cs.getType()==ColorSpace.TYPE_CMYK) { with if(cs!=null && cs instanceof CMYKColorSpace) { fixes the issue I think. Not entirely sure this is the most elegant/performant/full proof solution. Btw, sorry for the low quality of this patch (not having any real sponsors made me take shortcuts, like not making any unit tests). Always wrong. Anyway, should I submit a "real" fop patch for this or is this entry sufficient?
        Hide
        Jeremias Maerki added a comment -

        Sorry, Peter, I didn't want to imply that your patch was of low quality. It was
        actually very high quality. Just because I had to touch up the code a little
        concerning our code conventions doesn't make your patch bad. I was very happy
        with your patch and was only logging my changes to your original patch. If you
        can match our Java convention for the next patch, that's of course a good thing
        since it will save me time to go over it.

        No need to send a patch just to change one line. I'll look at it.

        Show
        Jeremias Maerki added a comment - Sorry, Peter, I didn't want to imply that your patch was of low quality. It was actually very high quality. Just because I had to touch up the code a little concerning our code conventions doesn't make your patch bad. I was very happy with your patch and was only logging my changes to your original patch. If you can match our Java convention for the next patch, that's of course a good thing since it will save me time to go over it. No need to send a patch just to change one line. I'll look at it.
        Hide
        Jeremias Maerki added a comment -
        Show
        Jeremias Maerki added a comment - Fix committed: http://svn.apache.org/viewvc?view=rev&rev=474327 Thanks!
        Hide
        Glenn Adams added a comment -

        batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed

        Show
        Glenn Adams added a comment - batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed

          People

          • Assignee:
            fop-dev
            Reporter:
            Peter Coppens
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development