Hello, I noticed that the transparent (alpha 24 bit) PNG use on this website: http://www.monauto.fr/fiche_auto.php?id=3 use for the alternance lines in the tables, displays in a strange gray colour on Tiger's Sarfari 2 instead of blue (it works as it should on Panther's Safari 1.x, though). This is the complete url of the png: http://www.monauto.fr/images/oddbg.png It works perfectly well in FireFox under Tiger, so it looks like a WebCore bug. Of course it works very well on Windows machines, but who cares? But there's also something strange. The PNG file was generated by Photoshop. If we use another soft like Gimp or similar, the bug disapears. Best regards, Benjamin Frere
You may have the explanation here : http://hsivonen.iki.fi/png-gamma/ I'm curently redesigning my corporate website (http://sports.fr/) and it's a pain to work with PNGs - first I need to remove the color profile so they work fine for most browsers (working with photoshop cs) - and then it doesn't work well on safari pre 10.4 as it seems to apply a default color profile to the PNGs (which is not correct and darken the colors). In your case I'd say that the previous Safari is not correct in its PNG handling, and the new one is. NB : PNG 8bit have the same issue.
I'd like to have other opinions about that. _Every_ other browser I've tried handles it correctly. If Safari really does something better than all those other browsers (which I doubt), maybe it's a case of trying too much.
Well, I don't think this issue is about PNG gamma. Only 1x1 images are affected - resaving in Photoshop as any other size fixes the problem, and resaving as 1x1 again makes it appear back (so, the workaround is to use larger images). This appears to be fixed in current nightly builds. I'm not closing as WORKSFORME yet because: 1) someone more familiar with image decoding might want to take a look at this bug; 2) looks like the problem remains for images loaded using a direct URL, not as a part of a document (though it's hard to tell, even using Pixie :-))) ); 3) might be worth investigating in order to file a Radar.
Created attachment 7072 [details] test case Besides (1)-(3), landing a test case could be useful.
We do have an optimization specific to 1x1 images that was new to Tiger. Sounds like that was going wrong.
Comment on attachment 7072 [details] test case I think the description "Should be uniform color" is a little terse, but seems like a good test.
(In reply to comment #3) > 2) looks like the problem remains for images loaded using a direct URL, not as > a part of a document (though it's hard to tell, even using Pixie :-))) ); The standalone image case is still using the old code -- that's a temporary situation so we don't have to worry about it for much longer, so I think there's no need to worry about this. > 3) might be worth investigating in order to file a Radar. I'm pretty sure there's no need. The 1x1 code is inside WebKit.
The 1x1 case optimization was restored today (r15529), and so was this bug. Reopening. Furthermore, my test case was apparently not quite right - I'm now seeing that the colors didn't quite match even with a yesterday's nightly (presumably because of PNG color correction). Should be easy to fix by removing colorspace-related chunks.
Comment on attachment 7072 [details] test case clearing the review flag
Created attachment 9583 [details] fixed test case
This bug turns out to be about premultiplied vs. non-premultiplied alpha! The m_solidColor member of Image is supposed to be non-premul, but in Image::checkForSolidColor() it is initialized from a premul bitmap context. Dividing out by the alpha (if it's non-zero) in checkForSolidColor() fixes the test case. A more straightforward approach (and more accurate, if the image format is non-premul) would be to use a non-premul bitmap context, however when I change the kCGImageAlphaPremultipliedLast to kCGImageAlphaLast in the CGBitmapContextCreate I get a NULL context -- I have no idea why. Next thing I'm going to try is use 8-bit unsigned instead of float components and see if that changes anything.
Created attachment 9764 [details] Use non-premul, 8-bit integer bitmap context Patch is missing a test and a change log, but I'd like to get feedback on switching to integers (is there any reason to use floats when you convert to int anyway?) and on whether I should use another type for the 8-bit quantity instead of unsigned char.
Comment on attachment 9764 [details] Use non-premul, 8-bit integer bitmap context This is totally fine. r=me
Comment on attachment 9764 [details] Use non-premul, 8-bit integer bitmap context We don't need kCGBitmapByteOrder32Host any more since the components are now single bytes -- byte order does not matter in that case. Should probably omit sizeof(unsigned char) -- that's defined by the language to be 1 and I think it's clear enough to just pass 8.
(In reply to comment #12) > whether I should use another type for the 8-bit quantity instead of unsigned char I thnk unsigned char is fine for this.
Created attachment 9773 [details] Patch, including change log and ap's test Also addressed Darin's comments.
(In reply to comment #16) > Patch, including change log and ap's test The test is supposed to replace fast/replaced/img-1x1.html.
Comment on attachment 9764 [details] Use non-premul, 8-bit integer bitmap context Clearing review flag.
Comment on attachment 9773 [details] Patch, including change log and ap's test r=me
Comment on attachment 9773 [details] Patch, including change log and ap's test I didn't test the patch correctly. Alexey just did, and it turns out that just like in the float case, you can't create a bitmap context with non-premul alpha http://developer.apple.com/qa/qa2001/qa1037.html .
(In reply to comment #20) > I didn't test the patch correctly. Alexey just did, and it turns out that just > like in the float case, you can't create a bitmap context with non-premul alpha > http://developer.apple.com/qa/qa2001/qa1037.html . Maybe we can get a pixel out of the bitmap in some way other than rendering it.
Created attachment 9776 [details] Undo the premultiplication > Maybe we can get a pixel out of the bitmap in some way other than rendering it. In case that doesn't work out, you can try to undo the premultiplication. For image formats that are natively premultiplied, this is as accurate as you can get anyway.
Comment on attachment 9776 [details] Undo the premultiplication I couldn't find a way to extract color from a CGImage without rendering it, so I'm proposing this fix for now.
Comment on attachment 9776 [details] Undo the premultiplication OK. Lets do it this way, then. r=me
Committed revision 15758.