Skip to content

Commit

Permalink
Merge pull request #2847 from dreampiggy/bugfix_mac_gif_loop_count
Browse files Browse the repository at this point in the history
Fix the macOS SDAnimatedImageRep to match Netscape standard of GIF loop count, which should use 1 when there are no loop count information
  • Loading branch information
dreampiggy committed Sep 21, 2019
2 parents d7ce577 + d20b25b commit 20c8adc
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 9 deletions.
3 changes: 2 additions & 1 deletion SDWebImage/Core/SDAnimatedImageRep.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
#if SD_MAC

/**
A subclass of `NSBitmapImageRep` to fix that GIF loop count issue because `NSBitmapImageRep` will reset `NSImageCurrentFrameDuration` by using `kCGImagePropertyGIFDelayTime` but not `kCGImagePropertyGIFUnclampedDelayTime`.
A subclass of `NSBitmapImageRep` to fix that GIF duration issue because `NSBitmapImageRep` will reset `NSImageCurrentFrameDuration` by using `kCGImagePropertyGIFDelayTime` but not `kCGImagePropertyGIFUnclampedDelayTime`.
This also fix the GIF loop count issue, which will use the Netscape standard (See http://www6.uniovi.es/gifanim/gifabout.htm) to only place once when the `kCGImagePropertyGIFLoopCount` is nil. This is what modern browser's behavior.
Built in GIF coder use this instead of `NSBitmapImageRep` for better GIF rendering. If you do not want this, only enable `SDImageIOCoder`, which just call `NSImage` API and actually use `NSBitmapImageRep` for GIF image.
This also support APNG format using `SDImageAPNGCoder`. Which provide full alpha-channel support and the correct duration match the `kCGImagePropertyAPNGUnclampedDelayTime`.
*/
Expand Down
5 changes: 4 additions & 1 deletion SDWebImage/Core/SDAnimatedImageRep.m
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ - (instancetype)initWithData:(NSData *)data {
}
if (CFStringCompare(type, kUTTypeGIF, 0) == kCFCompareEqualTo) {
// GIF
// Do nothing because NSBitmapImageRep support it
// Fix the `NSBitmapImageRep` GIF loop count calculation issue
// Which will use 0 when there are no loop count information metadata in GIF data
NSUInteger loopCount = [SDImageGIFCoder imageLoopCountWithSource:imageSource];
[self setProperty:NSImageLoopCount withValue:@(loopCount)];
} else if (CFStringCompare(type, kUTTypePNG, 0) == kCFCompareEqualTo) {
// APNG
// Do initilize about frame count, current frame/duration and loop count
Expand Down
7 changes: 0 additions & 7 deletions Tests/Tests/SDImageCoderTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,7 @@ - (void)test12ThatGIFWithoutLoopCountPlayOnce {
NSString * testImagePath = [[NSBundle bundleForClass:[self class]] pathForResource:@"TestLoopCount" ofType:@"gif"];
NSData *testImageData = [NSData dataWithContentsOfFile:testImagePath];
UIImage *image = [SDImageGIFCoder.sharedCoder decodedImageWithData:testImageData options:nil];
#if SD_MAC
// TODO, macOS's `NSBitmapImageRep` treate this loop count as 0, this need to be fixed in next PR.
expect(image.sd_imageLoopCount).equal(0);
#else
expect(image.sd_imageLoopCount).equal(1);
#endif
}

- (void)test13ThatHEICWorks {
Expand Down Expand Up @@ -194,8 +189,6 @@ - (void)verifyCoder:(id<SDImageCoder>)coder

- (void)test16ThatImageIOAnimatedCoderAbstractClass {
SDImageIOAnimatedCoder *coder = [[SDImageIOAnimatedCoder alloc] init];
NSString * testImagePath = [[NSBundle bundleForClass:[self class]] pathForResource:@"TestImage" ofType:@"png"];
NSData *testImageData = [NSData dataWithContentsOfFile:testImagePath];
@try {
[coder canEncodeToFormat:SDImageFormatPNG];
XCTFail("Should throw exception");
Expand Down

0 comments on commit 20c8adc

Please sign in to comment.