The implementation of the above functions is void BitmapImage::checkForSolidColor() { m_checkedForSolidColor = true; } This should be correctly implemented so it sets m_isSolidColor to true when you have 1x1 image with 1 frame so it can make use os optimization in Image.cpp that does a color fill when you're drawing a 1x1 image.
See http://code.google.com/p/chromium/issues/detail?id=80011 -- same issue, i think. If so, pls coordinate w/twiz.
possible duplicate of http://code.google.com/p/chromium/issues/detail?id=80011
See also related bug https://bugs.webkit.org/show_bug.cgi?id=58926
Created attachment 91563 [details] Patch
Comment on attachment 91563 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91563&action=review > Source/WebCore/platform/graphics/skia/ImageSkia.cpp:471 > + WebCore::NativeImageSkia* frame = frameAtIndex(currentFrame()); Is there a possibility that I need to check for a decode complete status here as well?
Comment on attachment 91563 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91563&action=review > Source/WebCore/platform/graphics/skia/ImageSkia.cpp:466 > + if (frameCount() != 1) the other ports seem to implement this check as frameCount() > 1. is there any significance to this being different? >> Source/WebCore/platform/graphics/skia/ImageSkia.cpp:471 >> + WebCore::NativeImageSkia* frame = frameAtIndex(currentFrame()); > > Is there a possibility that I need to check for a decode complete status here as well? it seems like this codepath wouldn't work if the bitmap was not decoded yet or if insufficient data was available to decode the image, right? the other ports null check the image here, at least. can you see in what cases the null check fails in the other ports and see what happens here? > Source/WebCore/platform/graphics/skia/ImageSkia.cpp:472 > + frame->lockPixels(); don't we have a pixel autolocker class to avoid the manual lock/unlock?
Created attachment 91590 [details] Patch
Comment on attachment 91563 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91563&action=review >> Source/WebCore/platform/graphics/skia/ImageSkia.cpp:466 >> + if (frameCount() != 1) > > the other ports seem to implement this check as frameCount() > 1. is there any significance to this being different? I was trying to be more defensive - in the case that frameCount could be 0. Uploaded a new patch, following the style of the other ports. >>> Source/WebCore/platform/graphics/skia/ImageSkia.cpp:471 >>> + WebCore::NativeImageSkia* frame = frameAtIndex(currentFrame()); >> >> Is there a possibility that I need to check for a decode complete status here as well? > > it seems like this codepath wouldn't work if the bitmap was not decoded yet or if insufficient data was available to decode the image, right? the other ports null check the image here, at least. > > can you see in what cases the null check fails in the other ports and see what happens here? Added a check for null in a new patch. The other ports do not check for completion of decoding of the image. I'm not familiar with this code - is it even possible to receive an incomplete bitmap at this point? >> Source/WebCore/platform/graphics/skia/ImageSkia.cpp:472 >> + frame->lockPixels(); > > don't we have a pixel autolocker class to avoid the manual lock/unlock? Uploaded new patch using the autolocker.
Fringe case, you'll want to check that there are valid pixels after the lock call. if (bitmap.getPixels() == NULL) { // there are no pixels allocated, or the lock failed }
Created attachment 91678 [details] Patch
Created attachment 91686 [details] Patch
Comment on attachment 91686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91686&action=review > Source/WebCore/platform/graphics/skia/ImageSkia.cpp:477 > + m_solidColor = Color(frame->getColor(0, 0)); Removed the clunky construction of Color from individual components, as per recommendation from Stephen White.
Comment on attachment 91686 [details] Patch YES! This is a huge win for some pages.
Comment on attachment 91686 [details] Patch Clearing flags on attachment: 91686 Committed r85422: <http://trac.webkit.org/changeset/85422>
All reviewed patches have been landed. Closing bug.