This work is needed for asynchronous image decoding. By default an image frame is decoded only when it'e needed. Usually the image frame is needed when it is drawn. If the image decoding is done on the main thread, two things might happen. The main thread is blocked such that an activity, like scrolling, is not smooth. Secondly the image decoding is slow such that rendering a layout frame is done in more than 16ms, so we miss drawing a frame. This bug should fix two things. It should add the option to immediately decode an image frame. This should happen on a different thread earlier than the drawing time. The BitmapImage should also control when the image frame is deleted from memory. Currently CG can cache an image frame even if WebKit does not have a reference to it. The only way to guarantee an image frame is removed from memory is to delete the reference to the CGImageSourceRef.
Created attachment 291298 [details] Patch
Created attachment 291300 [details] Patch
Comment on attachment 291300 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291300&action=review > Source/WebCore/ChangeLog:3 > + [CG] Add the option to immediacy decode an image frame and control its memory caching s/immediacy/immediately/ > Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:80 > + void append(const void *key, const void *value) > + { > + ASSERT(numOptions < basicOptions + extraOptions); > + keys[numOptions] = key; > + values[numOptions++] = value; > + } > + > + static const CFIndex basicOptions = 3; > + static const CFIndex extraOptions = 3; > + > + const void* keys[basicOptions + extraOptions]; > + const void* values[basicOptions + extraOptions]; > + CFIndex numOptions { 0 }; This is all very sad, maybe we should just use CFMutableDictionary? > Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:367 > + image = adoptCF(CGImageSourceCreateImageAtIndex(m_nativeDecoder.get(), index, imageSourceOptions(subsamplingLevel, decodingMode).get())); Maybe factor out creation of imageSourceOptions for cleanliness' sake?
Created attachment 291310 [details] Patch
Comment on attachment 291310 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291310&action=review > Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:351 > + if (decodingMode == DecodingMode::OnDemand) > + image = adoptCF(CGImageSourceCreateImageAtIndex(m_nativeDecoder.get(), index, options)); > + else > + image = adoptCF(CGImageSourceCreateThumbnailAtIndex(m_nativeDecoder.get(), index, options)); Do we need both the call to CGImageSourceCreateThumbnailAtIndex(), AND the kCGImageSourceCreateThumbnailFromImageAlways = kCFBooleanTrue in the options?
Comment on attachment 291310 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291310&action=review >> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:351 >> + image = adoptCF(CGImageSourceCreateThumbnailAtIndex(m_nativeDecoder.get(), index, options)); > > Do we need both the call to CGImageSourceCreateThumbnailAtIndex(), AND the kCGImageSourceCreateThumbnailFromImageAlways = kCFBooleanTrue in the options? YES, we do. Deleting one of them does not force immediate image decoding. The returned CGImageRef does not hold the decoded data. So we end up with synchronous image decoding which happens only at drawing time.
Created attachment 291322 [details] image-decode-CGImageSourceCreateThumbnailAtIndex-only.trace This is the Instruments trace when calling CGImageSourceCreateThumbnailAtIndex() with passing the flag kCGImageSourceCreateThumbnailFromImageAlways. GIFReadPlugin::decodeGIFFrame() is called from CG::DrawOp() in the drawing thread.
(In reply to comment #7) > Created attachment 291322 [details] > image-decode-CGImageSourceCreateThumbnailAtIndex-only.trace > > This is the Instruments trace when calling > CGImageSourceCreateThumbnailAtIndex() with passing the flag > kCGImageSourceCreateThumbnailFromImageAlways. I meant "without passing ...." > > GIFReadPlugin::decodeGIFFrame() is called from CG::DrawOp() in the drawing > thread.
Created attachment 291323 [details] image-decode-kCGImageSourceCreateThumbnailFromImageAlways-only.trace This is the Instruments trace when calling CGImageSourceCreateImageAtIndex() with passing the flag kCGImageSourceCreateThumbnailFromImageAlways. GIFReadPlugin::decodeGIFFrame() is called from CG::DrawOp() in the drawing thread
Comment on attachment 291310 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291310&action=review >>> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:351 >>> + image = adoptCF(CGImageSourceCreateThumbnailAtIndex(m_nativeDecoder.get(), index, options)); >> >> Do we need both the call to CGImageSourceCreateThumbnailAtIndex(), AND the kCGImageSourceCreateThumbnailFromImageAlways = kCFBooleanTrue in the options? > > YES, we do. Deleting one of them does not force immediate image decoding. The returned CGImageRef does not hold the decoded data. So we end up with synchronous image decoding which happens only at drawing time. I see. kCGImageSourceCreateThumbnailFromImageAlways says to always create the thumbnail from the full image (not any thumbnail embedded in the image file).
Comment on attachment 291310 [details] Patch Clearing flags on attachment: 291310 Committed r207182: <http://trac.webkit.org/changeset/207182>
All reviewed patches have been landed. Closing bug.
(In reply to comment #11) > Comment on attachment 291310 [details] > Patch > > Clearing flags on attachment: 291310 > > Committed r207182: <http://trac.webkit.org/changeset/207182> It broke the Apple Windows build, see build.webkit.org for details.
(In reply to comment #13) > (In reply to comment #11) > > Comment on attachment 291310 [details] > > Patch > > > > Clearing flags on attachment: 291310 > > > > Committed r207182: <http://trac.webkit.org/changeset/207182> > > It broke the Apple Windows build, see build.webkit.org for details. Yes. This is true. The error is: C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\cg\ImageDecoderCG.cpp(77): error C2065: 'kCGImageSourceShouldCacheImmediately': undeclared identifier [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] Working on a fix.
Fix the Windows build: Committed r207216: <http://trac.webkit.org/changeset/207216>