Merge pull request #3637 from dreampiggy/bugfix/color_at_point

Fix the sd_colorAtPoint/sd_colorsWithRect return wrong value on pre-multiplied CGImage

SDWebImage/Core/SDImageIOAnimatedCoder.m

@@ -184,10 +184,10 @@ static CGImageRef SDImageIOPNGPluginBuggyCreateWorkaround(CGImageRef cgImage) CF
     CGBitmapInfo bitmapInfo = CGImageGetBitmapInfo(cgImage);
     CGImageAlphaInfo alphaInfo = (bitmapInfo & kCGBitmapAlphaInfoMask);
     CGImageAlphaInfo newAlphaInfo = alphaInfo;
-    if (alphaInfo == kCGImageAlphaPremultipliedLast) {
+    if (alphaInfo == kCGImageAlphaLast) {
-        newAlphaInfo = kCGImageAlphaLast;
+        newAlphaInfo = kCGImageAlphaPremultipliedLast;
-    } else if (alphaInfo == kCGImageAlphaPremultipliedFirst) {
+    } else if (alphaInfo == kCGImageAlphaFirst) {
-        newAlphaInfo = kCGImageAlphaFirst;
+        newAlphaInfo = kCGImageAlphaPremultipliedFirst;
     }
     if (newAlphaInfo != alphaInfo) {
         CGBitmapInfo byteOrderInfo = bitmapInfo & kCGBitmapByteOrderMask;
@@ -244,6 +244,8 @@ static BOOL SDImageIOPNGPluginBuggyNeedWorkaround(void) {
 #endif
             }
         }
+        CFRelease(source);
+        CGImageRelease(cgImage);
     });
     
     return isBuggy;

SDWebImage/Core/UIImage+Transform.h

@@ -107,6 +107,7 @@ typedef NS_OPTIONS(NSUInteger, SDRectCorner) {
 
 /**
  Return the pixel color at specify position. The point is from the top-left to the bottom-right and 0-based. The returned the color is always be RGBA format. The image must be CG-based.
+ @note The point's x/y will be converted into integer.
  @note The point's x/y should not be smaller than 0, or greater than or equal to width/height.
  @note The overhead of object creation means this method is best suited for infrequent color sampling. For heavy image processing, grab the raw bitmap data and process yourself.
 
@@ -117,6 +118,7 @@ typedef NS_OPTIONS(NSUInteger, SDRectCorner) {
 
 /**
  Return the pixel color array with specify rectangle. The rect is from the top-left to the bottom-right and 0-based. The returned the color is always be RGBA format. The image must be CG-based.
+ @note The rect's origin and size will be converted into integer.
  @note The rect's width/height should not be smaller than or equal to 0. The minX/minY should not be smaller than 0. The maxX/maxY should not be greater than width/height. Attention this limit is different from `sd_colorAtPoint:` (point: (0, 0) like rect: (0, 0, 1, 1))
  @note The overhead of object creation means this method is best suited for infrequent color sampling. For heavy image processing, grab the raw bitmap data and process yourself.
 

SDWebImage/Core/UIImage+Transform.m

@@ -169,7 +169,32 @@ static inline UIColor * SDGetColorFromRGBA(Pixel_8888 pixel, CGBitmapInfo bitmap
         default: break;
     }
     switch (alphaInfo) {
-        case kCGImageAlphaPremultipliedFirst:
+        case kCGImageAlphaPremultipliedFirst: {
+            if (byteOrderNormal) {
+                // ARGB8888-premultiplied
+                a = pixel[0] / 255.0;
+                r = pixel[1] / 255.0;
+                g = pixel[2] / 255.0;
+                b = pixel[3] / 255.0;
+                if (a > 0) {
+                    r /= a;
+                    g /= a;
+                    b /= a;
+                }
+            } else {
+                // BGRA8888-premultiplied
+                b = pixel[0] / 255.0;
+                g = pixel[1] / 255.0;
+                r = pixel[2] / 255.0;
+                a = pixel[3] / 255.0;
+                if (a > 0) {
+                    r /= a;
+                    g /= a;
+                    b /= a;
+                }
+            }
+            break;
+        }
         case kCGImageAlphaFirst: {
             if (byteOrderNormal) {
                 // ARGB8888
@@ -186,7 +211,32 @@ static inline UIColor * SDGetColorFromRGBA(Pixel_8888 pixel, CGBitmapInfo bitmap
             }
         }
             break;
-        case kCGImageAlphaPremultipliedLast:
+        case kCGImageAlphaPremultipliedLast: {
+            if (byteOrderNormal) {
+                // RGBA8888-premultiplied
+                r = pixel[0] / 255.0;
+                g = pixel[1] / 255.0;
+                b = pixel[2] / 255.0;
+                a = pixel[3] / 255.0;
+                if (a > 0) {
+                    r /= a;
+                    g /= a;
+                    b /= a;
+                }
+            } else {
+                // ABGR8888-premultiplied
+                a = pixel[0] / 255.0;
+                b = pixel[1] / 255.0;
+                g = pixel[2] / 255.0;
+                r = pixel[3] / 255.0;
+                if (a > 0) {
+                    r /= a;
+                    g /= a;
+                    b /= a;
+                }
+            }
+            break;
+        }
         case kCGImageAlphaLast: {
             if (byteOrderNormal) {
                 // RGBA8888
@@ -546,9 +596,11 @@ static inline CGImageRef _Nullable SDCreateCGImageFromCIImage(CIImage * _Nonnull
     }
     
     // Check point
-    CGFloat width = CGImageGetWidth(imageRef);
+    size_t width = CGImageGetWidth(imageRef);
-    CGFloat height = CGImageGetHeight(imageRef);
+    size_t height = CGImageGetHeight(imageRef);
-    if (point.x < 0 || point.y < 0 || point.x >= width || point.y >= height) {
+    size_t x = point.x;
+    size_t y = point.y;
+    if (x < 0 || y < 0 || x >= width || y >= height) {
         CGImageRelease(imageRef);
         return nil;
     }
@@ -570,7 +622,7 @@ static inline CGImageRef _Nullable SDCreateCGImageFromCIImage(CIImage * _Nonnull
     size_t components = CGImageGetBitsPerPixel(imageRef) / CGImageGetBitsPerComponent(imageRef);
     CGBitmapInfo bitmapInfo = CGImageGetBitmapInfo(imageRef);
     
-    CFRange range = CFRangeMake(bytesPerRow * point.y + components * point.x, components);
+    CFRange range = CFRangeMake(bytesPerRow * y + components * x, components);
     if (CFDataGetLength(data) < range.location + range.length) {
         CFRelease(data);
         CGImageRelease(imageRef);
@@ -620,8 +672,8 @@ static inline CGImageRef _Nullable SDCreateCGImageFromCIImage(CIImage * _Nonnull
     }
     
     // Check rect
-    CGFloat width = CGImageGetWidth(imageRef);
+    size_t width = CGImageGetWidth(imageRef);
-    CGFloat height = CGImageGetHeight(imageRef);
+    size_t height = CGImageGetHeight(imageRef);
     if (CGRectGetWidth(rect) <= 0 || CGRectGetHeight(rect) <= 0 || CGRectGetMinX(rect) < 0 || CGRectGetMinY(rect) < 0 || CGRectGetMaxX(rect) > width || CGRectGetMaxY(rect) > height) {
         CGImageRelease(imageRef);
         return nil;

Tests/SDWebImage Tests.xcodeproj/project.pbxproj

@@ -47,6 +47,9 @@
 		326E69482334C0C300B7252C /* TestLoopCount.gif in Resources */ = {isa = PBXBuildFile; fileRef = 326E69462334C0C200B7252C /* TestLoopCount.gif */; };
 		327054E2206CEFF3006EA328 /* TestImageAnimated.apng in Resources */ = {isa = PBXBuildFile; fileRef = 327054E1206CEFF3006EA328 /* TestImageAnimated.apng */; };
 		327054E3206CEFF3006EA328 /* TestImageAnimated.apng in Resources */ = {isa = PBXBuildFile; fileRef = 327054E1206CEFF3006EA328 /* TestImageAnimated.apng */; };
+		3278F5E22B04C1AC0004A6EE /* IndexedPNG.png in Resources */ = {isa = PBXBuildFile; fileRef = 3278F5E12B04C1AC0004A6EE /* IndexedPNG.png */; };
+		3278F5E32B04C1AC0004A6EE /* IndexedPNG.png in Resources */ = {isa = PBXBuildFile; fileRef = 3278F5E12B04C1AC0004A6EE /* IndexedPNG.png */; };
+		3278F5E42B04C1AC0004A6EE /* IndexedPNG.png in Resources */ = {isa = PBXBuildFile; fileRef = 3278F5E12B04C1AC0004A6EE /* IndexedPNG.png */; };
 		327A418C211D660600495442 /* TestImage.heic in Resources */ = {isa = PBXBuildFile; fileRef = 327A418B211D660600495442 /* TestImage.heic */; };
 		327A418D211D660600495442 /* TestImage.heic in Resources */ = {isa = PBXBuildFile; fileRef = 327A418B211D660600495442 /* TestImage.heic */; };
 		328BB6DD20825E9800760D6C /* SDWebImageTestCache.m in Sources */ = {isa = PBXBuildFile; fileRef = 328BB6DC20825E9800760D6C /* SDWebImageTestCache.m */; };
@@ -149,6 +152,7 @@
 		3264FF2E205D42CB00F6BD48 /* SDWebImageTestTransformer.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SDWebImageTestTransformer.m; sourceTree = "<group>"; };
 		326E69462334C0C200B7252C /* TestLoopCount.gif */ = {isa = PBXFileReference; lastKnownFileType = image.gif; path = TestLoopCount.gif; sourceTree = "<group>"; };
 		327054E1206CEFF3006EA328 /* TestImageAnimated.apng */ = {isa = PBXFileReference; lastKnownFileType = file; path = TestImageAnimated.apng; sourceTree = "<group>"; };
+		3278F5E12B04C1AC0004A6EE /* IndexedPNG.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = IndexedPNG.png; sourceTree = "<group>"; };
 		327A418B211D660600495442 /* TestImage.heic */ = {isa = PBXFileReference; lastKnownFileType = file; path = TestImage.heic; sourceTree = "<group>"; };
 		328BAF262240C08E00FC70DD /* Test-Shared.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = "Test-Shared.xcconfig"; sourceTree = "<group>"; };
 		328BAF272240C08E00FC70DD /* Test-Release.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = "Test-Release.xcconfig"; sourceTree = "<group>"; };
@@ -266,6 +270,7 @@
 			isa = PBXGroup;
 			children = (
 				32648066250232F7004FA0FC /* 1@2x.gif */,
+				3278F5E12B04C1AC0004A6EE /* IndexedPNG.png */,
 				433BBBBA1D7EFA8B0086B6E9 /* MonochromeTestImage.jpg */,
 				324047432271956F007C53E1 /* TestEXIF.png */,
 				3264CD162AAB1E23001E338B /* TestJFIF.jpg */,
@@ -497,6 +502,7 @@
 				329922892365DC6C00EAFD97 /* TestImageLarge.jpg in Resources */,
 				32648069250232F7004FA0FC /* 1@2x.gif in Resources */,
 				3299228A2365DC6C00EAFD97 /* TestImage.png in Resources */,
+				3278F5E42B04C1AC0004A6EE /* IndexedPNG.png in Resources */,
 				329922842365DC6C00EAFD97 /* MonochromeTestImage.jpg in Resources */,
 				329922882365DC6C00EAFD97 /* TestImage.jpg in Resources */,
 				32515F9E24AF1919005E8F79 /* TestImageAnimated.webp in Resources */,
@@ -524,6 +530,7 @@
 				32B99EA6203B31360017FD66 /* TestImage.png in Resources */,
 				32648068250232F7004FA0FC /* 1@2x.gif in Resources */,
 				3297A0A023374D1700814590 /* TestImageAnimated.heic in Resources */,
+				3278F5E32B04C1AC0004A6EE /* IndexedPNG.png in Resources */,
 				32B99EA2203B31360017FD66 /* MonochromeTestImage.jpg in Resources */,
 				32905E65211D786E00460FCF /* TestImage.heif in Resources */,
 				32515F9D24AF1919005E8F79 /* TestImageAnimated.webp in Resources */,
@@ -551,6 +558,7 @@
 				433BBBB71D7EF8200086B6E9 /* TestImage.gif in Resources */,
 				32648067250232F7004FA0FC /* 1@2x.gif in Resources */,
 				433BBBB91D7EF8260086B6E9 /* TestImage.png in Resources */,
+				3278F5E22B04C1AC0004A6EE /* IndexedPNG.png in Resources */,
 				3297A09F23374D1700814590 /* TestImageAnimated.heic in Resources */,
 				327054E2206CEFF3006EA328 /* TestImageAnimated.apng in Resources */,
 				32515F9C24AF1919005E8F79 /* TestImageAnimated.webp in Resources */,

Tests/Tests/SDImageCoderTests.m

@@ -558,6 +558,21 @@
     expect(exifOrientation).equal(kCGImagePropertyOrientationDown);
 }
 
+- (void)test30ThatImageIOPNGPluginBuggyWorkaround {
+    // See: #3634
+    NSURL *url = [[NSBundle bundleForClass:[self class]] URLForResource:@"IndexedPNG" withExtension:@"png"];
+    NSData *data = [NSData dataWithContentsOfURL:url];
+    
+    UIImage *decodedImage = [SDImageIOCoder.sharedCoder decodedImageWithData:data options:nil];
+    UIColor *testColor1 = [decodedImage sd_colorAtPoint:CGPointMake(100, 1)];
+    CGFloat r1, g1, b1, a1;
+    [testColor1 getRed:&r1 green:&g1 blue:&b1 alpha:&a1];
+    expect(r1).beCloseToWithin(0.60, 0.01);
+    expect(g1).beCloseToWithin(0.91, 0.01);
+    expect(b1).beCloseToWithin(0.91, 0.01);
+    expect(a1).beCloseToWithin(0.20, 0.01);
+}
+
 #pragma mark - Utils
 
 - (void)verifyCoder:(id<SDImageCoder>)coder

Tests/Tests/SDImageTransformerTests.m

@@ -247,16 +247,15 @@ static void SDAssertCGImageFirstComponentWhite(CGImageRef image, OSType pixelTyp
     // Check left color, should be blurred
     UIColor *leftColor = [blurredImage sd_colorAtPoint:CGPointMake(80, 150)];
     // Hard-code from the output, allows a little deviation because of blur diffs between OS versions :)
-    // rgba(114, 27, 23, 0.75)
+    UIColor *expectedColor = [UIColor colorWithRed:0.59 green:0.14 blue:0.12 alpha:0.75];
-    UIColor *expectedColor = [UIColor colorWithRed:114.0/255.0 green:27.0/255.0 blue:23.0/255.0 alpha:0.75];
     CGFloat r1, g1, b1, a1;
     CGFloat r2, g2, b2, a2;
     [leftColor getRed:&r1 green:&g1 blue:&b1 alpha:&a1];
     [expectedColor getRed:&r2 green:&g2 blue:&b2 alpha:&a2];
-    expect(r1).beCloseToWithin(r2, 2.0/255.0);
+    expect(r1).beCloseToWithin(r2, 0.01);
-    expect(g1).beCloseToWithin(g2, 2.0/255.0);
+    expect(g1).beCloseToWithin(g2, 0.01);
-    expect(b1).beCloseToWithin(b2, 2.0/255.0);
+    expect(b1).beCloseToWithin(b2, 0.01);
-    expect(a1).beCloseToWithin(a2, 2.0/255.0);
+    expect(a1).beCloseToWithin(a2, 0.01);
     // Check rounded corner operation not inversion the image
     UIColor *topCenterColor = [blurredImage sd_colorAtPoint:CGPointMake(150, 20)];
     UIColor *bottomCenterColor = [blurredImage sd_colorAtPoint:CGPointMake(150, 280)];