Closed Bug 538114 Opened 15 years ago Closed 12 years ago

qcms doesn't support LUT ICCv2 profiles (both LAB & XYZ)

Categories

(Core :: Graphics: Color Management, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: cyber.spamage, Assigned: BenWa)

References

Details

Attachments

(6 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.7) Gecko/20091221 Firefox/3.5.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.7) Gecko/20091221 Firefox/3.5.7

qcms doesn't support LUT ICCv2 profiles (both LAB & XYZ).

If given a LAB LUT, Firefox ignores the profile.

If given a XYZ LUT profile without Matrix tags (rXYZ, bXYZ, gXYZ, rTRC, bTRC, gTRC), Firefox ignores the profile.

If given a XYZ LUT profile with Matrix tags (rXYZ, bXYZ, gXYZ, rTRC, bTRC, gTRC), Firefox ignores the LUT tags and uses the Matrix tags instead.

Reproducible: Always




Fixing this bug should also resolve Bug 509710: https://bugzilla.mozilla.org/show_bug.cgi?id=509710
Version: unspecified → Trunk
ICCv2 XYZ LUT with Dummy Matrix Tags (Red and Green Channels Swapped)
This shows how qcms uses Matrix Tags even when LUT tags are available.
See Also: → 509710
Attached file ICCv2 LAB LUT
This should mostly be used for inspiration because I don't have time to finish it right now.
Assignee: nobody → bgirard
Attached patch LUT SupportSplinter Review
This patch continues the work done by Jeff.

There's still a few things about style, naming, data structure that need to be reviewed and decided but the functionality is there. The patch currently uses LUT over tone reproduction curves.

This patch adds support for tri-linear and tetrahedral interpolation for Color Lookup Tables (CLUT).

With the patch applied I get similar result as lcms/ColorSync for the profiles listed here.
Attachment #427624 - Flags: review?(jmuizelaar)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Thank you for working on this Benoit.

Out of curiosity, what happens if you feed qcms an ICC v4 LUT profile with this patch applied? Any difference from the current behavior?
https://bugzilla.mozilla.org/show_bug.cgi?id=488800
The LUT is used when it is of type 'mftX' (multi-function table with X-byte precision) and the results are nearly identical. When the LUT is of type 'mAB' (Multi-function A-to-B Table) it is ignored and the tone reproduction curves are used if possible. I will be looking into support these LUT types soon.

I will also need to update the patch to support B2A0 conversion for LUT tables as well.
A couple of quick comments:
 - the change to clamp_u8 should probably be in a separate patch
 - the tetrahedral interpolation code should copy the note about which algorithm is used from lcms and it should reference that it was copied from lcms. (assuming it was). It could also probably use some more documentation about how it works.
 - no need to use a macro for f_1 instead of an inline function
 - the parenthesis should be close to the condition and not the if:
+		if( in_r >= 0.5 ) in_r = 0.5;
+		if( in_g >= 0.5 ) in_g = 0.5;
+		if( in_b >= 0.5 ) in_b = 0.5;

 - I'm not thrilled with the name 'module' but don't have a better suggestion yet.
 - split out the changes to colorsync-perf

Overall it looks good though. A more thorough review is to come.
Benoit and/or Jeff do either of you have any idea if the Interlaced PNG & Progressive JPEG (multi-pass rendered images) dead-slow performance problem with qcms is ever going to be fixed?

https://bugzilla.mozilla.org/show_bug.cgi?id=435628

I filed the bug back in 2008, but it seems to have been forgotten about. Right before the Firefox 3.5 release I even brought it to Mike Beltzner's attention and confirmed that the problem still occurs with qcms, but there has been only silence since. Is there some other reason why it hasn't been worked on, or is it just an issue that nobody has volunteered?

Any chance that special case handling of multi-pass images for better performance could be worked into any upcoming qcms patches?
Attachment #427624 - Flags: review?(jmuizelaar) → review-
Any word on this? Looks like the thorough review never happened.
This, along with additional changes to qcms, will be reviewed after the release of FF 4.0
Benoit, once Fx 4.0 launches, could you make available a test build with the current state of your qcms lut patch?

I ask because it sounds like the functionality is there and working, but seeing as things go, we may still be looking at many months till the patch is finalized/approved/landed, let alone appear (hopefully) in a 4.x point release.
Yes, unfortunately the previous build expired. I should off archived it.
@Benoit Girard

Reminder for a new tryserver build now that Firefox 4 is released.
I'm going to have some free time to work on this in May, I hope to get it on the radar then since most of the work is done if reviews go well.
There are new builds up at:

https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/b56girard@gmail.com-661b4dd715a4/

You'll need to set the gfx.color_management.force_v4 preference to true.
Great, so the above build contains initial support for ICCv4 as well?
https://bugzilla.mozilla.org/show_bug.cgi?id=488800

I'm a bit confused though. Do we still need to set gfx.color_management.force_v4 to true if we have a ICCv2 LUT profile?

Forcing ICCv4 doesn't sound like a good idea when ICCv2 profiles are handled fundamentally differently. Does the above build workaround the incompatibility between ICCv2 and ICCv4 profiles somehow?
force_v4 just enables v4 support. The pref could probably use a better name.
Once we are ready this preference will become default. This will likely depend on the result of the feedback and testing this gets.

(In reply to comment #19)
> Forcing ICCv4 doesn't sound like a good idea when ICCv2 profiles are handled
> fundamentally differently. Does the above build workaround the
> incompatibility between ICCv2 and ICCv4 profiles somehow?

We now, with the pref, default to using CLUT if both CLUT and TRC are present in profiles of any version. Is this what you mean?
(In reply to comment #21)
> We now, with the pref, default to using CLUT if both CLUT and TRC are
> present in profiles of any version. Is this what you mean?

Not exactly, but on that subject, qcms should always be using the CLUT by default when both CLUT & TRC are present, and it shouldn't be connected to an ICCv4 preference.

(In reply to comment #19)
> Forcing ICCv4 doesn't sound like a good idea when ICCv2 profiles are handled
> fundamentally differently. Does the above build workaround the
> incompatibility between ICCv2 and ICCv4 profiles somehow?

What I was talking about here was how ICCv2 profiles and ICCv4 profile normally shouldn't be mixed and matched. ICCv2 monitor profiles should be used only with ICCv2 color spaces and ICCv2 embedded image profiles. ICCv4 monitor profiles should be used only with ICCv4 color spaces and ICCv4 embedded image profiles. Since ICCv2 & ICCv4 profiles handle color correction differently from each other, you could introduce error by mixing and matching them. I'm not an expert in ICC color profiles, but I'm relatively certain this is true. If Firefox should care about this is another matter. I don't think Photoshop cares, and basically just expects users to know to use a ICCv4 workflow when using ICCv4 profiles.


Somewhat off-topic, but is there any way to see the currently active (auto-detected) ICC profile in Firefox? If not, that seems like something that belongs on about:support (or maybe create an about:qcms) which contains the name of the profile, whether qcms thinks it has a valid profile, if the profile is ICCv2 or ICCv4, and if qcms is using TRC or CLUT from the profile.
(In reply to comment #22)
> Not exactly, but on that subject, qcms should always be using the CLUT by
> default when both CLUT & TRC are present, and it shouldn't be connected to
> an ICCv4 preference.
> 

I understand but we don't plan in having qcms operate in two modes (v2, v4) but rather just have this preference by default.

> Somewhat off-topic, but is there any way to see the currently active
> (auto-detected) ICC profile in Firefox? If not, that seems like something
> that belongs on about:support (or maybe create an about:qcms) which contains
> the name of the profile, whether qcms thinks it has a valid profile, if the
> profile is ICCv2 or ICCv4, and if qcms is using TRC or CLUT from the profile.

This is something we would really like, as well as showing the profile of images when click 'View Info' but the work on this has not been started.
The new ICC LUT support isn't compatible with LAB profiles and gfx.color_management.mode 1. A bit disappointing if not easily fixed, since I always use mode 1 to correct chrome elements and untagged content.

Major breakage with LAB profile with everything turning teal: http://img27.imageshack.us/img27/8324/colman2lab.png
Just to confirm that you're only seeing this if you pref on the new icc v4 support?

We will likely fix rendering intents before turning on this feature by default.
Yes, it only seems to happen with the ICCv4 pref enabled and a LAB LUT profile.

Over-saturated compared to Photoshop with a LAB profile:

Firefox (AdobeRGB tagged image):
http://img830.imageshack.us/img830/3230/firefoxlabrel.png

Photoshop CS4 (AdobeRGB tagged image):
http://img220.imageshack.us/img220/4177/pscs4labrel.png

I also hope you have some major code speed-ups in hiding or qcms (quick cms) will need to be renamed scms (slow cms) considering you are now around ~2.5x slower than lcms1 color management in Firefox 3... Unless you are able to at least match the LUT performance of lcms1 in Firefox 3, the performance impact makes this dangerous to enable by default, since the browser just hangs until processing is finished. It wouldn't hurt to give LUT support a separate about:config entry from ICCv4, so the decision could be made to ship ICCv4 in Fx8 with LUT support disabled by default.

2175x3200 AdobeRGB1998-tagged JPEG:
Firefox 3.0 using lcms1 (LAB monitor profile): ~8 seconds
Firefox 8.0a1pre using qcms (Matrix monitor profile): ~3 seconds
Firefox 8.0a1pre using qcms (LAB monitor profile): ~21 seconds
Photoshop CS4 (LAB monitor profile): ~1 second

I assume there is no chance of adding (optional?) lcms2.2 support back into Firefox? It's had some major speeds-up compared to version 1 which was in Firefox 3, and I'd be unsurprised if it's 2-3x as faster all-around than qcms at this point.
The image comparison in the previous post was with the default gfx.color_management.mode 2, in case it wasn't clear.
(In reply to Alex Firestone from comment #26)
> 2175x3200 AdobeRGB1998-tagged JPEG:
> Firefox 3.0 using lcms1 (LAB monitor profile): ~8 seconds
> Firefox 8.0a1pre using qcms (Matrix monitor profile): ~3 seconds
> Firefox 8.0a1pre using qcms (LAB monitor profile): ~21 seconds
> Photoshop CS4 (LAB monitor profile): ~1 second
> 

I had made brief performance testing and the results I got were different. We will address performance issues before turning this feature on by default.

> I assume there is no chance of adding (optional?) lcms2.2 support back into
> Firefox? It's had some major speeds-up compared to version 1 which was in
> Firefox 3, and I'd be unsurprised if it's 2-3x as faster all-around than
> qcms at this point.

I don't think Jeff or myself would have time to look into this.
(In reply to Alex Firestone from comment #26)
> 2175x3200 AdobeRGB1998-tagged JPEG:
> Firefox 3.0 using lcms1 (LAB monitor profile): ~8 seconds
> Firefox 8.0a1pre using qcms (Matrix monitor profile): ~3 seconds
> Firefox 8.0a1pre using qcms (LAB monitor profile): ~21 seconds
> Photoshop CS4 (LAB monitor profile): ~1 second


Please file a new bug about the performance problem.
Created Bug 679527 for the performance problem.
This feature was added to qcms but requires the v4 feature to be enabled. Closing as fixed but this will require bug 679875 to be on by default.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: