Summary: | WebCore should retrieve unclamped frame delays from ImageIO | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Rowe (bdash) <mrowe> | ||||
Component: | WebCore Misc. | Assignee: | Mark Rowe (bdash) <mrowe> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | OS X 10.6 | ||||||
Attachments: |
|
Description
Mark Rowe (bdash)
2011-02-21 17:37:46 PST
Created attachment 83246 [details]
Patch
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. 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)? (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. |