WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Mark Rowe (bdash)
Comment 1
2011-02-21 17:44:10 PST
Created
attachment 83246
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug