Closed
Bug 460629
Opened 17 years ago
Closed 16 years ago
system Display profile should be ignored/modified if R+G+B does not equal white
Categories
(Core :: Graphics: Color Management, defect)
Core
Graphics: Color Management
Tracking
()
RESOLVED
FIXED
mozilla1.9.1b3
People
(Reporter: bugzilla, Assigned: bholley)
References
Details
(Keywords: fixed1.9.1)
Attachments
(3 files, 3 obsolete files)
616 bytes,
application/octet-stream
|
Details | |
5.54 KB,
patch
|
vlad
:
review+
beltzner
:
approval1.9.1+
|
Details | Diff | Splinter Review |
5.66 KB,
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•17 years ago
|
Reporter | ||
Comment 1•17 years ago
|
||
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 | ||
Updated•17 years ago
|
Updated•17 years ago
|
Assignee: bholley → nobody
Product: Firefox → Core
QA Contact: general → general
Target Milestone: Firefox 3.1 → ---
Assignee | ||
Updated•17 years ago
|
Assignee: nobody → bholley
Assignee | ||
Updated•16 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 2•16 years ago
|
||
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.
Reporter | ||
Comment 3•16 years ago
|
||
(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.
Assignee | ||
Comment 4•16 years ago
|
||
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.
Reporter | ||
Comment 5•16 years ago
|
||
(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?
Reporter | ||
Comment 6•16 years ago
|
||
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).
Assignee | ||
Comment 7•16 years ago
|
||
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.
Reporter | ||
Comment 8•16 years ago
|
||
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.
Assignee | ||
Comment 9•16 years ago
|
||
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.
Reporter | ||
Comment 10•16 years ago
|
||
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
Reporter | ||
Comment 11•16 years ago
|
||
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.
Assignee | ||
Comment 12•16 years ago
|
||
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
Assignee | ||
Comment 13•16 years ago
|
||
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?
Reporter | ||
Comment 14•16 years ago
|
||
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.
Assignee | ||
Comment 15•16 years ago
|
||
Ok - sounds good.
Uploading a patch without the fprintf, flagging vlad for review.
Attachment #345981 -
Attachment is obsolete: true
Attachment #345993 -
Flags: review?(vladimir)
Attachment #345993 -
Flags: review?(vladimir) → review+
Assignee | ||
Comment 16•16 years ago
|
||
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 17•16 years ago
|
||
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+
Assignee | ||
Comment 18•16 years ago
|
||
Added an identical patch with a user and commit message set for metered checkin.
Assignee | ||
Comment 19•16 years ago
|
||
Screwed up the username string. Fixed.
Attachment #350412 -
Attachment is obsolete: true
Comment 20•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
Comment 21•16 years ago
|
||
Backed out to investigate bug 467102:
http://hg.mozilla.org/mozilla-central/rev/a6d8a0bcac50
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 22•16 years ago
|
||
See bug 467102 comment 6 for an explanation about the perf regression.
Assignee | ||
Comment 23•16 years ago
|
||
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 → ---
Assignee | ||
Comment 24•16 years ago
|
||
Resolving as fixed - marked as needing 191 landing
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Whiteboard: [needs 191 landing]
Assignee | ||
Comment 26•16 years ago
|
||
Sorry for the cryptic comment - that meant to say:
Pushed to releases/mozilla-1.9.1 in 5e280f6121b1
Updated•16 years ago
|
Keywords: fixed1.9.1
Target Milestone: --- → mozilla1.9.1b3
Updated•16 years ago
|
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
Updated•16 years ago
|
Attachment #350415 -
Attachment description: MQ formatted, proper username. [Check Me In] → MQ formatted, proper username
[Checkin: Comment 23 & 26]
Comment 29•16 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•