Last Comment Bug 463221 - color reftesting
: color reftesting
Status: RESOLVED INCOMPLETE
[relnote - see comment 17 and 18]
: relnote
Product: Core
Classification: Components
Component: GFX: Color Management (show other bugs)
: Trunk
: All All
: -- minor with 19 votes (vote)
: ---
Assigned To: Zadkiel
:
Mentors:
Depends on:
Blocks: 455056
  Show dependency treegraph
 
Reported: 2008-11-05 07:48 PST by Zadkiel
Modified: 2011-11-25 11:58 PST (History)
22 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
colour reftests (842.42 KB, patch)
2008-11-09 05:04 PST, Zadkiel
no flags Details | Diff | Review
Colour Profile Tests Updated 30Nov08 (837.43 KB, patch)
2008-11-30 08:46 PST, Zadkiel
no flags Details | Diff | Review
Updated colour reftest (34.10 KB, patch)
2009-01-14 21:25 PST, Zadkiel
bobbyholley: review+
jmuizelaar: review-
Details | Diff | Review

Description Zadkiel 2008-11-05 07:48:04 PST
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.3) Gecko/2008092414 Firefox/3.0.3
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.3) Gecko/2008092414 Firefox/3.0.3

set of color reftests

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Comment 2 cmtalbert 2008-11-05 17:07:25 PST
Confirming this bug.  Zad, can we get these as a patch so that they are easier to peruse?

https://developer.mozilla.org/en/Creating_a_patch
Comment 3 Zadkiel 2008-11-09 05:04:01 PST
Created attachment 347152 [details] [diff] [review]
colour reftests
Comment 4 Justin Dolske [:Dolske] 2008-11-15 08:45:47 PST
Yay, reftests!

A few tiny drive-by nits:

"pngsuite" is actually an official PNG test suite, so these should go under a different name. Say, modules/libpr0n/test/reftest/png/color-profile/.

A single reftest.list manifest is fine, no need to break it into small chunks. The readme could just be distilled down into plain test, and rolled into the manifest as brief comments.
Comment 5 Bobby Holley (PTO through June 13) 2008-11-15 14:12:12 PST
Zad - did you ever figure out what was going on with 459617?
Comment 6 Zadkiel 2008-11-30 08:46:59 PST
Created attachment 350663 [details] [diff] [review]
Colour Profile Tests Updated 30Nov08

made the changes
Comment 7 Bobby Holley (PTO through June 13) 2008-12-03 16:27:13 PST
Zad, is one of these still failing on linux? If so, I can't r+ this until the issue is either discovered or it's marked fail-on-linux in the manifest so it doesn't break the build.
Comment 8 Zadkiel 2009-01-14 21:25:38 PST
Created attachment 357093 [details] [diff] [review]
Updated colour reftest

if Bug 456028 and Bug 459617 is ok this test should pass.
Comment 9 Zadkiel 2009-01-15 17:46:05 PST
Comment on attachment 357093 [details] [diff] [review]
Updated colour reftest

updated test.
Comment 10 Bobby Holley (PTO through June 13) 2009-01-19 23:09:24 PST
Hi Zad - I just landed bug 456028, so assuming it doesn't get backed out for some reason, you should test these reftests with tonights nightly builds and make sure they're passing on linux. Once you do that, confirm this over on bug 459617. Once that's resolved, I'll r+ this patch.
Comment 11 Bobby Holley (PTO through June 13) 2009-01-21 20:39:19 PST
Comment on attachment 357093 [details] [diff] [review]
Updated colour reftest

Looks ok to me. r=bholley assuming these pass. Also, I take it that you decided to scale the testing down from your previous patch?

Better have dolske look at it too.
Comment 12 Justin Dolske [:Dolske] 2009-05-06 16:13:35 PDT
Comment on attachment 357093 [details] [diff] [review]
Updated colour reftest

Better to have Jeff look at these, given the new qcms hotness.
Comment 13 Jeff Muizelaar [:jrmuizel] 2009-05-07 07:36:17 PDT
Comment on attachment 357093 [details] [diff] [review]
Updated colour reftest

This tests should go in modules/libpr0n/test/reftest/color-management/

It might be better to keep all of the pngs in the same directory and use the naming like: row1col1-AdobeRGB1998.png and row1col1-AdobeRGB1998-ref.png to be consistent with the other tests in that directory. 

It would also be good to have a description of what the tests are actually testing in the reftest.list file.
Comment 14 Mardeg 2009-06-04 19:32:52 PDT
I'm sure http://www.color.org/version4html.xalter was working in Firefox 3.1b3 so I don't know why it isn't working in 3.5b4 or trunk nightlies? Is this a regression not caught by the reftests?
If this is just something that changed at the site, then sorry for spamming the bug, but if not we need help finding a regression range.
Comment 15 Jeff Muizelaar [:jrmuizel] 2009-06-04 19:51:29 PDT
(In reply to comment #14)
> I'm sure http://www.color.org/version4html.xalter was working in Firefox 3.1b3
> so I don't know why it isn't working in 3.5b4 or trunk nightlies? Is this a
> regression not caught by the reftests?
> If this is just something that changed at the site, then sorry for spamming the
> bug, but if not we need help finding a regression range.

qcms doesn't have icc-v4 support and was introduced in b4 so this is expected behavior.
Comment 16 Mike Shaver (:shaver -- probably not reading bugmail closely) 2009-06-05 06:29:21 PDT
Is that regression (icc-v4 support) one that we want to take?  I hadn't understood that to be part of the plan, but I may not be recalling it.
Comment 17 Jeff Muizelaar [:jrmuizel] 2009-06-05 06:34:33 PDT
Yes, regressing icc-v4 was part of the plan.

Here's the plan Vlad gave:
"icc2 only, handling colorants, tone reproduction curves, and possibly white point, along with perceptual rendering intent (and possibly relative colorimetric, skipping absolute colorimetric)
Comment 18 Mike Shaver (:shaver -- probably not reading bugmail closely) 2009-06-05 06:36:09 PDT
OK, so for those of us who don't know everything that lcms supported, what exactly now in the unsupported list, when compared to the stuff that we supported in FF3?  That'll be important to relnote, at least.
Comment 19 Jeff Muizelaar [:jrmuizel] 2009-06-25 12:49:18 PDT
Here's a list of some of the things that are not supported by qcms compared to lcms for the release notes:

- Profile colorspaces other than RGB (i.e. CMYK)
- Relative colorimetric and absolute colorimetric rendering intents
- ICCv4
- White point correction
Comment 20 Marti 2009-07-11 11:25:20 PDT
I would rather add:

- CLUT based profiles 
- Black point compensation
- devicelink profiles
- Precedence of CLUT/Matrix-Shaper. 
- Lab encoding artifact handling. 
- Scum dot removal (white should match white).
- Gray profiles operating in Lab
- etc.

The only compelling feature for full v2 support is CLUT and tetrahedrical interpolation. If you also plan to support v4 there are many other things...
Comment 21 Michael 2009-10-15 08:17:10 PDT
Sorry to butt in here - please please please reinstate icc v4 colour correction!  v2 makes some of my photos look horrible.  Lots of other apps have v4 colour correction.....
Comment 22 ruben.nesvadba 2010-02-04 05:28:13 PST
Could somebody please explain what's against not fully supporting ICC v4? Is it the file size of the Firefox install executable?
Comment 23 d 2010-02-04 06:17:10 PST
(In reply to comment #22)
> Could somebody please explain what's against not fully supporting ICC v4? Is it
> the file size of the Firefox install executable?

The size of the executable is not a problem, but I think the issue is simply that no one has written a working patch. When that is done, it will probably be checked in.
Comment 24 Marti 2010-02-05 06:26:24 PST
That "working patch" would be of several thousands of lines of code. Supporting ICC V4 as well as fully supporting ICC V2 is not an easy task. The actual qcms code only implements a very narrow subset of ICC V2, this takes few code that can be (and it is, actually) optimized to speed up things.
Comment 25 ruben.nesvadba 2010-02-05 13:46:45 PST
(In reply to comment #24)
> That "working patch" would be of several thousands of lines of code. Supporting
> ICC V4 as well as fully supporting ICC V2 is not an easy task. The actual qcms
> code only implements a very narrow subset of ICC V2, this takes few code that
> can be (and it is, actually) optimized to speed up things.

It seems to me that it wouldn't be a bad idea to make this issue clear at these two bugpages as well:

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

https://bugzilla.mozilla.org/show_bug.cgi?id=515958
Comment 26 ruben.nesvadba 2010-02-09 03:28:45 PST
(In reply to comment #24)
> That "working patch" would be of several thousands of lines of code. Supporting
> ICC V4 as well as fully supporting ICC V2 is not an easy task. The actual qcms
> code only implements a very narrow subset of ICC V2, this takes few code that
> can be (and it is, actually) optimized to speed up things.

Can't you just copy the way Webkit (Safari) handles this stuff?
Comment 27 chris 2010-05-15 03:31:36 PDT
(In reply to comment #26)
> Can't you just copy the way Webkit (Safari) handles this stuff?

Safari has ICC v.4 support. IE9 preview 2 has ICC v.4 support. Color profiling tools generate ICC v.4 profiles. Image editing tools like Photoshop use ICC v.4 

Firefox 3.1, briefly, had ICC v.4 support (added but disabled in 3.0, removed due to the switch from lcms to qcms). Thus, some users are sticking to 3.1 while some (which switched to Firefox because of ICC support in 3.1) are contemplating switching to IE9
http://support.mozilla.com/en-US/forum/1/592216

Still listed as an issue in release notes for Firefox 3.5
http://www.mozilla.com/en-US/firefox/3.5/releasenotes/
Not listed for 3.6, leading some to hope that it was fixed
http://www.mozilla.com/en-US/firefox/3.6/releasenotes/
not clear why 3.6 release notes didn't list it; the issue is ongoing.

So yes, the fact that Firefox currently has partial ICC v.2 support and no ICC v.4 support is a significant issue.
Comment 28 Michael Marquardt 2010-07-12 03:05:23 PDT
Since qcms dropped significant functionality, why not just revert to lcms usage? Yes, it is slower, but this only has to run on tagged images within a page, which is in the minority. Would it really cause an appreciable performance penalty to revert to the much more complete lcms? 

For anyone interested in completely re-inventing the wheel and bringing qcms up to lcms standards with the missing functionality (non-RGB color support + ICCv4 support) current specs for ICCv4 (and 4.2) are here: http://www.color.org/icc_specs2.xalter
Comment 29 ruben.nesvadba 2010-07-13 12:29:40 PDT
copy pasted from lcms 2 blog from somebody called Marti Maria:

"Tuesday, April 20, 2010
LittleCMS 2.0 release date 
Beta 3 seems stable enough, so now I can set a final release date for lcms2.
LittleCMS 2.0 will hopefully be released on Saturday, May-8-2010
So, if you have found a bug, inconsistency, etc. Please let me know ASAP. Code is now frozen and I will start the final qualification phase. All change requests will be studied before commit, just to minimize risk.  Thanks again to all people that has contributed to this project! 
Posted by Marti Maria at 11:46 AM 0 comments  
Labels: release date"
Comment 30 ruben.nesvadba 2010-07-13 12:34:54 PDT
Oh wait, that person is on this page:P
Comment 31 Joe Drew (not getting mail) 2011-11-25 11:58:36 PST
This bug has become useless. People seem to be using it to advocate for ICCv4. I don't think Zadkiel is working on his colour correction reftests any more, so let's just close this out. Zadkiel, feel free to reopen this or open a new bug if you want to fix up the reftests.

Note You need to log in before you can comment on or make changes to this bug.