Hi, I just added two patches at https://issues.apache.org/jira/browse/FOP-2564 which attempt to give more control over ICC profile handling. On commons side I added the ICC profile as a separate parameter to the ImageBuffered/ImageRendered constructors. This causes quite some code changes in other classes, but I think is nicer than just resetting the ICC profile after instantiation of those objects. On FOP side the change is much simpler: instead of always using the ICC profile provided by the encoding helper (which is only necessary if the color model actually changes), the ICC profile of the image is used (just as in AbstractImageAdapter.getEffectiveICCProfile()). With this new version of my fix, both image types (files with and without ICC profiles) are correctly processed.
BR, Matthias On 13.01.2016 09:26, Luis Bernardo wrote: > > Yes, that is what I was suspecting. My suggestion then is to do the > check when loading the image, as you propose. If there is no embedded > profile in the PNG do not embed one by default. We could make this > configurable but I don;t see the need for that until we find an > example where having the default profile embedded helps. > > > On Wed, Jan 13, 2016 at 1:19 PM, Matthias Reischenbacher > <matthias8...@gmx.at <mailto:matthias8...@gmx.at>> wrote: > > Hi, > > FOP-1575 seems to be similar, but isn't the same. I also can't > reproduce > it on my platform (win, jdk 8), so perhaps it has been fixed in the > meantime by some other change. > > Regarding your other questions: getEffectiveICCProfile() is always > called, regardless of embedded or non-embedded profiles. In fact I > just > tried with more sample PNG images (with and without embedded profiles) > and my fix is definitely wrong, because embedded ICC profiles for Gray > color space are also of type ICC_ProfileGray, so also embedded > profiles > are now being ignored by my change. I think my fix needs to be added > more deep down, probably in the image loader > (ImageLoaderImageIO.loadImage). Perhaps it would be a good thing > to mark > the Image there as having or not an embedded profile, so that the > ImageRendererAdapter can better decide, if the profiles should be > used. > I will create a jira issue and check in more detail. > > BR, > Matthias > > On 12.01.2016 19:27, Luis Bernardo wrote: > > > > Can you check whether FOP-1575 is the same problem? > > > > Is getEffectiveICCProfile() only called if there is no embedded > > profile? Could there be cases where that profile is in fact correct? > > > > On 1/12/16 9:25 PM, Matthias Reischenbacher wrote: > >> Hi, > >> > >> I've been struggling today with the PDF display of a PNG with > "gray" > >> color space. When using the default PNG loader (with the raw > PNG loader > >> all works fine), the colors are displayed very "light" and "dull". > >> During a debug session I found out that the ImageIO loader > assigns a > >> default Gray ICC profile (of type > java.awt.color.ICC_ProfileGray), if > >> the PNG file doesn't have an embedded ICC profile. So the > >> ImageRenderedAdapter.getEffectiveICCProfile method returns this > Java > >> "ICC_ProfileGray" profile, which is then embedded in the output > PDF. > >> Displaying the PNG with this profile, causes the "incorrect" color > >> display. > >> I'm aware that an ICC profile should be embedded in the PNG > file, in > >> order to have full control over color reproduction, but I'm > wondering if > >> it doesn't go a bit too far to output a profile, which is > internally > >> assigned by the JRE. As a quick fix I locally changed the > >> ImageRenderedAdapter.getEffectiveICCProfile method the > following way: > >> > >> protected ICC_Profile getEffectiveICCProfile() { > >> ColorSpace cs = getImageColorSpace(); > >> if (cs instanceof ICC_ColorSpace) { > >> ICC_ColorSpace iccSpace = (ICC_ColorSpace)cs; > >> return !(iccSpace.getProfile() instanceof > ICC_ProfileGray) ? > >> iccSpace.getProfile() : null; > >> } else { > >> return null; > >> } > >> } > >> > >> Please let me know, if anybody disagrees with this approach or > if it > >> should be handled somewhere else. > >> > >> Thanks, > >> Matthias > >> > > > > >
