RESOLVED FIXED 54919
WebCore should retrieve unclamped frame delays from ImageIO
https://bugs.webkit.org/show_bug.cgi?id=54919
Summary WebCore should retrieve unclamped frame delays from ImageIO
Mark Rowe (bdash)
Reported 2011-02-21 17:37:46 PST
In bug 26455 we changed WebCore’s animated GIF frame delay clamping to match Firefox. However, ImageIO implements its own frame delay clamping on the value it returns for the kCGImagePropertyGIFDelayTime. We should switch to retrieving the raw, unclamped frame delay from ImageIO so that we really do match Firefox when using ImageIO.
Attachments
Patch (3.20 KB, patch)
2011-02-21 17:44 PST, Mark Rowe (bdash)
darin: review+
Mark Rowe (bdash)
Comment 1 2011-02-21 17:44:10 PST
Darin Adler
Comment 2 2011-02-21 17:48:18 PST
Comment on attachment 83246 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=83246&action=review > Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp:319 > + // Look for the unclamped frame delay if it exists. The phrasing here is awkward. We always look for it, but only find it if it exists.
Alexey Proskuryakov
Comment 3 2011-02-21 17:49:55 PST
Comment on attachment 83246 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=83246&action=review > Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp:46 > +static const CFStringRef kCGImagePropertyGIFUnclampedDelayTime = CFSTR("UnclampedDelayTime"); I suggest omitting the unnecessary "static". I see kCGImagePropertyGIFUnclampedDelayTime in some versions of public Snow Leopard headers. Will this break the build for those who have a new version? > Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp:324 > + CFNumberGetValue(num, kCFNumberFloatType, &duration); > + > + // Fall back to the clamped frame delay if the unclamped frame delay does not exist. > + else if (CFNumberRef num = (CFNumberRef)CFDictionaryGetValue(typeProperties, kCGImagePropertyGIFDelayTime)) The blank line could confuse someone. Maybe move comments inside the block (and add braces then)?
Mark Rowe (bdash)
Comment 4 2011-02-21 18:08:07 PST
(In reply to comment #3) > (From update of attachment 83246 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=83246&action=review > > > Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp:46 > > +static const CFStringRef kCGImagePropertyGIFUnclampedDelayTime = CFSTR("UnclampedDelayTime"); > > I suggest omitting the unnecessary "static". > > I see kCGImagePropertyGIFUnclampedDelayTime in some versions of public Snow Leopard headers. Will this break the build for those who have a new version? It will. I’ll change this to always provide and use our own version of this constant.
Mark Rowe (bdash)
Comment 5 2011-02-21 18:16:36 PST
Fixed in r79277.
Note You need to log in before you can comment on or make changes to this bug.