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

Status: RESOLVED FIXED

(Core Graveyard :: GFX, defect, P2)

Description

[Justin Dolske [:Dolske]]

Attachment: Screencap of color mismatch

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.

Comment 1

[tor]

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.

Comment 2

[Colin Barrett [:cbarrett]]

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)

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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.

Comment 4

[Colin Barrett [:cbarrett]]

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?

Comment 5

[Colin Barrett [:cbarrett]]

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?

Comment 6

[Chris Murphy]

(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.

Comment 7

[Colin Barrett [:cbarrett]]

Attachment: fix v1.0

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.

Comment 8

[Alex Faaborg [:faaborg] (Firefox UX)]

>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.

Comment 9

[Colin Barrett [:cbarrett]]

(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?

Comment 10

[Stuart Parmenter]

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.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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).

Comment 12

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Sidebar colors

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.

Comment 13

[Colin Barrett [:cbarrett]]

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.

Comment 14

[Alex Faaborg [:faaborg] (Firefox UX)]

>still get colors to look decent on Mac.

Doesn't this issue effect every platform where we are doing color management?

Comment 15

[Stuart Parmenter]

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.

Comment 16

[Colin Barrett [:cbarrett]]

(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?

Comment 17

[Mike Schroepfer]

So do we want to land this and close it?

Comment 18

[Colin Barrett [:cbarrett]]

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.

Comment 19

[Alex Faaborg [:faaborg] (Firefox UX)]

>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.

Comment 20

[Stefan [:stefanh]]

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? 

Comment 22

[Joe Drew (not getting mail)]

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. :)

Comment 23

[Stefan [:stefanh]]

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?

Comment 24

[Joe Drew (not getting mail)]

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?

Comment 25

[Alex Faaborg [:faaborg] (Firefox UX)]

>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.

Comment 26

[Stefan [:stefanh]]

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?

Comment 27

[Joe Drew (not getting mail)]

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.

Comment 28

[Stuart Parmenter]

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.

Comment 29

[Joe Drew (not getting mail)]

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.

Comment 30

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

> 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?

Comment 32

[Joe Drew (not getting mail)]

Attachment: updated for bitrot, carrying over r/sr+

The patch has bitrotted, but this updates it to current trunk. And it does fix the problem of the unified look being wrong, fwiw.

Comment 33

[Joe Drew (not getting mail)]

However, the patch doesn't fix the sidebar colour, as Alex Faaborg noted in comment 12.

Comment 34

[Ernst de Haan]

Attachment: 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.

Comment 35

[Ernst de Haan]

This bug is still visible after upgrading (from Firefox 3.0 beta 4) to 3.0 beta 5.

Comment 36

[Thomas Stache]

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?

Comment 37

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in; the sidebar issue is less important, I believe.  -> FIXED

Comment 39

[Chris Lawson (gone)]

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.

Comment 40

[Joe Drew (not getting mail)]

Bug 406730 is not entirely checked in yet; that's probably what you're seeing.

Comment 41

[Chris Lawson (gone)]

(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.

Comment 42

[Joe Drew (not getting mail)]

That's quite a different effect, one that looks kind of like your colour scheme isn't quite calibrated.

Comment 43

[Chris Lawson (gone)]

(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.

Comment 44

[u194118]

(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