Re: [Gimp-developer] the Gimp lcms.c plug-in

[Elle Stone <l elle stone gmail com>]

On 7/24/12, Michael Schumacher <schumaml gmx de> wrote:
> there are two bug reports in Bugzilla about changes in GIMP related to
> lcms2:
>
> This one introduces lcms2 to get support for ICC V4 profiles:
> https://bugzilla.gnome.org/show_bug.cgi?id=662739
>
>
> That one has been created to move everything to lcms2 to counter some
> performance problems:
> https://bugzilla.gnome.org/show_bug.cgi?id=675558
>
>
> I'm not sure if you are aware of them. Both have patches attached, and
> you should check if any of them - in particular the latter - is a
> duplication of your efforts.

Patches compared:
I've checked both patches, line for line comparing each patch to the
code changes that I've made so far to convert the Gimp lcms.c plug-in
to use lcms2.

Both patches cover *all*  the source code files that are affected by a
port from lcms to lcms2 (not just the lcms.c file itself). So far I've
only worked on the actual lcms.c plug-in code. Before working on the
other files myself, I want to add "printf" statements to the other
relevant source code files so I can get a better idea of how all of
Gimp color management hangs together. In particular, I want to track
down where/why Gimp doesn't offer the user the option of using or not
using black point compensation with the user-chosen monitor
profile/conversion intent.

Both patches are similar to what I've written so far. The second patch
lcms.c code is very, very close to what I've written and the first
patch already does some of what was on my "to do" list:

1. Retrieving  textual information:
The first patch code replaces the three existing information
collecting functons (that all actually produce the same information),
and adds code that collects additional information (model, copyright,
etc). The second patch and my own code both use a "three to one"
workaround (keeping more of the original code intact, not necessarily
a plus, but perhaps a safer first move). Also the first patch directly
uses the lcms2 UTF8 information, rather than getting the ascii
information and passing it to Gimp to change to UTF8 - a definite
plus, it seems to me.

2. Error Handling (bad profiles, lcms errors in conversions, etc):
The first patch keeps the existing non-functioning error code
reporting intact. The second patch and my own code both eliminate the
non-functioning error code. Ideally there should be some error code
reporting/some way to detect and deal with bad profiles. But based on
testing as well as examining the code, the existing code doesn't do
anything at all. I have located examples of functioning lcms1 and
presumably functioning lcms2 error handling, but haven't tried to
incorporate same.

3. Checksum:
Both patches update to lcms2 some MD5 code that I've temporarily
commented out. I commented the code out partly because I hadn't found
the right code to replace the old code, and mostly because I don't
understand the point of calculating the MD5 of the profile header (to
make sure you still have the profile you just opened??). See functions
"lcms_calculate_checksum" and the no doubt related
"lcms_sRGB_checksum". What is a "digest"? Why calculate the MD5 of the
profile header?

The next steps:
Based on the code, both patches look like they would probably work
just fine. My next step is to actually apply the patches (separately,
of course) and see if/how well the patches work in practice. And then
pick and choose the best bits of each patch. It will be several days
(at least) before I finish this next step.

Bug fix? enhancement?
It seems to me that porting to use lcms2 is both a fix for some bugs
(wrong results when converting from a linear gamma to the Gimp sRGB
built in profile; inability to read V4 profiles, which are becoming
more and more common), and also an enhancement in terms of speed,
based on reports by others. And of course lcms2 is a prerequisite for
32-bit floating point ICC profile conversions.

Alexandre, thanks! for the link to the upcoming lcms2 changes
regarding supported floating point variants.