In order to complement the use of gamma correction in PNGs, as well as the tremendous benefit of cross platform color matching, we need to implement gamma corrected CSS color values as specified in both CSS1 and CSS2. The omission of this portion of the specification has a detrimental effect on PNG use in the production of new Themes. We need a gamma correction method that works the exactly the same throughout the application. http://www.w3.org/TR/REC-CSS1#color-units http://www.w3.org/TR/REC-CSS2/syndata.html#color-units

*** Bug 53606 has been marked as a duplicate of this bug. ***

From the CSS 2 rec.: "Conforming user agents may limit their color-displaying efforts to performing a gamma-correction on them. sRGB specifies a display gamma of 2.2 under specified viewing conditions. User agents should adjust the colors given in CSS such that, in combination with an output device's "natural" display gamma, an effective display gamma of 2.2 is produced. See the section on gamma correction for further details. Note that only colors specified in CSS are affected; e.g., images are expected to carry their own color information." "For information about gamma issues, please consult the the Gamma Tutorial in the PNG specification ([PNG10])."

The following patch gamma corrects CSS and HTML specified colors along with GIF colormaps. This isn't the final version, just something for people to play with. The gamma value should really be requested from the platform gfx instead of hardcoded, and calculation of the gamma correction should be moved to one function instead of gfx and gifcom both having a copy. JPEGs are still not gamma corrected - I'll look at that later.

So I can estimate the priority of this, do we care enough about getting gamma correction working properly to try for 6.0?

nice! i will be sure that this gets looked at, however, since this isn't a showstopper i doubt we can get it into B3. I definitely don't have the final say on this, but the tree closes tonight for B3 and anything else that wants in for RTM must pass extreme scrutiny. Which doesn't mean it isn't important. any amount of effort you make will get the ball rolling, just don't stay up all night tonight to try to get it in. thanks for contributing, truly appreciated.

On the question of whether this should be a priority for 6.0, my answer is a resounding "yes." Not only is this a standards-compliance issue (it is required for CSS), it is also going to be a somewhat painful change for designers to deal with, regardless of when it happens--and with all the other major standards-related changes in 6.0, there will never be a better time to implement it. That said, it also needs to be absolutely correct--inverting the exponent would be even worse than what Adobe did to Photoshop 5.0. (Insofar as the code appears to be based on that in libpng and libimg/pngcom, I don't have any serious doubts--but I've gotten confused enough in the past that I don't trust anything until I test it personally, and preferably Chris Lilley does, too.) The only way to test it effectively is to check on an SGI or NeXT console or on a stock (non-ColorSync'd-to-gamma-2.2) Macintosh. I don't have access to any of those machines. But many thanks to Marlon for entering this bug and to Tim for supplying the patch. This is an important one.

The updated patch attached moves all the gamma logic to one place (nsColor) and adds gamma correction for jpeg images. The gamma value is still hardcoded per platform because I don't have the code needed to query the gamma value for Win32 or MacOS, and the version for X needs the gfx connection to be already established. Though it might be using the wrong gamma value, at least it's now used consistently for all of css/html/gif/jpeg/[pmj]ng. CCing chris@w3.org for feedback.

I don'tremember who told me this, but I understand that Xcms is widely unimplemented despite having been part of X11 for around a decade or so... FWIW.

For X I was thinking of using XSolarisGetVisualGamma() (solaris) and XF86VidModeGetGamma() (XFree86) with the appropriate autoconf tricks.

Oh, cool--I wasn't aware of either of those. (Of course, I've used XF86 every day for the last 6 years, and I've never seen any config setting or helper app that would allow me to calibrate and set the gamma value... Is that new? It doesn't have a man page on my 3.3.6 test system.)

New to XFree86 4.x, I believe. It's part of the XFree86 X server video mode extension (XFree86-VidModeExtension - xf86vmode.h). The program for examining/setting the gamma value is "xgamma".

bear with me on this, but how can you perform gamma correction on GIF and Jpeg if they have no source value. i thought the whole algorithm was based on the gamma of the display device and the source value embedded in the PNG.

Just like how you deal with a [PJM]NG without any gamma information - you assume a default file gamma value (0.45455).

Marlon, this is something you have to do because of existing practice. Designers implicitly assume that their (unlabelled) GIF and JPEG images are in the same color space as CSS and HTML colors (that is, they assume that the colors will match when they place images on background colors). Since HTML and CSS colors are defined to be sRGB, this means that GIF and JPEG images are implicitly sRGB, also. In fact, this is precisely what the International Color Consortium (the folks behind sRGB and ICC profiles) recommends, and I'm sure existing practice is the reason behind the recommendation. Consider the alternative if that doesn't convince you: designers will go absolutely apeshit if their carefully matched image+HTML/CSS combos suddenly stop matching even on truecolor displays--ESPECIALLY since most of them seem to use Macs, where it will really show up dramatically.

dbaron: I guess this explains the bug with the WaSP image from ~18 months ago... Greg: We will need a thorough testcase for this, are you aware of any?

I don't have a definitive test case (or set of them), but here are two GIF-based ones to get you started (both from the recent Webmonkey article at http://hotwired.lycos.com/webmonkey/00/37/index2a.html): short version: http://hotwired.lycos.com/webmonkey/00/37/stuff2a/complete_websafe_216/reallysafe_palette.html full version: http://hotwired.lycos.com/webmonkey/00/37/stuff2a/complete_websafe_216/test.html It would be trivial to convert the GIFs in these to PNGs, either with or without an sRGB chunk or the corresponding gAMA and cHRM chunks (newer versions of libpng will treat the sRGB and gAMA+cHRM cases equivalently). I may have time this weekend to copy and convert the big page; if so, I'll make it available from the PNG site. Btw, I believe the ICC has defined some means of including ICC profile info in both GIF and JPEG images, so my earlier comment about GIF and JPEG images being implicitly sRGB should have qualified that as "_unlabelled_ GIF and JPEG images." I'm not certain, but I suspect Photoshop 5.5 may embed full ICC profiles in all of its saved images. (The iCCP chunk it puts in PNG images is broken, however. I believe libpng 1.0.8 detects and works around that breakage, probably with an error message.)

OK, I've created four copies of the "websafe" table here: http://www.libpng.org/pub/png/colorcube/gamma-consistency-test.html One is the original GIF version (with some minor changes noted in the comments at the top); one is the corresponding PNG version (unlabelled); one is a PNG version using gamma 1/2.2 PNGs; and one is a PNG version using sRGB PNGs (which also include a gamma 1/2.2 chunk--I haven't decided if that's good or bad in this context). The link above indicates what tests still need to be created. I'll work on those as time permits, but if there's a CSS whiz around who can sketch out the basic method of converting loose HTML 4.0 colors to strict HTML 4.0 + CSS, it would save me some time perusing the specs.

cc'ing joe hewitt, might be able to help us out

Nominating for RTM decision, as this needs to be fixed before mozilla/Netscape can in good faith claim full PNG support.

And CSS-1 support! Gamma correction is not optional for CSS conformance. Btw, I've completed (or at least fleshed out) my gamma test suite [URL field above]; there are now 10 cases: GIF/HTML, PNG-nogamma/HTML, PNG-gamma16/HTML, PNG-gamma22/HTML, PNG-sRGB/HTML, and five corresponding cases using CSS colors. All test cases have been validated for both CSS and HTML 4.0, I believe; in any case, there are buttons at the top of each page to do that. Note that testing on Windows/x86 or Linux/x86 won't do much, aside from verifying Tim's patch; where problems will really stand out is on Macs or SGIs. I don't have access to either. Also note that these pages trigger a *seriously* ugly performance problem in Mozilla, at least on Linux--perhaps someone more familiar with its current status knows of an open bug already. With images already in the cache, rendering time is roughly two orders of magnitude slower than NN 4.x, and scrolling time is two to three orders of magnitude worse. Mozilla is basically unusable on these pages on a PII-266.

Greg, if you want to test what happens on the Mac, just add a #define/#undef XP_MAC pair around around NS_DisplayGammaValue. That's how I tested the patch when creating it on Linux.

greg, thanks for that comprehensive test suite. I tested on an unpatched current mac NS6 build and saw no color matching in every case except for GIF on HTML. I am trying to get tor's patch installed for the mac and will test again

notice how even unlabelled pngs aren't matching css colors on the mac, because of that default value

First a big THANK YOU, Tim, for doing that work... A problem though: I feel very strongly that the "LUT_exponent" should be implemented in the platform-specific parts of GFX, not as #ifdefs in the Image Library. It would be nice too function to put comments in each platform-specific "GetLUTExponent()" that list what is the returned value on other platforms, just in case we later need to do some fine-tuning.

One reason I haven't moved the display gamma query into the heart of platform gfx is that in order to return an actual queried values the gfx system will need to be initialized. I'm thinking that GetColorValue() is probably being called early in the startup before gfx is started. True?

I really want this to make it into RTM, but not sure if it is going to happen. We can't even attack PDT with it until we've got a patch here. cc'ing Eric Krock, since we can't promote "full" CSS1 compliance anymore

Err... we do have a patch (9/22 attachment), currently under review.

pierre: I've attached an updated patch that adds a GetGammaValue to nsIScreen. Is this the sort of thing you were looking for?

It looks better but you will have to implement GetGammaValue() on all the platforms, not just GTK. I'm not the best guy to review this patch: Pam Nunn and Don Cone are the gfx specialists.

One thing to note - this patchkit assumes that the gamma value will remain fixed throughout the lifetime of the application. This is probably a forgivable assumption except maybe for multi-screen MacOS boxes. If we want to handle changing gamma on the fly, things get more involved: * gamma correction for colors will be moved up to platform gfx (setcolor) * system css colors will have to be pushed through an inverse gamma transform * need some way of invalidating all images so that they can be uncompressed and gamma corrected again (we don't want to run them through multiple gamma correction stages because the operation is lossy).

Is anyone still working on this for RTM? For all the reasons described in this bug, it's important for 6.0... but if it isn't going to happen it should probably be rtm-'d Failing that, nominating for mozilla1.0 in hopes of getting it onto the right people's radars.

The patch is in review limbo - I've asked for reviews, but haven't received anything except the suggestions from pierre.

I started a review the other week, then had a crash while composing my comments. I've just now restarted the process via IRC with tor, who is preparing a new patch. Sorry for the delay. /be

Updated patch based on a conversation with brendan. Changes: * check screen is valid in InitializeGamma * rename static NS_InitializeGamma to InitializeGamma * use appropriate class in platform nsScreen*::nsGammaValue

Looks good to me. I'd invert the "double gamma" calculation in InitializeGamma(), though--the 0.45455 is OK for PNG since PNG actually stores the file gamma value as a fixed-point integer (45455), but for floating-point calculations, you may as well be precise: double gamma = 2.2/gammaValue; Also, for the two places where you hardcode the SGI value (1.7), I've got code that can read that from the relevant system config file, if you wish: *aGammaValue = 2.2/1.7; FILE *infile = fopen("/etc/config/system.glGammaVal", "r"); if (infile) { char tmpline[80]; fgets(tmpline, 80, infile); fclose(infile); double sgi_gamma = atof(tmpline); if (sgi_gamma > 0.0) *aGammaValue = 2.2/sgi_gamma; } And in the same two places, you could add an ifdef for NeXT, on the off-chance that that platform ever gets a port: #elif defined(NeXT) *aGammaValue = 1.0; You never know... Greg

Changes this time: * change from 1/0.45455 to 2.2 in InitializeGamma * adds a "gfx.gamma" pref that overrides the platform GetGammaValue(). This pref is a character string, as the preference service doesn't support floats * add sgi and NeXT gamma code suggested by Greg

BTW: ignore the MINIMUM_DELAY_TIME bit of the gif.cpp change - that's the patch from 55997 that got accidentally included when I made the diff.

rtm-, not a stop ship.

Argh! I thought this was ready in plenty of time for rtm... what happened? (well, I guess I know what happened... review limbo for too long. But why? I didn't see any negative reviews, and the latest comments suggest that the current patch reflects all the reviews...)

Another patch has been attached based on the latest feedback from Brendan. Changes this time: * remove the extra space in "result = prefs->CopyCharPref(..." * remove extra parens in "gammaRamp[i] = (unsigned char)(val);" * use sizeof(tmpline) instead of 80 in nsScreen{Gtk,Xlib}::GetGammaValue() * add nsScreenBeOS::GetGammaValue() * remove MINIMUM_DELAY_TIME patch from 55997 that was accidentally included in the last patch

is it possible to predict if this will now cause any perceivable lag in display? it is especially critical to UI/theme elements display, which is already too slow. also, how will this effect the display of modern, which is a GIF skin? some were worried that fixing this now would break modern. and, is there a way to turn it off?

It will cause gif and jpeg image loading to be a tiny bit slower. Once the images come out of the image cache (like the chrome) there is no change in speed. Someone from the layout/style group could probably give a ballpark estimate about how often ns{CSS,HTML}Value::GetColorValue() gets called. modern2 changes its apparent brightness based on the gamma value returned. I've run with a wide range of gamma values and haven't noticed any color mismatches due to gamma correction. You can override the gamma correction to a linear ramp by adding "user_pref("gfx.gamma", "2.2");" to prefs.js.

As I understand it, there is a simple reason why this change should not break any existing gif-based skins... The current behavior is: - GIFs and CSS colors are NOT gamma-corrected - PNGs with gamma ARE gamma-corrected - PNGs without gamma ARE gamma-corrected with a default gamma, I *think*. This patch causes the following behavior: - GIFs, CSS colors and gamma-less PNGs are gamma-corrected with a default gamma value. - PNGs with gamma are gamma-corrected with the value they specify. So both before *and* after, GIFs will match CSS colors because they are treated the same. The only difference is that it is now possible to reliably persuade PNGs to also match.

I believe Stuart's analysis is accurate--that is, various chrome colors will shift in the same way on some platforms, not causing any visible mismatches--but there may be a slight change in the appearance from what the designers originally intended. That is, based on Nikhil Bhatla's <nbhatla@netscape.com> comments in mozilla.ui (which I think are what led to this bug being filed), the GIF icons were tuned on non-sRGB Macs ("our development platform") without CSS/HTML gamma correction, so with gamma correction turned on, they'll look somewhat darker than that across all platforms. Now, it doesn't sound like the absolute brightness level was a major concern (at least, not in that thread), and the appearance on Windows and Linux/x86 boxes is/was always that "somewhat darker" shade, so I don't *think* this should bother anyone too much. But Mac users with old and new versions of Mozilla running side-by-side will be able to see a difference between the two.

I meant to comment earlier, but I've been too busy. The DEFAULT_CRT_GAMMA change is a bad idea for two reasons: (1) all CRTs have essentially the same "gamma" value (and most of the world agrees it's close to 2.2, though Poynton stubbornly insists 2.5); and (2) it's used as the default value for a variable called "gammaValue" and later "gamma", which is completely misleading. In case it wasn't obvious from the PNG changes, the term "gamma" is overused and overloaded with meanings, with Apple, SGI, camera makers and everybody else all defining it incompatibly or, worse yet, just using the term without ever defining what they mean. The split into CRT_EXPONENT and LUT_EXPONENT may have seemed pedantic overkill, but at least it has the benefit of forcing one to think about what's really changing across platforms (the lookup table, not the CRT or other display device), and it has a clearly written tutorial to back it up (http://www.libpng.org/pub/png/spec/PNG-GammaTutorial.html). I don't have specific suggestions for fixing this, but I think the previous patch was closer to being on track. Greg

Ok, I've removed the DEFAULT_CRT_GAMMA definition until someone really comfortable with gamma can help us clarify our terminology.

My only comment is that you can optimize the GIF loop to avoid reloads from memory in the loop control required due to memory ambiguity: RCS file: /cvsroot/mozilla/modules/libimg/gifcom/gif.cpp,v retrieving revision 1.35 diff -u -r1.35 gif.cpp --- gif.cpp 2000/11/06 20:08:29 1.35 +++ gif.cpp 2000/11/10 16:56:17 @@ -1047,9 +1047,9 @@ #endif /* M12N */ for (i=0; i < gs->global_colormap_size; i++, map++) { - map->red = *q++; Put gs->global_colormap_size in a limit register. I think this patch has many r='s, and at waterson and I have sr='d it. That's more than enough to get checked in, I think. Anyone object? Speak fast. /be

I think this is an unneeded optimization - you're attempting to speed up the control of a loop which is exectuted infrequently, runs for <= 256 iterations, and contains a number of function calls and memory lookups. The last I heard (Nov 8), buster was looking at these changes.

Oh, ok -- I am old-school and hard-core -- I always save code space and runtime, at the expense of a little source complexity, with such loop control memory disambiguation. But that's just me. Sorry about that, buster -- can you r= soon? /be /be

the change looks fine to me as far as layout is concerned. I'm no expert in this area, though. I'd feel more comfortable if someone from gfx-land also reviewed. Kevin, Don? But if waterson and/or be think that enough knowledgable folks have reviewed and approved already, I'm fine with it as is.

How does this patch relate to the gamma correction code which already exists in S:\mozilla\gfx\src\nsDeviceContext.cpp and S:\mozilla\gfx\src\windows\nsRenderingContextWin.cpp? Does it replace it? The existing code does the gamma correction by adjusting the color value passed to nsRenderingContext::SetColor method when setting the current color. The patch adjusts the value returned for CSS colors. If the color value is set through the CSS OM then retrieved the CSS color will not be same as what is passed in. It will be corrected for gamma. Is this the correct behavior?

It essentially replaces the existing gamma code (which is only hooked up half-heartedly for win32). The current code only supported correction for colors, and did it at the gfx endpoint where it would mess up system CSS colors. Nothing in mozilla ever set the gamma value used by this code to anything other than the identity map. Ian Hickson is a better person to answer the CSS DOM question - I don't pretend to be a CSS expert, even in bugzilla.

It is my understanding that the CSS OM _is_ expected to roundtrip colour values, so we should not be changing them. In CSS terms, as I understand it the specified and computed values in the CSS OM should not be gamma corrected, only the _actual_ values should be gamma corrected. (It is not possible using the CSS OM to get to the actual values.) David should correct me if I am wrong on this...

It all depends on which is the specified value, which is the computed value, and which is the actual value. This is an area where the spec is in general hopelessly vague. However, I would expect (not according to the spec but according to good sense) that the CSSOM would return the original value so that the color returned could be reused somewhere else. However, I think the stronger argument in the spec is for the reverse - from sections 6.1.2 and 6.1.3 say that the difference between computed and actual values is only approximations such as rounding to the nearest pixel and dealing with unavailable fonts. However, one could easily make the counter-argument that the gamma-correction is something that should be handled by a layer lower than the CSS rendering system... Perhaps I should make a plea to the WG that CSS3 should include clear statements throughout of which value is the computed value...

Kevin makes a great point about round-trip fidelity, and I think David's first instinct is correct. It seems odd and unreasonable for an author to get a different value through the CSSOM than was specified in the style rule. After talking with Kevin, I'm convinced that, as much as possible, gamma correction ought to happen in the output system. Layout and style should be oblivious.

Ok, then how should we handle system CSS colors? Presumably (at least, this is how mozilla handles it) they are returned as color values that can be written directly into the framebuffer without correction. Should they be pumped through an inverse-gamma function before the CSS system sees them?

Note that the system css color -> inverse-gamma -> gamma -> output conversion is lossly, so the colors won't match exactly.

The problem is that the style system and gfx/widget are currently locked together - when the style system turns a css rule into a nsStyleColor it calls nsLookAndFeel::GetColor() and stores the result as a raw color. Since the value returned is a framebuffer value it shouldn't be gamma corrected. Adding a gamma flag to nsStyleColor or otherwise indicating that the color should be evaluated lazily is likely to raise the ire footprint team. Additionally nsStyleColor is a raw structure without accessor methods, so all the users of it would need to be modified to do the right thing. Any style people want to offer opinions on how to proceed?

What if the gamma correction or the system color to real color lookup are both done at the same point, sometime between extracting the value from the DOM and rendering it? In other words, keep the knowledge of both gamma correction and of the actual system colors away from layout and the style system.

Would it be possible to put some kind of internal flag on system colors in the CSSOM? Or does CSSOM return colors as pure RRGGBB values? Presumably CSSOM can at least *extract* the RRGGBB values, and if so, it should probably retrieve inverse-gamma'd values. However, a flag that would at least eliminate the issue if the result of a getColor() was used in a setColor() (hypothetical methods since I don't know CSSOM) would cover 99% of the cases. Additionally, on a 24bit display it would be unlikely that the difference would be detectable by the human eye even with an off-by-one error in the R, G and B components, so I don't think that even the worst case is disastrous. Could we at least get a patch in place for NON-system colors (with perhaps slightly imperfect behavior on the system colors) and then file separate bugs to get the system color handling perfect? It seems a shame to me that this patch is rotting.

*** Bug 48772 has been marked as a duplicate of this bug. ***

*** Bug 48772 has been marked as a duplicate of this bug. ***

*** Bug 65356 has been marked as a duplicate of this bug. ***

Off-by-one errors aren't visually acceptable. Been there. dbaron's suggestion seems sound. How hard is this to do? Re-architect the whole image system, or just a few tweaks?

[Retyping a comment I lost by hitting back in a mid-air collision.] It's not a huge change, but it would probably lead to a patch a number of times larger than the current one. What it would probably require is defining a second color type that would hold system colors as constants rather than color values and would never be gamma-corrected. We could then move the gamma-correction and system-color lookup to the site where we convert from one type of color value to the other.

Disowning.

Is this seeing work (with libpr0n, maybe? pavlov?)... If not, this should probably be retargeted at moz0.9...

time to decide on this... ready to go for m0.8 or no???

tor disowned this bug, so I'm moving its TM to 0.9 and pleading with someone on the cc: list to take ownership. Saari? Pavlov? /be

Netscape's standard compliance QA team reorganised itself once again, so taking remaining non-tables style bugs. Sorry about the spam. I tried to get this done directly at the database level, but apparently that is "not easy because of the shadow db", "plus it screws up the audit trail", so no can do...

Updated patch - moves color gamma correction into gfx. System colors are now gamma corrected. I'll commit to writing a SetUncorrectedColor for the various platforms if someone teaches the style system to use it.

tor, have you reclaimed? moving nobody's bugs off the 0.9 list. somebody assign it the right milestone

moving nobody's bugs off the 0.9.1 milestone list.

tor, Pav? what's up with this one? it's my favorite bug of all time, i hope we can get it in. Themes are patiently waiting for relief, as are billions of designers out there

I need some help from the style people to prevent system css colors from being gamma corrected. I'm willing to write the required platform gfx code, but need assistance hooking it into the style system.

Mebbe hyatt, who is hacking on the style system, can help tor here. /be

Any update on this? Did hyatt respond to Brendan's request offline? If not, are there any other style system gurus who could step in here? #include <std-we-need-to-clone-hyatt.h> I believe it's really important for this fix to make it into 1.0. Since tor appears to have most of the work done, it'd be really sad to see this feature miss 1.0 for lack of style people stepping up to help.

CC'ing attinasi@netscape.com for style system help.

I'll try to help with the style systems issues. From what I can see, we need to store a bit on each nsStyleColor to indicate if it is a system color or not, and then somebody decides based on that flag whether or not to gamma correct the value, right? If that is the case, then we either need a small bitfield on the nsStyleColor struct, or we need to create some kind of external mapping of nsStyleColor instances that represent system colors (yech!@#$*#@). I'd like to do some measurements of how many nsStyleColor instances there are on a given set of pages (and extrapolate the extra bloat required for the flags) and maybe also how many nsStyleColor instances actually represent a system color. Or, am I missing something here? (sorry, I'm not Hyatt's clone so I need a little education)

Taking the bug

Moving again.... 0.9.6

For the record, in response to an offer from Tim Rowley to take over the bug, I wrote: ---- Here are some pointers from a style system standpoint. I haven't looked into what we should do in ImageLib or GFX to not correct the already gamma-corrected colors. I wanted to implement Marc's idea of an additional bitfield that indicates whether the color is an already gamma corrected system color (comment #93). These colors are obtained through nsILookAndFeel::GetColor(). See in particular: http://lxr.mozilla.org/seamonkey/source/content/base/src/nsRuleNode.cpp#355 The work was originally fairly straightforward but it was a bit complicated by Dave Hyatt's rule tree landing. See the following comment: http://lxr.mozilla.org/seamonkey/source/content/shared/public/nsStyleStruct.h#160 Depending on the changes you are going to make, please update the patch for bug 77991 or mark it invalid: http://bugzilla.mozilla.org/show_bug.cgi?id=77991 Before checking in, you will probably be asked for an estimate of the increase in terms of memory footprint

This is probably one of the few standards compliance bugs that should really be a mozilla 1.0 blocker, so I'm a little disappointed to see it moved off to mozilla 1.2.

Ooops, sorry: I'm not familiar enough with imaging and gamma correction to make the call on how important the problem is for users who have an interest in it. I pushed it to moz1.2 because Tim Rowley offered to take the bug and I thought that if he wasn't done by then, I would do it. Another reason was the low-priority Hixie-P3 status. Back to moz1.0 as a "DBaron-P1" :)

While adding a flag on nsStyleColor to indicate if a color needs gamma correction seems fine from a bloat point of view (browsing a few pages only seemed to generate a hundred or so nsStyleColors), tracking down all the uses of nsStyleColor to call the appropriate method on the rendering context is going to be a bit of a nightmare. In the gross hack category to minimize the number of changes, we could hide the flag in the alpha channel of the nscolor . Poking through the tree, nobody seems to be seriously using the alpha channel of the nscolor. It's used in nsDocShell and nsViewManager to check for unset values; and in nsCSSValue and nsHTMLValue when converting to a string.

I read thru these ramblings and don't still don't know why, in theory, the color space should be rendered with any modification at all. Rendering of html and css colors, yes that requires gamma consider, etc. However, the answer is simple [unfortunately] just replicate IE. 90% of the browsers are IE, so designers pick colors that look good with IE; period. As I understand it, PNG is a compression algorithm. Therefore, the decompression algorithm should repoduce the orginal, without any modification. Al.............

Moving to Mozilla1.1. Engineers are overloaded with higher priority bugs.

Bulk moving from Moz1.1 to future-P1. I will pull from this list when scheduling work post Mozilla1.0.

Changing the CSS colors to be gamma corrected should not be changed this close to Mozilla1.0. The gamma correction would have to turned off by default anyway to maintain backward compatibility with both IE and Nav4.x. nsbeta1-.

With regard to comment 103, I _hope_ you meant "would have to turned off by default anyway [if it were fixed this close to release]." God forbid we should ship a standards-compliant browser. Greg P.S.: Read the rest of the bug. The entire point of gamma-correcting PNGs _and_ MNGs _and_ JNGs _and_ GIFs _and_ JPEGs _and_ HTML colors _and_ CSS colors [and anything else that has color values other than 0 or 255] is to preserve color-matching-by-eye as practiced by generations of web designers using MSIE and NN 4.x and so on. IOW, different color-things on any given page will not suddenly become mismatched if they were matched before (except for PNGs with stored gamma values, but fixing that bug is one of the goals of this bug, and PNGs that *should* have matched but didn't previously, now will). A side-by-side comparison on a Macintosh of an old browser and the fixed Mozilla would look different, but the same test on 99.9% of Windows and Linux/x86 machines would look identical. And a comparison of the fixed Mozilla on Mac and on Windows would suddenly match (assuming the system gamma values are known or guessed correctly), which should make most designers MUCH happier. (Note that even on a misconfigured system, the colors on the web page will be self-consistent; only a side-by-side comparison with an old/broken browser would show the difference.)

Like Greg says, if implemented correctly this won't alter the consistency of any page (except those that had PNGs, which were already inconsistent), and there isn't any reason it couldn't be turned on by default. drivers@mozilla are interested in getting this issue addressed for mozilla1.0, both for standards correctness and to encourage the uptake of PNG and related formats. As always, the complicating factor is system css colors. A possible low-impact patch is to push the system colors through an inverse gamma mapping when they are obtained, then do gamma correction on all colors passing through gfx for output. This of course will introduce some error into system color reproduction, but hopefully at a fairly imperceptible level. I'll try to come up a patch for this worse-is-maybe-better solution this weekend.

Comment on attachment 74548 [details] [diff] [review] merge to tip > /** > * The color used to paint with. > */ >- attribute nscolor color; >+ void SetColor(in nscolor aColor); Match the rest of that .idl file, and Mozilla house IDL style: setColor, not SetColor. Expand tabs to 4-space equivalents if you can stand the cvs blame, while you're there. >@@ -298,8 +297,10 @@ > if (mCurrentFont == nsnull) mCurrentFont = (BFont *)be_plain_font; > > mView->SetFont(mCurrentFont); >- mView->SetHighColor(mGammaTable[NS_GET_R(mCurrentColor)], >- mGammaTable[NS_GET_G(mCurrentColor)], mGammaTable[NS_GET_B(mCurrentColor)], 255); >+ mView->SetHighColor(NS_GammaCorrectComponent(NS_GET_R(mCurrentColor)), >+ NS_GammaCorrectComponent(NS_GET_G(mCurrentColor)), >+ NS_GammaCorrectComponent(NS_GET_B(mCurrentColor)), >+ 255); Mebbe indent underhanging args so they line up with first one's initial column. >+++ gfx/src/gtk/nsRenderingContextGTK.h 16 Mar 2002 19:20:10 -0000 >@@ -106,7 +106,7 @@ > NS_IMETHOD GetLineStyle(nsLineStyle &aLineStyle); > > NS_IMETHOD SetColor(nscolor aColor); >- NS_IMETHOD GetColor(nscolor &aColor) const; >+// NS_IMETHOD GetColor(nscolor &aColor) const; Any reason to comment out rather than remove? >+++ layout/html/base/src/nsImageFrame.cpp 16 Mar 2002 19:22:06 -0000 >@@ -1165,6 +1165,7 @@ > } > } > >+/* > // if we could not draw the image, then just draw some grafitti > if (!iconUsed) { > nscolor oldColor; >@@ -1175,6 +1176,7 @@ > NS_STATIC_CAST(int,(size/2)-(2*p2t)),NS_STATIC_CAST(int,(size/2)-(2*p2t))); > aRenderingContext.SetColor(oldColor); > } >+*/ #if 0, don't comment out, if you can't just remove -- but remove if possible (cvs will remember). >+++ embedding/browser/webBrowser/nsWebBrowser.cpp 16 Mar 2002 19:22:22 -0000 >@@ -1583,11 +1583,11 @@ > case NS_PAINT: { > nsPaintEvent *paintEvent = NS_STATIC_CAST(nsPaintEvent *, aEvent); > nsIRenderingContext *rc = paintEvent->renderingContext; >- nscolor oldColor; >- rc->GetColor(oldColor); >+// nscolor oldColor; >+// rc->GetColor(oldColor); > rc->SetColor(browser->mBackgroundColor); > rc->FillRect(*paintEvent->rect); >- rc->SetColor(oldColor); >+// rc->SetColor(oldColor); > break; > } > Ditto -- #if 0 if you can't remove -- why not remove? A major comment explaining the new approach should be added if you #if 0. Fix these, and I'll sr= when there's an r=. Thanks for doing this, /be

Where's the inverse-gamma-for-system-colors code hiding?

That patch wasn't ready for review - it was just a checkpoint. As dbaron says, it doesn't have the system css color modifications, plus I'm not sure if we should remove GetColor.

Review of attachment 76346 [details] [diff] [review]: Although this patch has gotten a good bit of review before, I'd be a little more comfortable if someone a little more familiar with GFX / image code said that all the corrections were being made in the right places. However, I guess that testing with a non-2.2 gamma value would show any bugs there (this should probably be done on all the different gfx ports, although perhaps it's not worth the bother for those that always use 2.2), although not necessarily potential performance problems of doing it at a different stage than the optimal one (although it seems like that's already been thought through, so I'm more worried about potential missed cases somewhere). (Did you check that the Windows code is actually always using the gamma-corrected colors when it should be? It jumps through quite a few hoops with mCurrentColor vs. mColor.) I'd rather see the NS_InitializeGamma call made from something like the GFX module constructor, where it will only happen once, and isn't tied to the docshell code, which seems like an odd dependency. However, I think the gfx module constructor itself won't actually work since the GetService to get the screen manager wouldn't work yet. Perhaps the layout module constructor would be better? I'm a little worried about everything linking OK with your global |nsGammaRamp| and |nsInverseGammaRamp| and some of the stricter linkers on the ports tinderbox, but I guess it should be OK... In NS_InitializeGamma, these casts should be to PRUint8 (which is probably guaranteed to be |unsigned char|) for consistency: + nsGammaRamp[i] = (unsigned char)val; + nsInverseGammaRamp[i] = (unsigned char)val; Also, how can you possibly end up with |val>255.5| in either case? Since you can't, why bother with the > 255.0 check? (Or do you just want an assertion of some sort?) Also, any reason you're using |exp| and |log| to build the inverse gamma ramp rather than just using |pow| to the |1/gamma|? The nsScreenQT.cpp change should probably look like the GTK/Xlib one, although it might be nice (although probably not worth it for something this size, although that code has the potential to grow) to consolidate that somewhere (in a new file in gfx/src/x11shared/, which seems to have a fun build mechanism, perhaps?). The nsScreenMac.cpp change is inconsistent with what the imgContainerMNG / nsPNGDecoder code was doing -- shouldn't it return 2.2 * 1.8 / 2.61 ? (Or is there an API for determining the right value?) Isn't Mac the major platform that's supposed to be affected by this change? I think the nsXPLookAndFeel.cpp change deserves a clear comment explaining why you're doing the inverse gamma correction and why it's bad (although only because of rounding errors). It seems like the nsRenderingContextXlib.cpp and nsRenderingContextWin.cpp changes undo one of the things that code was originally doing, which was setting the alpha bits of the color to 255. It's probably worth going back to what the original code did. (Also, in the windows code, is the RGB macro guaranteed to be the same as NS_RGB? It sounds to me like a Windows macro.) Any reason you prefer using the gamma ramp directly (by copying the pointer) in the nsJPEGDecoder changes rather than using the macros? Might this actually compile to something faster since the pointer to the gamma ramp will be in a local variable rather than another library -- or do most optimzers handle this properly? Other than that, r=dbaron.

This patch addresses all your comments except for two: * nsScreenQT::GetGammaValue left as-is, since Qt is a cross-platform toolkit * NS_InitializeGamma left in docshell since I'm not sure where to move it

Comment on attachment 76441 [details] [diff] [review] address dbaron's comments OK, r=dbaron, but: * I still think the layout module constructor would be better (Initialize in layout/build/nsLayoutModule.cpp) * Do we use our Qt GFX port on anything other than Unix? (There's GTK+ for Windows too...)

>Isn't Mac the major platform that's supposed to be affected by this change? Yes. The Mac gamma is hard-coded in the patch, but 1.8 is only the default on Mac OS / Mac OS X. The user can calibrate the display differently in which case hard-coding the default backfires. Getting the gamma value from the OS or at least making configurable via a pref would be better than hard-coding.

After reading http://www.inforamp.net/~poynton/notes/colour_and_gamma/GammaFAQ.html#Macintosh http://www.inforamp.net/~poynton/notes/color/GammaFQA.html http://developer.apple.com/techpubs/quicktime/qtdevdocs/REF/whatsnewqt5/Max.70.htm it seems to me that the system setting for gamma actually causes QuickDraw to do the correction, and we're just hard-coding for the default correction, which is not done on other systems. Anybody more knowledgable want to comment?

Request for an implementation of nsScrenMac::GetGammaValue() is bug 75133.

Comment on attachment 76464 [details] [diff] [review] flesh out nsScreenQT::GetGammaValue, move NS_InitializeGamma to layout >+// Gamma correction >+extern PRUint8 nsGammaRamp[256], nsInverseGammaRamp[256]; Do you need PR_EXPORT_DATA(PRUint8) instead of extern PRUint8 here, and PR_IMPLEMENT_DATA(PRUint8) in the .cpp file, for some platforms? >+ >+double NS_DisplayGammaValue(void); >+void NS_InitializeGamma(void); >+#define NS_GammaCorrectComponent(x) (nsGammaRamp[x]) >+#define NS_GammaCorrectColor(x) NS_RGBA(nsGammaRamp[NS_GET_R(x)], \ >+ nsGammaRamp[NS_GET_G(x)], \ >+ nsGammaRamp[NS_GET_B(x)], \ >+ NS_GET_A(x)) >+#define NS_InverseGammaCorrectComponent(x) (nsInverseGammaRamp[x]) >+#define NS_InverseGammaCorrectColor(x) NS_RGBA(nsInverseGammaRamp[NS_GET_R(x)], \ >+ nsInverseGammaRamp[NS_GET_G(x)], \ >+ nsInverseGammaRamp[NS_GET_B(x)], \ >+ NS_GET_A(x)) These could use an empty line between decls and related macros for readability; also, indent the definitions to the same column. More important, later we get calls of this form: + map->red = NS_GammaCorrectComponent(*q++); which requires that NS_GammaCorrectComponent use its macro parameter exactly once, but NS_GammaCorrectColor and the Inverse counterpart expand their params more than once. They should be named using the UNSAFE_MACRO_CONVENTION: NS_GAMMA_CORRECT_COLOR -- or they should be inline functions. Otherwise looks good -- sr=brendan@mozilla.org with some attention here. /be

Comment on attachment 77316 [details] [diff] [review] polish macros and fix linkage per brendan's comments Carrying stamps forward. /be

Verifying for BeOS

I'm hitting some link problems on BeOS. It seems that we only link the decoders against gkgfx on win32. Since the decoders use these added functions, they will need to be linked against gkgfx on any platforms that require you to resolve symbols at link time: BeOS, OS/2, AIX and linux when using -Wl,-Bsymbolic (which we use).

Filed bug 135634 ("Implement support for XSolarisGetVisualGamma()") to get support for the gamma-correction in the Solaris Xservers (Xsun and the Solaris version of Xprt) ...

Can someome please file a bug for the discussion and work to get support for Xcms (I can take the bug on demand :) implemented ?

Comment on attachment 77316 [details] [diff] [review] polish macros and fix linkage per brendan's comments Verifying for Xlib gfx and Xprint module - both seem to work ...

Just wondering: Where can I set the gamma value for the printer ?

bug 135634 now contains a patch to implement support for |XSolarisGetVisualGamma()| on Solaris platforms. Who wants to r=/sr= it ? :)

Another (very minor) issue: Which gamma value is correct: "2.2" or "2.22" ? Some docs use "2.2", other use "2.22" - and Solaris Xsun uses 2.22 as default gamma value (unless the hardware supplies a value - which is usually 2.22) ...

The CSS specs have the official answer for you. (2.22 IIRC)

http://www.w3.org/TR/REC-CSS2/syndata.html#color-units says 2.2 http://www.w3.org/TR/REC-CSS2/colors.html#gamma-correction doesn't say, but suggests that using 2.22 for NeXT is correct

Looks like a typo in transcribing from the PNG gamma tutorial which says 2.2 Glenn

CSS -> sRGB -> 2.2 With regard to the quoted section of the CSS spec, I don't know who wrote it, but it looks wrong to me. Certainly it's extremely imprecise, yet again muddying the waters by using the term "gamma" without defining precisely what it means by that. In Roland's context (Solaris), gamma == 2.2 or 2.22 would refer to the combination of LUT_exponent and CRT_exponent--i.e., where there is no LUT (just as in typical PCs), so the CRT_exponent is equal to the display gamma. In that same context, NeXTs are 1.0 and Macs and SGIs are 1.3 and 1.5 (though I've forgotten which is which). In the CSS context, it _appears_ that they're talking about the LUT_exponent only (or, more precisely, its inverse)--i.e., 1.0 (no adjustment) for PCs, 2.2 (== CRT_exponent) for NeXTs, etc. But I'm not sure I trust any of those numbers, insofar as they appear to be derived in some bass-ackward way from the information in the (out-of-date) PNG 1.0 spec. Greg

>In the CSS context, it _appears_ that they're talking about the LUT_exponent >only (or, more precisely, its inverse)--i.e., 1.0 (no adjustment) for PCs, 2.2 >(== CRT_exponent) for NeXTs, etc. But I'm not sure I trust any of those >numbers, insofar as they appear to be derived in some bass-ackward way from the >information in the (out-of-date) PNG 1.0 spec. The same numbers appear in the (current) PNG 1.2 spec, in Section 13.9. Default LUT exponents are PC: 1.0, Mac: 1/1.45, SGI: 1/1.7, NeXT: 1/2.2 This section has however been removed from the upcoming ISO PNG spec. Glenn

Reassigning this bug to tor, since he has a patch that will hopefully be checked in soon (right?).

tor: What about merging the patch in bug 135634 ("Implement support for XSolarisGetVisualGamma()") with this one ?

changing PR_EXPORT_DATA to PR_IMPORT_DATA seems to work on windows-platforms too. I still need compile the rest of my tree and run before i'm compleatly sure

After a rough landing this has been checked into the trunk. Give it a shot. QA: MacOS is the platform that really needs to be tested before verifying. Leaving open for possible branch pickup.

Sorry about the PR_EXPORT_DATA bum steer -- if memory serves now, the best way to use these macros is to define some other macro only when buildling the library, and test that other macro with an #ifdef to decide whether to EXPORT (if defined) or IMPORT (for consumers of the lib's .h file). Can we do that? Is there XPCOM NS_COM or similar macro layering we can use? /be

When doing the classic libimg to libpr0n update I missed gamma correction for RGB images on unix platforms.

Comment on attachment 79136 [details] [diff] [review] jpeg gamma correction for unix platforms r=biesi

Comment on attachment 79136 [details] [diff] [review] jpeg gamma correction for unix platforms sr=brendan@mozilla.org, good catch for 1.0 (for RC1?). /be

jpeg/rgb/unix fix checked in.

Sorry if I'm spamming, but would this patch have caused a shift in the "breakpoints" between shades of gray? See http://www.meyerweb.com/eric/css/tests/grayscale.html in Mozilla 2002040908 (or earlier) versus 2002041308, for example. Loading up that page in IE5/Mac and 2002040908 shows that they exactly match in where the breakpoints fall. They're inconsistent between IE5/Mac and 2002041308. A design that relies on subtle shades of gray-- such as, for example, alternate-row highlighting in a long table-- could be horked by such a change. One place where that actually happened is http://developer.netscape.com/evangelism/sidebar/css2qr/prop-visual.html (it's the only reason I even noticed this bug). In older Mozilla builds, there was a slight variation between rows. In 2002041308, the shades are rendered the same. It's a small thing, but it's also the kind of thing that can really enrage authors when it happens, and even more when they figure out why it happened. (My first instinct was that my class names were being ignored.) Again, sorry if I'm spamming on a bug unrelated to my problem, but I'm trying to figure if this iws what caused the problem, and if so why we're making this change when it's going to lead to inconsitencies (some would argue visual incompatibilites) between 1.0 and previous milestones.

It seems like this would have caused such a change. However, it seems more important to me than the colors are consistent between Mac and Windows than that we maintain the bug (according to CSS1) that we display colors differently and incorrectly on the Mac. (If you use a 16-bit or whatever depth display you have on Mac, on a Windows machine, do the threshholds look like the ones you used to see or the ones you see now? I'm not sure what the answer will be, but I'd be somewhat surprised if they're consistent. On my 24-bit display I don't see any threshholds, and I suspect that on an 8-bit display there would be a single threshhold within that whole panel, if that.)

> However, it seems more important to me than the colors are consistent between > Mac and Windows than that we maintain the bug (according to CSS1) that we > display colors differently and incorrectly on the Mac. For the record, due to bug 75133 the colors are still incorrect but in a different way on Macs whose display gamma is different from value that is hard-coded in Mozilla. This can make the colors quite dark on systems where the display gamma is over 1.8 and then Mozilla applies the darkening "correction" on top of that.

just a few comments on the results of this for the Mac. I believe we applied an overcorrection to the gifs and html colors in both content and chrome on mac. since the modern theme and all web sites are designed to fit between 1.8 and 2.2 as a compromise to windows and mac gammas, what we see today on the mac is tremendously oversaturated. I am not sure the best way to go about fixing it but it seems like we may just need to lower the hard coded gamma setting, or turn off color correction of gif and html, or any inherently uncorrectable format to stay true to what the designer intended. then again, it wouldn't be nice to have discrepencies between html and css either... just having PNG and CSS match is more important for designers IMHO

As I pointed out in my (slightly testy) newsgroup posting that triggered this bug (message-ID <8p6im9$4v21@secnews.netscape.com>, cross-posted to netscape.public.mozilla.ui, netscape.public.dev.skins, and netscape.public.mozilla.xpfe on 2000-09-06), designers doing "gamma by eye" on the Mac platform are almost guaranteed to be doing the wrong thing--i.e., that methodology is fundamentally in conflict with specifications that require the sRGB color space (unless all of the Macs in question have ColorSync and are set up to use the sRGB color space). In particular, the absolute "brightness level" was wrong by design and shouldn't be a concern when _correcting_ the spec-conformance bug (which is the point of this bug report and its associated patches). The primary concern for backward compatibility should be that unlabelled images and (previously uncorrected) HTML/CSS colors that used to match, still do match. So if the browser used for the "gamma by eye" approach doesn't do any gamma correction at all, then the colors that matched then should still match now (with the exception of PNGs with gAMA chunks that were created that way, but there aren't any, at least until 8415 is resolved--right?). Anyway, that much works as intended, doesn't it? Marlon's comment implies that the absolute "brightness level" has gotten out of whack somehow--is that true of all 10 cases at the test URL above, or is it really limited only to GIFs and HTML colors? If the correction were working as intended (and assuming the hardcoded Mac gamma value in Mozilla matches the Mac being tested), then the brightness level should be very similar to that of a standard PC when seen side-by-side. Is that not true? IOW, is there a double-correction being applied in the Mac code somehow? Btw, note that "Mac gamma 1.8" is really 1.5 in the sense that corresponds to 2.2 on PCs. (There's a weird 2.61/2.2 fudge factor in there.) Greg

Greg: I would characterize the difference between what i see going from mac to pc as: incredibly huge (btw, using the same brand of monitor on both mac and pc and set with equivalent adjustments). the tests you've given all pass, because they are incapable of revealing the particular problem we're dealing with at the moment. no such test could detect the gamma shift that has occurred since before and after the code was turned on. You would need a way to toggle correction off and on to see this difference. there is a clear problem happening, that's all i am technically capable of telling you. It may be this 'double correcting' you're speaking of, at least it sounds like an accurate description of what i am seeing. Windows looks great btw, good job. to summarize- everthing *is* internally matching, but not cross-platform, which is the desired result

see anything peculiar about this comparison? this image has the benefit of being monitor independent since it was captured by each platform's system screen video dump. It appears that windows is now displying an overcorrected macintosh-like gamma (i.e. looks almost the way it did *on the mac* prior to the gamma patch); and the mac is displaying a double corrected windows gamma (i.e. looks twice as intense as the original uncorrected windows gamma prior to the patch). so it seems neither hue nor brightness levels match across platforms after the patch, when they should be identical. otherwise everything else is fine. the test cases pass with flying colors (literally) and no other browser can claim to do that.

> see anything peculiar about this comparison? this image has the benefit of > being monitor independent since it was captured by each platform's system > screen video dump. Well, it seems like the Macintosh video dump utility might be doing some sort of gamma correction, although the numbers seem quite small it could have something to do with rounding errors in the tool that merged the images. (How were the images merged into a single image? Did they have gamma specified in the image beforehand? Afterwards? And do you have a gamma value specified in your system settings on the Mac that's different from the default?) FWIW, the numbers gimp is telling me for the tree header (which is a constant color, unlike most of the image) are: R G B Mac before: 199 208 217 Mac after: 178 190 202 Windows: 198 211 222 The Mac before and Windows numbers seem too close together to reflect what was really going on then.

> It appears that windows is now displying an overcorrected > macintosh-like gamma (i.e. looks almost the way it did *on the mac* prior to > the gamma patch) The patch should not have changed the behavior on Windows at all, and I don't think it did. Why do you think the appearance is different?

>Well, it seems like the Macintosh video dump utility might be doing some sort >of gamma correction, nope, not using a utility it is being done by the OS. it sends exactly what is being rendered by the system to a file. >(How were the images merged into a single image? Did they have gamma specified >in the image beforehand? Afterwards? And do you have a gamma value specified in >your system settings on the Mac that's different from the default?) not that it matters since all the images were produced in the same way, the example in the comparison are are all correct relative to each other and relative to the the gamma space of photoshop (which is notoriously suppling a strange gamma value to pngs; however most browsers until now ignore that value so the colors are 'real') anyway, here's how i created the attachment: screen capture from Mac OS (colorsync does not change any values here, since the capture is pre-colorsync), open in photoshop (with no colorspace correction turned on). next step, screen capture from win [print scrn] command pasted into an uncolorcorrected photoshop environment and exported as PNG (with weird gAMA chunk). Transfer to mac, composite images in photoshop (reads it's own weirdness fine). Export as PNG, post in bugzilla. >The Mac before and Windows numbers seem too close together to reflect what was >really going on then. true the numbers are too close when they should be the same, which probably has something to do with the adobe's PNG >The patch should not have changed the behavior on Windows at all, and I don't >think it did. Why do you think the appearance is different? you are correct i jumped to conclusion on that.

> see anything peculiar about this comparison? this image has the benefit of > being monitor independent since it was captured by each platform's system > screen video dump Well, first of all, keep in mind that capturing the framebuffer isn't definitive since we're trying to account for the entire display system, and the fb certainly has no knowledge of the monitor (though all of those behave pretty much the same, aside from color-temperature settings in some higher-end models) and probably not of the lookup table in the graphics card. It probably _does_ include any modifications done by the color-management system (e.g., ColorSync), since CMSes generally sit between the application and the framebuffer. So it's a useful check on that (assuming whatever utility was used to read the memory didn't also get adjusted by the CMS). Thus the fact that the "after" Windows capture looks pretty much the same as the "before" Mac shot is expected: Windows/Linux x86 boxes tend to be pretty close to sRGB already, so no correction is necessary, and that matches the behavior of the "before" code. The fact that the "after" Mac shot is darker is also to be expected: images that look "normal" on a Mac need to be lightened on a PC, and correspondingly images that look normal on a PC (i.e., sRGB images) need to be darkened on a Mac. And, in fact, the "after" Mac shot looks great to me, as displayed on my somewhat too-bright (i.e., Mac-like) Linux display system. The other two images look a bit washed out, which is how I would expect most sRGB chrome would look on a Mac without gamma correction. So I don't see any obvious problems with this attachment. The only definitive test would be to photograph both monitors with exactly the same exposure and aperture and upload those photos for comparison. Obviously that's kind of a pain (not to mention tricky, given the scanning pattern of the electron beam), but I don't have access to a Mac to see for myself. :-/ Greg

>It probably _does_include any modifications done by the color-management system >(e.g., ColorSync), since CMSes generally sit between the application and the >framebuffer. So it's a useful check on that (assuming whatever utility was used >to read the memory didn't also get adjusted by the CMS) ok i will rephrase, the screen capture is unaffected by colorsync - i've tested that before. apple's DigitalColor meter utility is also unaffected by colorsync, so i am not sure where colorsync lies in the application-framebuffer-screencapture relationship, but i know for sure that the screenshot i produced is a relatively accurate representation of the differences. >The fact that the "after" Mac shot is darker is also to be >expected: images that look "normal" on a Mac need to be lightened on a PC, and >correspondingly images that look normal on a PC (i.e., sRGB images) need to be >darkened on a Mac. And, in fact, the "after" Mac shot looks great to me, as >displayed on my somewhat too-bright (i.e., Mac-like) Linux display system. it's most definitely not 'great'. In fact if i apply a 2.2 gamma curve to the original Mac 'before' shot, i don't get a result as dark as what our gamma correction has done for us. As a matter of fact, the difference in brightness is 79 compared to 82 on a scale of 100 (100 is full brightness): 3 point discrepency in brightness is quite substantial. We're also off by one degree on the hue scale. also note that 'lightness' is not a good way to describe gamma conversion because it's not linear, and involves saturation as well. >So I don't see any obvious problems with this attachment. The only definitive >test would be to photograph both monitors with exactly the same exposure and >aperture and upload those photos for comparison. taking photos is not going to be a good test, i recommend we either hire an ISF technican to measure our results or lease a colorimeter ourselves. shouldn't be a problem getting Netscape to do that, since this affects how accurately we render the entire web too.

here's a more dramatic comparison which i hope will get my point across. look at the numbers measured in the blue area - in this case the darkness on the mac has increased by 68%!!! this proves that the problem is not linear. we need to make an extremely clear point here, the judgement: 'looks great' is not yours or mine to make. you can take virtually any image and make it subjectively 'look great' by tweaking the gamma curve in photoshop. but does that mean if i keep boosting gamma left and right that i am making an image look better? ah - nope. In this case i'll take 'accuracy' over 'looks great' any time, because i don't want to mess with intent of some company to produce their trademark color of blue.

btw, since i am not sure what's going on with our PNG reading on builds with this code checked in, you might want to view my PNG attachments in another viewer which won't pick up the faulty photoshop gAMA chunk. try using a previous build or whatever else you have that ignores gAMA

If the gAMA chunks are bogus you can use pngcrush to remove or correct them, and post a new set without them or with corrected gAMA chunks.

bug 137975 appears to be a dup of the over correction issue on Mac.

I'll take future comments over to bug 137975 >in this case the darkness on the mac has increased by 68%!!! this >proves that the problem is not linear. btw, i should correct myself - that's 32% darker not 68 - sorry for my 'fuzzy math' randeg: i couldn't get pngcrush to change gAMA on my windows machine last time i tried.. it did basically nothing, and i don't have a linux box. Know of any other tools on mac or win? Debabelizer 5 is out perhaps it corrected previous png problems

Marlon: Glenn's been busy with libpng releases this week, but basically you do this: pngcrush -rem gAMA -d outputdir foo.png That will put the modified foo.png in outputdir. I don't think the -rem option is case-sensitive, so "gama" should work, too. Greg

I'm confused now. In 2002041308, there was the reported shifting in shading levels on a "thousands of colors" display. In RC1, there isn't, which means the CSS-based color shading seems to be back to its 0.9.9 behavior. The attachments show that Before and Windows After match fairly well, whereas Mac After does not. That seems like a step backwards, since we're going from a close match to a noticeable separation. Plus, when I load the attached PNGs into IE5.1.3/Mac and RC1/Mac, the images are clearly different shades in the two browsers; RC1 yields darker RGB levels than IE when the pixels are checked. When comparing the same images between IE5.1.3/Mac to 0.9.9, the shades look the same and in fact have the same on-screen RGB values. (I used ZoomLens to compare the RGB values at each step.) Unless someone can clearly explain why this is a needed step in the real world (as in: site performance/appearance will be improved by this patch), and show that it isn't making the overall problem worse, my recommendation is that we push this patch back to 1.1. I recognize that PNG is not a widespread format, but this is a potentially major change that could affect all color-handling, and so far the evidence I see is that it's a change for the worse.

Eric, unless I'm mistaken, this hasn't been checked in on the branch yet. Which explains why you're seeing the 0.9.9 behavior on RC1... Note this bug is on the "Make RC2 not suck" bug.

I still think we definitely want this for 1.0. I question the validity of the images because image editing software and transferring files from system to system is highly unlikely to reflect what's actually displayed. There is a possibility that we need to change the mac gamma correction constant to something weaker (i.e., closer to 2.2), but all the experts seem to be saying that we should be doing some correction on the mac. *If* the number we're using is wrong (I'm not yet convinced by the comments here), we should fix it, rather than using the incorrect number only for PNG and MNG images and doing all other colors with no correction at all (which is also wrong). (The comments above, in comment 151, suggest to me that PNG and CSS colors are now matching, which would mean that an incorrect gamma value seems like the only possibility of what went wrong, if anything is wrong. These tests do pass, right?) Has anyone compared our rendering of PNGs to IE5/Mac's to see whether the same gamma correction is used? Can anyone point to documentation on what the correct correction for Mac should be?

Adding bug 86925 as being blocked by this bug. http://www.debian.org/ looks good to me (although I can't check the exact values) with trunk build 2002041903 on mac os x with default ColorSync settings.

David asked in comment 165, "Has anyone compared our rendering of PNGs to IE5/Mac's to see whether the same gamma correction is used?" Yes. See my attachments (in comments 19 and 20 of that bug) to bug 75133. I'm less concerned about this than I was, since my RC1 comparisons should be thrown out (I was mistaken about the patch being applied there). Unfortunately, the attachments in 75133 do show small but noticeable color differences between IE5.1.3 and the 2002041903 trunk build, so I still have some concern. If we get the desired gamma correction and not introduce inconsistencies between browsers, I'll happily withdraw the opposition expressed in comment 163.

I just posted a lengthy comment and proposal/request for Mac sample code in bug 75133 comment 21 ( http://bugzilla.mozilla.org/show_bug.cgi?id=75133#c21 , in case bugzilla's autolinking doesn't work across bug boundaries). This is purely for determining the precise location and nature of the bug--i.e., is it IE or Moz or both?--which is important to know before attempting to fix it. Greg

I'm coming in here without really having read any of the previous comments. I thought I'd quickly add some "user feedback." I released Chimera on the Mac with this gamma correction included, and I'm receiving tons of complaints and feedback that Web sites are now "way too dark." Given the volume of the complaints, it's clear there's a problem on the Mac with the current settings, but I'll leave it to smarter people than I to figure out how to fix the problem.

Possible ugly compromise: perform no correction on jpeg/gif/css/etc... and always pass in a gamma of 2.2 for libpng and libmng. This would keep the overbright images that people are used to on the Mac and still allow *NG colors to be matched.

Just to add more information, see attachment 80399 [details] (to bug 75133) where I compare IE5.1.3/Mac and 2002041903 under MacOS9.1-- I haven't installed OS X yet-- and find that straight HTML+CSS with no images of any kind yields different shades of the same colors.

I don't know a whole lot about gamma correction, so forgive me if this is a stupid idea. If Mac users like the way the web renders *now*, wouldn't it be possible to choose a Gamma value that gives the equivalent result to what they see today for GIFs, HTML colors and CSS colors? That way PNGs would get gamma-corrected to match the existing colors that they seem to be happy with. This may not be technically correct, but it would probably give the desired effect?

Feedback on Chimera 0.2.3 for OS X: "Browser is still showing heaps of promise and I can use it most of the time as my browser with few problems. The only problem is the digusting new gamma settings which render images like a 10 year old PC!....UGH! " "Shows great promise... but the new gamma settings make everything seem too dark and sludgy" "The gamma in 0.2.3 is significantly darker - not great for us web designers that already have our monitors set to a windows gamma.... Otherwise, this is one great browser" "Very promising and fast. However, 0.2.3 now displays everything darker (windows gamma?)." etc. The complaints are all the same: "too dark". :)

As to comment 173, at least one of those four must be seeing double correction. I think this is feature is an improvement (OS X). http://www.debian.org/ looks better, and the insect image http://bugzilla.mozilla.org/ does not look as horribly washed out, which hides some of the brutal artifacting (and the moz dino looks better red than orange). The darker chrome is pleasant as well.

W.r.t. the third item in comment 173, that's a known bug in the implementation: the code assumes the system has a standard Mac gamma setting (1.5, a.k.a. "1.8"), and the user has set a standard PC setting (presumably 2.2, a.k.a. "2.61"). Not much to be done there except figure out how to extract the system gamma from the ICC profile returned by the ColorSync API. W.r.t. comment 170 (basically the same as 172), that's probably a reasonable option--but it should be selectable, and insofar as it's technically incorrect, it should _not_ be the default option. But before much of anything changes, it would be *really* good to determine whether this is working as intended or not. Given the Mac MSIE PNG results, it sounds like it's a simple bug in Moz's Mac front end. Don't any of the Mac folks have (or know someone who has) basic C code to display a bitmap using the standard Mac APIs? (bug 75133) Greg

Unsure if this is related, but on windows all images with a white background are showing up with a green/pink background, and show up when the page background is white. The easiest testcase for me is google, where the head image has a white background, along with the page. I shouldn't be able to see the where the image ends and the page starts, but I can, because of the image not being true white. Some screenshot comparisons: http://www.mozillazine.org/jason/white.png - shows how mozilla shows the image, and how a normal image program shows it; http://www.mozillazine.org/jason/white.png - shows the difference between the page and the image whites.

Why does bug 135634 depend on this and not bug 75133?

Can someone tell me an easy way of disabling gamma correction in my build so that I can see how it was behaving before the correction? I'd also like to disable it in Chimera releases until these Mac problems are resolved. I don't want to back the whole huge patch out. I just want to know if there's a simple hack I can do in my tree to revert to the old behavior.

hyatt: just change nsScreenMac::GetGammaValue() to set a gamma of 2.2. Alternatively modify NS_InitializeGamma() to set a linear ramp.

The comments here and in bug 75133 make it clear that the gamma in mozilla needs more thought. Since hyatt is turning it off for Chimera, the gamma code is essentially a wasteful no-op at this point. This backout patch removes correction for gif/jpeg/bmp/ico/css/html, and sets a display gamma of 2.2 for libpng and libmng.

Maybe it wouldn't be turned off for Chimera if we used a constant that led to less gamma correction?

Comment on attachment 81029 [details] [diff] [review] gamma backout patch r=dbaron

I believe we should try a few days with my fix on bug 75133 and see how this is received before backing out tor's changes.

Comment on attachment 81029 [details] [diff] [review] gamma backout patch Sorry to see us back out -- what is the plan to get the right fix in, once we have backed out? rs=brendan@mozilla.org

Comment on attachment 81029 [details] [diff] [review] gamma backout patch a=asa (on behalf of drivers) for checkin to the 1.0 branch

gamma correction backed out on the trunk. Asa: gamma correction never made it to the 1.0 branch.

ahh. sorry for my confusion. removing from the RC2 list.

If gamma is going to be wrong on the branch, let's at least be consistently wrong. Patch sets a display gamma of 2.2 in the PNG and MNG decoders regardless of platform. This is the same as what's on the trunk since the gamma backout.

Comment on attachment 81936 [details] [diff] [review] branch gamma patch r=dbaron

Comment on attachment 81936 [details] [diff] [review] branch gamma patch sr=brendan@mozilla.org /be

Comment on attachment 81936 [details] [diff] [review] branch gamma patch a=roc+moz

Attachment 81936 [details] [diff] checked into branch.

adding fixed1.0.0 keyword (branch resolution). This bug has comments saying it was fixed on the 1.0 branch and a bonsai checkin comment that agrees. To verify the bug has been fixed on the 1.0 branch please replace the fixed1.0.0 keyword with verified1.0.0.

cc'ing myself

tor: is there a plan to revive this patch for the trunk? Gerv

Gerv: no immediate plans to do so - any reason you ask?

Because (as far as I can tell) it what is blocking any sort of PNG chrome effort. Have I got the wrong end of the stick? Gerv

I am somehow unable to reproduce the bug in a build earlier than 05/02/2002 (that is when the patch for the fix was moved into branch, according to comment 192), on all platforms. Previous build, as well as, the current branch builds display the testcases (mentioned in the above url) as desired (with gamma correction). Anyone can tell me which build to look at to reproduce the bug before i mark it verified for the branch

tor: What's the status on this?

tor, any update 4 years later? Does having lcms on the trunk help with this bug?

>Does having lcms on the trunk help with this bug? Looking at the colorcube URL above with FF-3.0-beta2, it seems that PNG images with the iCCP chunk are improved but not perfect, while PNG images with the sRGB chunk are worse (similar to iCCP), when gfx.color_management.enabled is "true". Incredibly, even pure white is rendered incorrectly when the sRGB or iCCP chunk is present.

Is this still a blocker for turning color management on by default?

The referenced URL is still displayed incorrectly for me using FF-3.0 with color_management enabled and using the default Windows sRGB screen profile. Of the 216 colors on the color cube, only pure black is displayed correctly. Even pure white is wrong. I consider it still a blocker. Glenn

It will be interesting to see if Argyllcms does a better job of rendering this URL. See bug #445468.

I recently landed bug 452676, which fixed some problems in how we were handling gAMA chunks (among other things) when color management was turned on. Looking at the color squares, they all look perfect (including the sRGB ones), except for the iCCP ones, which look off in a few places. Glenn, how do they look for you? Are the issues you were seeing before resolved?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080909032504 Minefield/3.1b1pre, gfx.color_management=1, Windows-supplied sRGB profile, looks good except as you say there are some very faint differences on the iCCP tests.

On my Mac, with gfx.color_management=1, and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080909020334 Minefield/3.1b1pre Perfect, except for the iCCP ones. I have a Camino 2.0a1pre/gecko 1.9.0.3 pre build next to it to compare. Some squares are off on 1.9.0.3, but correct on 1.9.1b1pre.

Resolving this mammoth bug as fixed. Discussion about the faint differences with iCCP chunks can continue in bug 454688.

Google chrome apparently has color management enabled but the test URL looks just the same as unpatched Firefox 3.0.1 with color management mode 1. Evidently it needs the patch from bug #452676.