Closed Bug 460629 Opened 12 years ago Closed 12 years ago

system Display profile should be ignored/modified if R+G+B does not equal white

Categories

(Core :: GFX: Color Management, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.1b3

People

(Reporter: bugzilla, Assigned: bholley)

References

Details

(Keywords: fixed1.9.1)

Attachments

(3 files, 3 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

The XYZs for red, green, and blue primaries should equal D50 white, if the profile is correctly built per the ICC spec. A display with a white point other than D50 has primaries based on actual display color temperature, and need to be chromatically adapted to D50. If this is not done, currently OS's will not reject the profile as invalid, and applications including FireFox will display white as something other than white. See Bug 450923.

The formula is to add up the X values for red, green and blue primaries in the display profile; Y values for red, green, and blue, and the Z values for red, green and blue. The resulting XYZ value should be: 0.96420, 1.00000, 0.82491.

Reproducible: Always

Steps to Reproduce:
gfx.color_management.mode 1 (easier to test, also occurs with 2)
set attached display profile as the current display profile


Actual Results:  
content that should display white (255,255,255) are reproduced as yellow; overall there is a yellow shift.

Expected Results:  
content that should reproduce white (255,255,255) should display white.
Blocks: 45077
Version: unspecified → 3.0 Branch
Blocks: 455077
No longer blocks: 45077
this profile aids in exhibiting the problem described in the bug. its RGB primaries are not correctly adapted. When added, they come to the 6500K white, not D50, therefore white always appears yellowish on screen when it is used, and color management is enabled.
Assignee: nobody → bholley
Blocks: 455056
No longer blocks: 455077
Target Milestone: --- → Firefox 3.1
Version: 3.0 Branch → Trunk
Assignee: bholley → nobody
Product: Firefox → Core
QA Contact: general → general
Target Milestone: Firefox 3.1 → ---
Assignee: nobody → bholley
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Added a patch to detect bogus profiles.

Builds with this patch are at https://build.mozilla.org/tryserver-builds/2008-10-29_19:33-bobbyholley@stanford.edu-bholleycmsdetectbogus3/

If you run one of these builds from the console, it should say "Output color profile looks bogus - using sRGB" if the detection comes out positive. The detection happens both on the system profile and custom profiles configured in gfx.color_management.display_profile in about:config, so it should be pretty easy to test.

Chris, can you grab one of the builds and give it a shot? It seems to work (rejects the attached ICC profile, accepts my monitor's profile and sRGB), but it would be good to check it on a range of profiles.
(In reply to comment #2)

> If you run one of these builds from the console, it should say "Output color
> profile looks bogus - using sRGB"

Suggest the following: "Display profile is invalid – using sRGB instead."

Of for more information: "Display profile is invalid (unadapted primaries) – using sRGB instead."

There are three device classes for ICC profiles: display, input, output. So I think the error should explicitly point out the problem is with a display profile. I'm OK with including either "color" or "ICC" as a qualifier.
As a clarification - the message is only to help testers of this patch/build determine whether this patch correctly identifies bogus profiles and resolves issue - it will be removed before this patch is landed in trunk.
(In reply to comment #2)
> Chris, can you grab one of the builds and give it a shot? It seems to work
> (rejects the attached ICC profile, accepts my monitor's profile and sRGB), but
> it would be good to check it on a range of profiles.

I'm not getting a message of any sort. Should I always get a message if I have a bad display profile selected? Or is there some other condition that needs to be fulfilled?
No message, but it appears to be working in that I'm not getting a white shift, and also test tagged content is still being color managed (i.e. the ICC web site's v4 test page works fine).
The message should print only in cases when a bad profile is selected.

Are you running firefox from the console? The message is just a printf, so you won't see it unless you launch it from a command line.
Ahh. OK Mac OS has a separate Console application that shows these kinds of messages as they get logged. I just launched it and I see the following entry...


11/2/08 4:20:13 PM [0x0-0x17d17d].org.mozilla.firefox[4883] Output color profile looks bogus. Using sRGB... 

Everything seems to be working as expected then.
Does it still seem to respect any valid profiles you have lying around? I just want to make sure I don't need to twiddle with the tolerance threshold (currently .01) to prevent legitimate profiles from being disabled.
Is is respecting valid profiles. But all of the ones I have, have exactly correctly adapted primaries.

If the primaries were adapted imprecisely, I can imagine some error is possible, but I can't imagine it would be much more than +- 0.002 in normalized XYZ.

Still, 0.01 is a pretty tight tolerance. Tighter than what Adobe is using (not more than 3 in any one channel in LAB space). The approximate equivalent would be:

+ 0.017460 for X
- 0.017252

+ 0.018108 for Y
- 0.017891

+ 0.037694 for Z
- 0.036580
So you could use 0.02 for X and Y, and .04 for Z and be close to, but slightly more tolerant than what Adobe is doing.
I've altered the patch per Chris' recommendations. A build is in the pipe, and I'll post a link to it here once it's finished.
Attachment #345437 - Attachment is obsolete: true
Builds came through, but the win32 builder is down. Mac and linux builds are here:
https://build.mozilla.org/tryserver-builds/2008-11-02_14:20-bobbyholley@stanford.edu-bholleycmsdetectbogus4/

Still work ok?
Still rejects sample bad profiles, and honors known good profiles. We could use a larger sample size, but I'm confident in the tolerance factor at this point, and in the solution.
Ok - sounds good.

Uploading a patch without the fprintf, flagging vlad for review.
Attachment #345981 - Attachment is obsolete: true
Attachment #345993 - Flags: review?(vladimir)
Comment on attachment 345993 [details] [diff] [review]
bogus profile detection patch - ready for review

Requesting approval 1.9.1.

This is a very important feature for LCMS, as it detects and disables bogus profiles, which are probably the #1 user complaint we hear when it comes to color management. Shipping without this could potentially cause a lot of people to garbled images when they have embedded profiles. We know that this patch detects a lot of the reference bogus profiles we have, so the biggest risk we run is disabling some legitimate profiles that exist out in the wild. We haven't yet encountered a legitimate profile that this code rejects, and it's unclear what that would even mean, since according to Chris a legitimate profile _must_ follow this criterion. The only subjective issue here is the tolerance/fuzzfactor that we allow. We chose one copied from adobe, which is probably a safe bet.
Attachment #345993 - Flags: approval1.9.1?
Comment on attachment 345993 [details] [diff] [review]
bogus profile detection patch - ready for review

a191=beltzner
Attachment #345993 - Flags: approval1.9.1? → approval1.9.1+
Added an identical patch with a user and commit message set for metered checkin.
Screwed up the username string. Fixed.
Attachment #350412 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/0586ee185c87
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
Backed out to investigate bug 467102:
http://hg.mozilla.org/mozilla-central/rev/a6d8a0bcac50
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
See bug 467102 comment 6 for an explanation about the perf regression.
pushed to mozilla-central in 31f8780c1963

We'll let this bake tonight, and hopefully we won't see that perf regression again (I was always a bit skeptical of it). Assuming it all looks good, I'll push to 191 tomorrow.
Component: General → GFX: Color Management
QA Contact: general → color-management
Target Milestone: mozilla1.9.1b3 → ---
Resolving as fixed - marked as needing 191 landing
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Whiteboard: [needs 191 landing]
5e280f6121b1
Whiteboard: [needs 191 landing]
Sorry for the cryptic comment - that meant to say:

Pushed to releases/mozilla-1.9.1 in 5e280f6121b1
Duplicate of this bug: 450923
Keywords: fixed1.9.1
Target Milestone: --- → mozilla1.9.1b3
Attachment #350412 - Attachment description: Same final patch as above, Modified with user/commit message. [Check me in]. → Same final patch as above, Modified with user/commit message
Attachment #350415 - Attachment description: MQ formatted, proper username. [Check Me In] → MQ formatted, proper username [Checkin: Comment 23 & 26]
Duplicate of this bug: 468589
in bug #450923 Bobby asked: 

"This should be fixed in the 3.2 nightlies already, and in the 3.1 nightlies
once they roll tonight. Can someone confirm that this is the case? If so, I'll
close this bug."

I see the bug with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b1) Gecko/20081007 Firefox/3.1b1

I don't see it with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20081211 Minefield/3.2a1pre

Hope that helps.
Duplicate of this bug: 479358
Duplicate of this bug: 469315
Duplicate of this bug: 487840
Duplicate of this bug: 497581
You need to log in before you can comment on or make changes to this bug.