The current impl assumes it can read the address returned by the device's bitmap, and had it to the image encoders. The fix is to check if we need to call device->readPixels() first, which will always succeed, even if the device is backed by some other means than a CPU bitmap (e.g. opengl fbo)
Created attachment 82008 [details] fix for 54224 - call readPixels() if need be, if the address from getPixels() is null
Comment on attachment 82008 [details] fix for 54224 - call readPixels() if need be, if the address from getPixels() is null View in context: https://bugs.webkit.org/attachment.cgi?id=82008&action=review R=me > WebCore/ChangeLog:9 > + No new tests. Existing canvas tests exercise this. Do we know which test(s) cover this?
Don't you need to unlock the bitmap to be able to access the pixel data? iirc, there were cases where a perfectly valid memory-backed bitmap returns 0 for getPixels() because the bitmap wasn't locked. Maybe that doesn't apply in this specific case, depends on what bitmap the canvas is created on.
Created attachment 82014 [details] add explicit test that exercises this code path, explicitly lockPixels() before calling getPixels()
Comment on attachment 82014 [details] add explicit test that exercises this code path, explicitly lockPixels() before calling getPixels() View in context: https://bugs.webkit.org/attachment.cgi?id=82014&action=review Great! Thanks for adding the test reference, it'll be useful to have this information if that test every breaks in the future. Would you like me to c-q+ this as well? > WebCore/platform/graphics/skia/ImageBufferSkia.cpp:354 > + // if we can't see the pixels directly, call readPixels() to get a copy. > + // this could happen if the device is backed by a GPU. > + bitmap.lockPixels(); // balanced by our destructor, or explicitly if getPixels() fails nit: In WebKit, comments start with an uppercase letter and end with a period.
can this be queued for commit? thanks.
Comment on attachment 82014 [details] add explicit test that exercises this code path, explicitly lockPixels() before calling getPixels() Clearing flags on attachment: 82014 Committed r78356: <http://trac.webkit.org/changeset/78356>
All reviewed patches have been landed. Closing bug.