Closed Bug 403169 Opened 17 years ago Closed 17 years ago

enabling color management (cms) causes unified toolbar color mismatch.

Categories

(Core Graveyard :: GFX, defect, P2)

x86
macOS

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Dolske, Assigned: joe)

References

Details

(Whiteboard: [has patch][has review josh][needs review stuart])

Attachments

(4 files, 1 obsolete file)

Attached image Screencap of color mismatch (deleted) —
With gfx.color_management.enabled enabled, the unified toolbar on OS X doesn't match the toolbar color. The toolbar background is slightly darker than the titlebar (or, vice versa, the titlebar is slightly lighter than the toolbar). When color management is off (as it is by default), the menubar/toolbar are the same color. My understanding is that it's unlikely color management will be default-enabled for FF3... But since bug 16769 is blocking and still open (presumably waiting for a decision for flipping the pref), I'll request blocking on this bug too.
Flags: blocking1.9?
Summary: color management (cms) alters causes unified toolbar color mismatch. → enabling color management (cms) causes unified toolbar color mismatch.
Assignee: nobody → cbarrett
If the theme was using a system css color to draw the lower portion of the toolbar, that should match - pav fixed things so that system css colors are not corrected.
Assuming there's a flag to check if color management is on or not, we can easily fix this -- initialize the color as calibrated instead of device. There might be (probably are) other places where we use device colors -- if we are going to turn on color management, we will want to fix those places as well (and may want to fix them anyway)
Flags: blocking1.9? → blocking1.9+
Priority: -- → P4
Priority: P4 → P3
There's a flag, on gfxPlatform -- we shouldn't be color correcting system colors, which we don't on win32, so we need to make sure the same thing happens on the mac.
Priority: P3 → P4
Actually from what I can gather, using calibrated colors the encouraged default for drawing (I had no idea). Our using device colors was because Gecko didn't, I'm guessing?
So, looking more closely, it seems to me more likely that this is what's happening: The colors that are set for the toolbox are color corrected sRGB -> output when drawn. The color set on the @titlebarcolor property is just used untransformed. I'm not entirely sure this is the right thing to do. I could see assuming all unknown colors are sRGB making sense for content, but does it really make sense for colors in chrome? I think it makes more sense to assume they are already in the output colorspace and do no correction. Seems this already came up as I heard (but can't find the bug) that "system color" were made to not be color corrected. Really, isn't the whole UI supposed to match with the system around it, not the content inside it?
(In reply to comment #5) > Seems this already came up as I heard (but can't find the bug) that "system > color" were made to not be color corrected. Really, isn't the whole UI supposed > to match with the system around it, not the content inside it? No what you'd really want is the UI to have a particular color appearance defined in a device independent color space, and cause the whole UI to be color managed so it would look consistent across display technologies. I do not know if Apple has done this themselves or documented it for OS 10.5. I'm reasonably certain this was the idea for Vista before Avalon was pulled from the initial release of Vista.
Attached patch fix v1.0 (obsolete) (deleted) — Splinter Review
I really don't think this is the correct way to fix this. When you flip on color correct, the title bar no longer matches the rest of the system in unified mode. Had a discussion with stuart on irc though, and he said the CSS spec says all untagged content should be assumed sRGB. I don't see why that has to apply to chrome, but whatever.
Attachment #290927 - Flags: review?(joshmoz)
Attachment #290927 - Flags: superreview?(pavlov)
Attachment #290927 - Flags: review?(joshmoz)
Attachment #290927 - Flags: review+
>When you flip on >color correct, the title bar no longer matches the rest of the system in >unified mode. Had a discussion with stuart on irc though, and he said the CSS >spec says all untagged content should be assumed sRGB. I don't see why that has >to apply to chrome, but whatever. We need the title bar color to match every other window on the system. Regardless of the CSS spec, not visually integrating with the OS is a pretty major polish problem.
(In reply to comment #8) > >When you flip on > >color correct, the title bar no longer matches the rest of the system in > >unified mode. Had a discussion with stuart on irc though, and he said the CSS > >spec says all untagged content should be assumed sRGB. I don't see why that has > >to apply to chrome, but whatever. > > We need the title bar color to match every other window on the system. > Regardless of the CSS spec, not visually integrating with the OS is a pretty > major polish problem. It isn't just the titlebar. All colors in chrome are affected. There is a patch that makes specific named system colors not color corrected, but if you are trying to match some other OS color, color correction will happen (which could cause things to look pretty annoying). Is there a way to tag colors in CSS as being in the monitor profile and not sRGB?
you certainly have the option to convert system colors if they're in monitor profile already to sRGB in nsLookAndFeel and then we can convert them back to the monitor profile later.
It seems like the color management pref is doing two distinct things: 1. making us honor color profiles in images, rather than assuming that all images are in the same colorspace (which, given (2), we could call sRGB) 2. making us convert colors in our internal color space (which CSS requires to be sRGB) into the correct color space for calls to the system. (At a code level, the two transformations are done at the same place in many cases, but there are still conceptually two different things.) Doing (2) has many similarities to bug 53597 (which was unfortunately backed out), and it's the part that's causing this problem. (I'm not entirely sure that the issue is conversion to the monitor profile -- it might just the conversion between sRGB and Mac's default gamma.) It really seems bad to have either of these be a pref -- as a platform, we should be consistent about whether we're doing these things and not make users of our platform (e.g., theme authors) deal with the inconsistency. So I think we should make firm decisions about these prefs for each release (i.e., potentially changing from off to on at some major release, now or in the future) and say that the opposite value is unsupported. But we also might want to separate the decisions about (1) and (2).
Attached image Sidebar colors (deleted) —
Note that this isn't just an issue with the unified toolbar (although that deviation is obvious), but a problem with all of the colors we use in chrome.
Whiteboard: [has patch][has review josh][needs review stuart]
The patch above is the correct fix for things as they stand now. One option might be a function or CSS constructor that does exactly what we do in nsILookAndFeel -- convert Monitor -> sRGB so when the sRGB -> Monitor transform is done later, the color comes out looking correctly. Exposing that to web content seems unnecessary, but it sure would be handy for chrome. Moving many of the system colors in to LookAndFeel is possible, but it doesn't help when you are using images, or are trying to pick a color to complement to an OS color. As dbaron pointed out, this would be easier if we were forcing color correction on. I wonder if the WebKit folks would have any suggestions about ways to handle the sRGB-nature of CSS and still get colors to look decent on Mac.
Attachment #290927 - Flags: superreview?(pavlov) → superreview+
>still get colors to look decent on Mac. Doesn't this issue effect every platform where we are doing color management?
No. This isn't a problem on Windows at least. I expect what is going on is that other platforms use the native nsLookAndFeel colors which are explicitly converted from monitor profile to sRGB and then back. If you're using explicit colors in the css for the new mac theme you're going to get hosed because they're not in a standard colorspace. You want to use nsLookAndFeel colors instead of explicit non-sRGB colors in chrome.
(In reply to comment #15) > No. This isn't a problem on Windows at least. > > I expect what is going on is that other platforms use the native nsLookAndFeel > colors which are explicitly converted from monitor profile to sRGB and then > back. If you're using explicit colors in the css for the new mac theme you're > going to get hosed because they're not in a standard colorspace. You want to > use nsLookAndFeel colors instead of explicit non-sRGB colors in chrome. If they're using nsLookAndFeel colors, which have already been sRGB->Monitor transformed, this patch will cause them to look wrong, no?
So do we want to land this and close it?
Well, if Proto switches over to using nsLookAndFeel colors where it wants to match the system, then this patch will in fact, break things, AIUI. I think that'd probably be the better solution (have proto use nsLookAndFeel colors where it wants to match the system), than doing something like this.
>I think that'd probably be the better solution (have proto use nsLookAndFeel >colors where it wants to match the system), than doing something like this. Ok, should we create a spin off bug for getting all the colors we need to use listed in nsLookAndFeel.
Priority: P4 → P3
Since toolbar/statusbar isn't just about color: Is it possible to paint the "polished metal" look with a gradient in nsNativeThemeCocoa.mm? If it is, how about a NS_THEME_METAL_BAR in nsNativeThemeCocoa.mm?
I've looked in many different places (including NSColor, the Appearance Manager and HITheme) and it doesn't appear there's any way to get the colour of the unified titlebar. Therefore, it might be a good idea to check in the existing patch. However, I'm willing to be proven wrong -- just point me to the documentation. :)
We set a color for the non-unified titlebar in nsCocoaWindow.mm (line 1570 etc). I suppose one could use that combined with painting a gradient on the toolbar in nsNativeThemeCocoa.mm (by using CGContextDrawShading or something), no?
Assignee: mozcbarrett → joe
The colour needs to be colour-corrected regardless. Alex, Colin, Stuart: are you OK with the idea of checking in the patch in this bug?
>Alex, Colin, Stuart: are >you OK with the idea of checking in the patch in this bug? since color management is going to be off by default, this issue isn't huge, but someone else should comment on if we want to land the patch or not.
If the patch gets in, would it cause problems if you used a matching toolbar background from nslookandfeel or a toolbar gradient done in nsNativeThemeCocoa?
Flags: tracking1.9+ → blocking1.9+
Priority: P3 → P2
If we're not going to be turning on colour-correction, this should probably not be a blocker. If no one objects, I'll denom.
I suspect a lot of users will be turning it on. It won't be the default, but it is a pretty big feature for anyone doing photography stuff.
Well, in that case, can I get some feedback as to whether we want to check in attachment 290927 [details] [diff] [review] as at least a stop-gap measure? We can always do the "right thing" (ie draw the gradient using nsNativeThemeCocoa) at a later time, and then back it out.
> If they pass a color with a complete transparent alpha component, use the native titlebar appearance. Why is this done this way, instead of a system color and/or -moz-appearance, like on win32? Native theme colors do not get color correction applied, nor do native theme rendered backgrounds. Even if this remains like this, what's wrong with the patch as written?
The patch has bitrotted, but this updates it to current trunk. And it does fix the problem of the unified look being wrong, fwiw.
Attachment #290927 - Attachment is obsolete: true
Attachment #311926 - Flags: superreview+
Attachment #311926 - Flags: review+
However, the patch doesn't fix the sidebar colour, as Alex Faaborg noted in comment 12.
Another screencap, this time with the Proto theme. Taken with Firefox 3.0 beta 4 on Mac OS X 10.4.11.
This bug is still visible after upgrading (from Firefox 3.0 beta 4) to 3.0 beta 5.
I'm hesitant to mention, but for me on OSX 10.4 the mismatch went away after disabling/uninstalling UNO. Now the title bar blends in with the toolbar, even if gfx.color_management.enabled is set to true. Just a glitch on my side?
Checked in; the sidebar issue is less important, I believe. -> FIXED
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
I'm still seeing this in the latest trunk nightly ("camino" is my general Mozilla stuff folder; the screenshots are Minefield). Compare: http://chrislawson.net/camino/cms-on.png http://chrislawson.net/camino/cms-off.png The entire window chrome is changing color, not just the title bar (though it's most obvious there, where you can see a reddish tint). Tab bar, unified toolbar, title bar, everything.
Bug 406730 is not entirely checked in yet; that's probably what you're seeing.
(In reply to comment #40) > Bug 406730 is not entirely checked in yet; that's probably what you're seeing. I doubt it; compare http://chrislawson.net/camino/focus-cms.png http://chrislawson.net/camino/focus-nocms.png Same effect, focused window.
That's quite a different effect, one that looks kind of like your colour scheme isn't quite calibrated.
(In reply to comment #42) > That's quite a different effect, one that looks kind of like your colour scheme > isn't quite calibrated. Er, both show exactly the same thing: color management being applied to the window chrome, and the creation of an obvious reddish tint in the title bar is the easiest way to see that.
(In reply to comment #34) > Created an attachment (id=311986) [details] > Screencap of Proto with colors mismatching > > Another screencap, this time with the Proto theme. Taken with Firefox 3.0 beta > 4 on Mac OS X 10.4.11. > I'm seeing the same thing with firefox beta5 on MacOSX leopard with the "color management" extension installed
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: