Closed Bug 463221 Opened 16 years ago Closed 13 years ago

color reftesting

Categories

(Core :: Graphics: Color Management, defect)

defect
Not set
minor

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: zadkiel.m, Assigned: zadkiel.m)

References

Details

(Keywords: relnote, Whiteboard: [relnote - see comment 17 and 18])

Attachments

(1 file, 2 obsolete files)

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.
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch colour reftests (obsolete) — Splinter Review
Component: General → ImageLib
Product: Firefox → Core
QA Contact: general → imagelib
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.
Zad - did you ever figure out what was going on with 459617?
Blocks: 455056
made the changes
Attachment #347152 - Attachment is obsolete: true
Attachment #350663 - Flags: review?(bholley)
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.
Component: ImageLib → GFX: Color Management
QA Contact: imagelib → color-management
Version: unspecified → Trunk
if Bug 456028 and Bug 459617 is ok this test should pass.
Attachment #350663 - Attachment is obsolete: true
Attachment #350663 - Flags: review?(bholley)
Comment on attachment 357093 [details] [diff] [review]
Updated colour reftest

updated test.
Attachment #357093 - Flags: review?(bholley)
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 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.
Attachment #357093 - Flags: review?(dolske)
Attachment #357093 - Flags: review?(bholley)
Attachment #357093 - Flags: review+
Assignee: nobody → zadkiel.m
Comment on attachment 357093 [details] [diff] [review]
Updated colour reftest

Better to have Jeff look at these, given the new qcms hotness.
Attachment #357093 - Flags: review?(dolske) → review?(jmuizelaar)
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.
Attachment #357093 - Flags: review?(jmuizelaar) → review-
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.
(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.
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.
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)
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.
Keywords: relnote
Whiteboard: [relnote - see comment 17 and 18]
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
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...
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.....
Could somebody please explain what's against not fully supporting ICC v4? Is it the file size of the Firefox install executable?
(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.
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.
(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
(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?
(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.
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
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"
Oh wait, that person is on this page:P
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.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.