Merge pull request SDWebImage#3798 from dreampiggy/bugfix/sd_colorAtPoint_early_return

fix: sd_colorAtPoint should early return when pixel format is not supported

Author: dreampiggy

SDWebImage/Core/SDImageCoder.h

@@ -89,9 +89,9 @@ FOUNDATION_EXPORT SDImageCoderOption _Nonnull const SDImageCoderDecodeUseLazyDec
 FOUNDATION_EXPORT SDImageCoderOption _Nonnull const SDImageCoderDecodeScaleDownLimitBytes;
 
 /**
- A Boolean value to provide converting to HDR during decoding.
+ A Boolean value (NSNumber) to provide converting to HDR during decoding. Currently if number is 0, use SDR, else use HDR. But we may extend this option to use `NSUInteger` in the future (means, think this options as int number, but not actual boolean)
  @note Supported by iOS 17 and above when using ImageIO coder (third-party coder can support lower firmware)
- Defaults to NO, decoder will automatically convert SDR.
+ Defaults to @(NO), decoder will automatically convert SDR.
  @note works for `SDImageCoder`
  */
 FOUNDATION_EXPORT SDImageCoderOption _Nonnull const SDImageCoderDecodeToHDR;

SDWebImage/Core/SDWebImageDefine.h

@@ -338,9 +338,9 @@ FOUNDATION_EXPORT SDWebImageContextOption _Nonnull const SDWebImageContextImageT
 FOUNDATION_EXPORT SDWebImageContextOption _Nonnull const SDWebImageContextImageScaleDownLimitBytes;
 
 /**
- A Boolean value to provide converting to HDR during decoding.
+ A Boolean value (NSNumber) to provide converting to HDR during decoding. Currently if number is 0, use SDR, else use HDR. But we may extend this option to use `NSUInteger` in the future (means, think this options as int number, but not actual boolean)
  @note Supported by iOS 17 and above when using ImageIO coder (third-party coder can support lower firmware)
- Defaults to NO, decoder will automatically convert SDR.
+ Defaults to @(NO), decoder will automatically convert SDR.
  */
 FOUNDATION_EXPORT SDWebImageContextOption _Nonnull const SDWebImageContextImageDecodeToHDR;
 

SDWebImage/Core/UIImage+Transform.h

@@ -123,6 +123,7 @@ typedef NS_OPTIONS(NSUInteger, SDRectCorner) {
  @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.
 
  @param point The position of pixel
+ @warning This API currently support 8 bits per component only (RGBA8888 etc), not RGBA16U/RGBA10
  @return The color for specify pixel, or nil if any error occur
  */
 - (nullable UIColor *)sd_colorAtPoint:(CGPoint)point;

SDWebImage/Core/UIImage+Transform.m

@@ -149,7 +149,7 @@ static inline CGRect SDCGRectFitWithScaleMode(CGRect rect, CGSize size, SDImageS
 #endif
 }
 
-static inline UIColor * SDGetColorFromRGBA(Pixel_8888 pixel, CGBitmapInfo bitmapInfo, CGColorSpaceRef cgColorSpace) {
+static inline UIColor * SDGetColorFromRGBA8(Pixel_8888 pixel, CGBitmapInfo bitmapInfo, CGColorSpaceRef cgColorSpace) {
     // Get alpha info, byteOrder info
     CGImageAlphaInfo alphaInfo = bitmapInfo & kCGBitmapAlphaInfoMask;
     CGBitmapInfo byteOrderInfo = bitmapInfo & kCGBitmapByteOrderMask;
@@ -752,6 +752,19 @@ - (nullable UIColor *)sd_colorAtPoint:(CGPoint)point {
         return nil;
     }
 
+    // Check pixel format
+    CGBitmapInfo bitmapInfo = CGImageGetBitmapInfo(imageRef);
+    size_t bitsPerComponent = CGImageGetBitsPerComponent(imageRef);
+    if (@available(iOS 12.0, tvOS 12.0, macOS 10.14, watchOS 5.0, *)) {
+        CGImagePixelFormatInfo pixelFormat = (bitmapInfo & kCGImagePixelFormatMask);
+        if (pixelFormat != kCGImagePixelFormatPacked || bitsPerComponent > 8) {
+            // like RGBA1010102, need bitwise to extract pixel from single uint32_t, we don't support currently
+            SD_LOG("Unsupported pixel format: %u, bpc: %zu for CGImage: %@", pixelFormat, bitsPerComponent, imageRef);
+            CGImageRelease(imageRef);
+            return nil;
+        }
+    }
+    
     // Get pixels
     CGDataProviderRef provider = CGImageGetDataProvider(imageRef);
     if (!provider) {
@@ -766,8 +779,7 @@ - (nullable UIColor *)sd_colorAtPoint:(CGPoint)point {
 
     // Get pixel at point
     size_t bytesPerRow = CGImageGetBytesPerRow(imageRef);
-    size_t components = CGImageGetBitsPerPixel(imageRef) / CGImageGetBitsPerComponent(imageRef);
-    CGBitmapInfo bitmapInfo = CGImageGetBitmapInfo(imageRef);
+    size_t components = CGImageGetBitsPerPixel(imageRef) / bitsPerComponent;
 
     CFRange range = CFRangeMake(bytesPerRow * y + components * x, components);
     if (CFDataGetLength(data) < range.location + range.length) {
@@ -793,9 +805,9 @@ - (nullable UIColor *)sd_colorAtPoint:(CGPoint)point {
         CFRelease(data);
         CGImageRelease(imageRef);
         // Convert to color
-        return SDGetColorFromRGBA(pixel, bitmapInfo, colorSpace);
+        return SDGetColorFromRGBA8(pixel, bitmapInfo, colorSpace);
     } else {
-        SD_LOG("Unsupported components: %zu", components);
+        SD_LOG("Unsupported components: %zu for CGImage: %@", components, imageRef);
         CFRelease(data);
         CGImageRelease(imageRef);
         return nil;
@@ -826,6 +838,19 @@ - (nullable UIColor *)sd_colorAtPoint:(CGPoint)point {
         return nil;
     }
 
+    // Check pixel format
+    CGBitmapInfo bitmapInfo = CGImageGetBitmapInfo(imageRef);
+    size_t bitsPerComponent = CGImageGetBitsPerComponent(imageRef);
+    if (@available(iOS 12.0, tvOS 12.0, macOS 10.14, watchOS 5.0, *)) {
+        CGImagePixelFormatInfo pixelFormat = (bitmapInfo & kCGImagePixelFormatMask);
+        if (pixelFormat != kCGImagePixelFormatPacked || bitsPerComponent > 8) {
+            // like RGBA1010102, need bitwise to extract pixel from single uint32_t, we don't support currently
+            SD_LOG("Unsupported pixel format: %u, bpc: %zu for CGImage: %@", pixelFormat, bitsPerComponent, imageRef);
+            CGImageRelease(imageRef);
+            return nil;
+        }
+    }
+    
     // Get pixels
     CGDataProviderRef provider = CGImageGetDataProvider(imageRef);
     if (!provider) {
@@ -840,7 +865,7 @@ - (nullable UIColor *)sd_colorAtPoint:(CGPoint)point {
 
     // Get pixels with rect
     size_t bytesPerRow = CGImageGetBytesPerRow(imageRef);
-    size_t components = CGImageGetBitsPerPixel(imageRef) / CGImageGetBitsPerComponent(imageRef);
+    size_t components = CGImageGetBitsPerPixel(imageRef) / bitsPerComponent;
 
     size_t start = bytesPerRow * CGRectGetMinY(rect) + components * CGRectGetMinX(rect);
     size_t end = bytesPerRow * (CGRectGetMaxY(rect) - 1) + components * CGRectGetMaxX(rect);
@@ -855,7 +880,6 @@ - (nullable UIColor *)sd_colorAtPoint:(CGPoint)point {
     size_t col = CGRectGetMaxX(rect);
 
     // Convert to color
-    CGBitmapInfo bitmapInfo = CGImageGetBitmapInfo(imageRef);
     NSMutableArray<UIColor *> *colors = [NSMutableArray arrayWithCapacity:CGRectGetWidth(rect) * CGRectGetHeight(rect)];
     // ColorSpace
     CGColorSpaceRef colorSpace = CGImageGetColorSpace(imageRef);
@@ -874,12 +898,13 @@ - (nullable UIColor *)sd_colorAtPoint:(CGPoint)point {
         } else {
             if (components == 3) {
                 Pixel_8888 pixel = {pixels[index], pixels[index+1], pixels[index+2], 0};
-                color = SDGetColorFromRGBA(pixel, bitmapInfo, colorSpace);
+                color = SDGetColorFromRGBA8(pixel, bitmapInfo, colorSpace);
             } else if (components == 4) {
                 Pixel_8888 pixel = {pixels[index], pixels[index+1], pixels[index+2], pixels[index+3]};
-                color = SDGetColorFromRGBA(pixel, bitmapInfo, colorSpace);
+                color = SDGetColorFromRGBA8(pixel, bitmapInfo, colorSpace);
             } else {
-                SD_LOG("Unsupported components: %zu", components);
+                SD_LOG("Unsupported components: %zu for CGImage: %@", components, imageRef);
+                break;
             }
         }
         if (color) {

Tests/SDWebImage Tests.xcodeproj/project.pbxproj

@@ -13,6 +13,10 @@
 		320224F82440C39B00E5B29D /* TestImageLarge.png in Resources */ = {isa = PBXBuildFile; fileRef = 320224F62440C39B00E5B29D /* TestImageLarge.png */; };
 		320224F92440C39B00E5B29D /* TestImageLarge.png in Resources */ = {isa = PBXBuildFile; fileRef = 320224F62440C39B00E5B29D /* TestImageLarge.png */; };
 		320630412085A37C006E0FA4 /* SDAnimatedImageTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 32A571552037DB2D002EDAAE /* SDAnimatedImageTest.m */; };
+		321DDA342D688A8400F7971A /* TestHDR.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 321DDA332D688A8400F7971A /* TestHDR.jpeg */; };
+		321DDA352D688A8400F7971A /* TestHDR.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 321DDA332D688A8400F7971A /* TestHDR.jpeg */; };
+		321DDA362D688A8400F7971A /* TestHDR.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 321DDA332D688A8400F7971A /* TestHDR.jpeg */; };
+		321DDA372D688A8400F7971A /* TestHDR.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 321DDA332D688A8400F7971A /* TestHDR.jpeg */; };
 		321F310E27D0DC490042B274 /* TestImage.bmp in Resources */ = {isa = PBXBuildFile; fileRef = 321F310D27D0DC490042B274 /* TestImage.bmp */; };
 		321F310F27D0DC490042B274 /* TestImage.bmp in Resources */ = {isa = PBXBuildFile; fileRef = 321F310D27D0DC490042B274 /* TestImage.bmp */; };
 		321F311027D0DC490042B274 /* TestImage.bmp in Resources */ = {isa = PBXBuildFile; fileRef = 321F310D27D0DC490042B274 /* TestImage.bmp */; };
@@ -198,6 +202,7 @@
 		2D7AF05E1F329763000083C2 /* SDTestCase.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SDTestCase.h; sourceTree = "<group>"; };
 		2D7AF05F1F329763000083C2 /* SDTestCase.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = SDTestCase.m; sourceTree = "<group>"; };
 		320224F62440C39B00E5B29D /* TestImageLarge.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = TestImageLarge.png; sourceTree = "<group>"; };
+		321DDA332D688A8400F7971A /* TestHDR.jpeg */ = {isa = PBXFileReference; lastKnownFileType = image.jpeg; path = TestHDR.jpeg; sourceTree = "<group>"; };
 		321F310D27D0DC490042B274 /* TestImage.bmp */ = {isa = PBXFileReference; lastKnownFileType = image.bmp; path = TestImage.bmp; sourceTree = "<group>"; };
 		3222417E2272F808002429DB /* SDUtilsTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SDUtilsTests.m; sourceTree = "<group>"; };
 		3226ECB920754F7700FAFACF /* SDWebImageTestDownloadOperation.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SDWebImageTestDownloadOperation.h; sourceTree = "<group>"; };
@@ -368,6 +373,7 @@
 				3264CD162AAB1E23001E338B /* TestJFIF.jpg */,
 				3261EC882D66235D00F2702E /* TestHDR.avif */,
 				3261EC892D66235D00F2702E /* TestHDR.heic */,
+				321DDA332D688A8400F7971A /* TestHDR.jpeg */,
 				3261EC8A2D66235D00F2702E /* TestHDR.jxl */,
 				321F310D27D0DC490042B274 /* TestImage.bmp */,
 				433BBBB61D7EF8200086B6E9 /* TestImage.gif */,
@@ -630,6 +636,7 @@
 				32464A9E2B7B1833006BE70E /* TestImageAnimated.webp in Resources */,
 				32464A972B7B1833006BE70E /* TestImage.bmp in Resources */,
 				32464AA32B7B1833006BE70E /* TestImage.heif in Resources */,
+				321DDA352D688A8400F7971A /* TestHDR.jpeg in Resources */,
 				32B4A4832C082A160004E42C /* TestImage.svg in Resources */,
 				32464AA12B7B1833006BE70E /* MonochromeTestImage.jpg in Resources */,
 				32464AA42B7B1833006BE70E /* TestImageLarge.png in Resources */,
@@ -664,6 +671,7 @@
 				3278F5E42B04C1AC0004A6EE /* IndexedPNG.png in Resources */,
 				329922842365DC6C00EAFD97 /* MonochromeTestImage.jpg in Resources */,
 				329922882365DC6C00EAFD97 /* TestImage.jpg in Resources */,
+				321DDA342D688A8400F7971A /* TestHDR.jpeg in Resources */,
 				32B4A4822C082A160004E42C /* TestImage.svg in Resources */,
 				32515F9E24AF1919005E8F79 /* TestImageAnimated.webp in Resources */,
 				3299228E2365DC6C00EAFD97 /* TestImageAnimated.heics in Resources */,
@@ -698,6 +706,7 @@
 				3278F5E32B04C1AC0004A6EE /* IndexedPNG.png in Resources */,
 				32B99EA2203B31360017FD66 /* MonochromeTestImage.jpg in Resources */,
 				32905E65211D786E00460FCF /* TestImage.heif in Resources */,
+				321DDA372D688A8400F7971A /* TestHDR.jpeg in Resources */,
 				32B4A4812C082A160004E42C /* TestImage.svg in Resources */,
 				32515F9D24AF1919005E8F79 /* TestImageAnimated.webp in Resources */,
 				327A418D211D660600495442 /* TestImage.heic in Resources */,
@@ -732,6 +741,7 @@
 				3278F5E22B04C1AC0004A6EE /* IndexedPNG.png in Resources */,
 				3297A09F23374D1700814590 /* TestImageAnimated.heics in Resources */,
 				327054E2206CEFF3006EA328 /* TestImageAnimated.apng in Resources */,
+				321DDA362D688A8400F7971A /* TestHDR.jpeg in Resources */,
 				32B4A4802C082A160004E42C /* TestImage.svg in Resources */,
 				32515F9C24AF1919005E8F79 /* TestImageAnimated.webp in Resources */,
 				326E69472334C0C300B7252C /* TestLoopCount.gif in Resources */,

Tests/Tests/Images/TestHDR.jpeg

@@ -610,12 +610,13 @@ - (void)test30ThatImageIOPNGPluginBuggyWorkaround {
     url = [[NSBundle bundleForClass:[self class]] URLForResource:@"RGBA16PNG" withExtension:@"png"];
     data = [NSData dataWithContentsOfURL:url];
     decodedImage = [SDImageIOCoder.sharedCoder decodedImageWithData:data options:nil];
-    testColor1 = [decodedImage sd_colorAtPoint:CGPointMake(100, 1)];
-    [testColor1 getRed:&r1 green:&g1 blue:&b1 alpha:&a1];
-    expect(r1).beCloseToWithin(0.60, 0.01);
-    expect(g1).beCloseToWithin(0.60, 0.01);
-    expect(b1).beCloseToWithin(0.33, 0.01);
-    expect(a1).beCloseToWithin(0.33, 0.01);
+    CGBitmapInfo bitmapInfo = CGImageGetBitmapInfo(decodedImage.CGImage);
+    size_t bpc = CGImageGetBitsPerComponent(decodedImage.CGImage);
+    expect(bpc).equal(16);
+    CGImageAlphaInfo alphaInfo = bitmapInfo & kCGBitmapAlphaInfoMask;
+    CGImageByteOrderInfo byteOrderInfo = bitmapInfo & kCGBitmapByteOrderMask;
+    expect(alphaInfo).equal(kCGImageAlphaLast);
+    expect(byteOrderInfo).equal(kCGImageByteOrder16Little);
 }
 
 - (void)test31ThatSVGShouldUseNativeImageClass {
@@ -635,7 +636,7 @@ - (void)test31ThatSVGShouldUseNativeImageClass {
     }
 }
 
-- (void)test32ThatHDRDecodeWorks {
+- (void)test32ThatISOHDRDecodeWorks {
     // Only test for iOS 17+/macOS 14+/visionOS 1+, or ImageIO decoder does not support HDR
 #if SD_MAC || SD_IOS || SD_VISION
     if (@available(macOS 14, iOS 17, tvOS 17, watchOS 10, *)) {
@@ -652,18 +653,59 @@ - (void)test32ThatHDRDecodeWorks {
 
             expect([SDImageCoderHelper CGImageIsHDR:HDRImage.CGImage]).beTruthy();
             expect(HDRImage.sd_isHighDynamicRange).beTruthy();
+            // Current we lack of some HDR RGBA1010102
+            size_t HDRBPC = CGImageGetBitsPerComponent(HDRImage.CGImage);
+            expect(HDRBPC).beGreaterThan(8);
+            expect([HDRImage sd_colorAtPoint:CGPointMake(1, 1)]).beNil();
+            
             // FIXME: on Simulator, the SDR decode options will not take effect, so SDR is the same as HDR
 #if !TARGET_OS_SIMULATOR
             expect([SDImageCoderHelper CGImageIsHDR:SDRImage.CGImage]).beFalsy();
             expect(SDRImage.sd_isHighDynamicRange).beFalsy();
+            size_t SDRBPC = CGImageGetBitsPerComponent(SDRImage.CGImage);
+            expect(SDRBPC).beLessThanOrEqualTo(8);
+            expect([SDRImage sd_colorAtPoint:CGPointMake(1, 1)]).notTo.beNil();
 #endif
+            
             // FIXME: Encoding need iOS 18+/macOS 15+
             // And need test both GainMap HDR or ISO HDR, TODO
         }
     }
 #endif
 }
 
+- (void)test33ThatGainMapHDRDecodeWorks {
+    // JPEG Gain Map HDR need iOS 18+
+    // GitHub Action virtualization framework contains issue for Gain Map HDR convert:
+    // [xctest] +[HDRImageConverter imageConverterWithOptions:]:40: ☀️ Failed to initialize Metal converter, falling back to SIMD for image conversion (slow)
+    if (SDTestCase.isCI) {
+        return;
+    }
+#if SD_MAC || SD_IOS || SD_VISION
+    if (@available(macOS 15, iOS 18, tvOS 18, watchOS 11, *)) {
+        NSArray *formats = @[@"jpeg"];
+        for (NSString *format in formats) {
+            NSURL *url = [[NSBundle bundleForClass:[self class]] URLForResource:@"TestHDR" withExtension:format];
+            NSData *data = [NSData dataWithContentsOfURL:url];
+            // Decoding
+            UIImage *HDRImage = [SDImageIOCoder.sharedCoder decodedImageWithData:data options:@{SDImageCoderDecodeToHDR : @(YES)}];
+            UIImage *SDRImage = [SDImageIOCoder.sharedCoder decodedImageWithData:data options:@{SDImageCoderDecodeToHDR : @(NO)}];
+            
+            expect(HDRImage).notTo.beNil();
+            expect(SDRImage).notTo.beNil();
+            
+            // Gain Map HDR does not use Rec. 2100 color space
+            size_t HDRBPC = CGImageGetBitsPerComponent(HDRImage.CGImage);
+            size_t SDRBPC = CGImageGetBitsPerComponent(SDRImage.CGImage);
+            expect(HDRBPC).beGreaterThan(8);
+            expect(SDRBPC).beLessThanOrEqualTo(8);
+//            expect([SDImageCoderHelper CGImageIsHDR:HDRImage.CGImage]).beTruthy();
+//            expect(HDRImage.sd_isHighDynamicRange).beTruthy();
+        }
+    }
+#endif
+}
+
 #pragma mark - Utils
 
 - (void)verifyCoder:(id<SDImageCoder>)coder