[ASImageNode]fix incorrect backing size calculation

AsyncDisplayKit.xcodeproj/project.pbxproj

@@ -101,6 +101,7 @@
 		3917EBD41E9C2FC400D04A01 /* _ASCollectionReusableView.h in Headers */ = {isa = PBXBuildFile; fileRef = 3917EBD21E9C2FC400D04A01 /* _ASCollectionReusableView.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		3917EBD51E9C2FC400D04A01 /* _ASCollectionReusableView.mm in Sources */ = {isa = PBXBuildFile; fileRef = 3917EBD31E9C2FC400D04A01 /* _ASCollectionReusableView.mm */; };
 		3C9C128519E616EF00E942A0 /* ASTableViewTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 3C9C128419E616EF00E942A0 /* ASTableViewTests.mm */; };
+		471D04B1224CB98600649215 /* ASImageNodeBackingSizeTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 471D04B0224CB98600649215 /* ASImageNodeBackingSizeTests.mm */; };
 		4E9127691F64157600499623 /* ASRunLoopQueueTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 4E9127681F64157600499623 /* ASRunLoopQueueTests.mm */; };
 		509E68601B3AED8E009B9150 /* ASScrollDirection.mm in Sources */ = {isa = PBXBuildFile; fileRef = 205F0E111B371BD7007741D0 /* ASScrollDirection.mm */; };
 		509E68611B3AEDA0009B9150 /* ASAbstractLayoutController.h in Headers */ = {isa = PBXBuildFile; fileRef = 205F0E171B37339C007741D0 /* ASAbstractLayoutController.h */; };
@@ -684,6 +685,7 @@
 		4640521B1A3F83C40061C0BA /* ASTableLayoutController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASTableLayoutController.h; sourceTree = "<group>"; };
 		4640521C1A3F83C40061C0BA /* ASTableLayoutController.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASTableLayoutController.mm; sourceTree = "<group>"; };
 		4640521D1A3F83C40061C0BA /* ASLayoutController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASLayoutController.h; sourceTree = "<group>"; };
+		471D04B0224CB98600649215 /* ASImageNodeBackingSizeTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = ASImageNodeBackingSizeTests.mm; sourceTree = "<group>"; };
 		4E9127681F64157600499623 /* ASRunLoopQueueTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASRunLoopQueueTests.mm; sourceTree = "<group>"; };
 		68355B2E1CB5799E001D4E68 /* ASImageNode+AnimatedImage.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = "ASImageNode+AnimatedImage.mm"; sourceTree = "<group>"; };
 		68355B361CB57A5A001D4E68 /* ASPINRemoteImageDownloader.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASPINRemoteImageDownloader.mm; sourceTree = "<group>"; };
@@ -1339,6 +1341,7 @@
 				058D0A30195D057000B7D73C /* ASDisplayNodeTestsHelper.h */,
 				058D0A31195D057000B7D73C /* ASDisplayNodeTestsHelper.mm */,
 				697B31591CFE4B410049936F /* ASEditableTextNodeTests.mm */,
+				471D04B0224CB98600649215 /* ASImageNodeBackingSizeTests.mm */,
 				056D21541ABCEF50001107EF /* ASImageNodeSnapshotTests.mm */,
 				ACF6ED551B178DC700DA7C62 /* ASInsetLayoutSpecSnapshotTests.mm */,
 				CCE4F9B21F0D60AC00062E4E /* ASIntegerMapTests.mm */,
@@ -2212,6 +2215,7 @@
 			developmentRegion = English;
 			hasScannedForEncodings = 0;
 			knownRegions = (
+				English,
 				en,
 				Base,
 			);
@@ -2319,6 +2323,7 @@
 				058D0A3A195D057000B7D73C /* ASDisplayNodeTests.mm in Sources */,
 				9644CFE02193777C00213478 /* ASThrashUtility.m in Sources */,
 				696FCB311D6E46050093471E /* ASBackgroundLayoutSpecSnapshotTests.mm in Sources */,
+				471D04B1224CB98600649215 /* ASImageNodeBackingSizeTests.mm in Sources */,
 				CC583AD81EF9BDC300134156 /* OCMockObject+ASAdditions.mm in Sources */,
 				69FEE53D1D95A9AF0086F066 /* ASLayoutElementStyleTests.mm in Sources */,
 				4E9127691F64157600499623 /* ASRunLoopQueueTests.mm in Sources */,

Source/Private/ASImageNode+CGExtras.mm

@@ -53,26 +53,27 @@ void ASCroppedImageBackingSizeAndDrawRectInBounds(CGSize sourceImageSize,
   // Per the API contract as commented in the header, we will adjust input parameters (destinationWidth, destinationHeight) to ensure that the image is not upscaled on the CPU.
   CGFloat boundsAspectRatio = (CGFloat)destinationWidth / (CGFloat)destinationHeight;
 
-  CGSize scaledSizeForImage = sourceImageSize;
+  CGSize minimumDestinationSize = sourceImageSize;
   BOOL cropToRectDimensions = !CGRectIsEmpty(cropRect);
 
+  // Given image size and container ratio, calculate minimum container size for the image under different contentMode
   if (cropToRectDimensions) {
-    scaledSizeForImage = CGSizeMake(boundsSize.width / cropRect.size.width, boundsSize.height / cropRect.size.height);
+    minimumDestinationSize = CGSizeMake(boundsSize.width / cropRect.size.width, boundsSize.height / cropRect.size.height);
   } else {
     if (contentMode == UIViewContentModeScaleAspectFill)
-      scaledSizeForImage = _ASSizeFillWithAspectRatio(boundsAspectRatio, sourceImageSize);
+      minimumDestinationSize = _ASSizeFitWithAspectRatio(boundsAspectRatio, sourceImageSize);
     else if (contentMode == UIViewContentModeScaleAspectFit)
-      scaledSizeForImage = _ASSizeFitWithAspectRatio(boundsAspectRatio, sourceImageSize);
+      minimumDestinationSize = _ASSizeFillWithAspectRatio(boundsAspectRatio, sourceImageSize);
   }
 
   // If fitting the desired aspect ratio to the image size actually results in a larger buffer, use the input values.
   // However, if there is a pixel savings (e.g. we would have to upscale the image), override the function arguments.
   if (CGSizeEqualToSize(CGSizeZero, forcedSize) == NO) {
     destinationWidth = (size_t)round(forcedSize.width);
     destinationHeight = (size_t)round(forcedSize.height);
-  } else if (forceUpscaling == NO && (scaledSizeForImage.width * scaledSizeForImage.height) < (destinationWidth * destinationHeight)) {
-    destinationWidth = (size_t)round(scaledSizeForImage.width);
-    destinationHeight = (size_t)round(scaledSizeForImage.height);
+  } else if (forceUpscaling == NO && (minimumDestinationSize.width * minimumDestinationSize.height) < (destinationWidth * destinationHeight)) {
+    destinationWidth = (size_t)round(minimumDestinationSize.width);
+    destinationHeight = (size_t)round(minimumDestinationSize.height);
     if (destinationWidth == 0 || destinationHeight == 0) {
       *outBackingSize = CGSizeZero;
       *outDrawRect = CGRectZero;
@@ -82,39 +83,39 @@ void ASCroppedImageBackingSizeAndDrawRectInBounds(CGSize sourceImageSize,
 
   // Figure out the scaled size within the destination bounds.
   CGFloat sourceImageAspectRatio = sourceImageSize.width / sourceImageSize.height;
-  CGSize scaledSizeForDestination = CGSizeMake(destinationWidth, destinationHeight);
+  CGSize scaledSizeForImage = CGSizeMake(destinationWidth, destinationHeight);
 
   if (cropToRectDimensions) {
-    scaledSizeForDestination = CGSizeMake(boundsSize.width / cropRect.size.width, boundsSize.height / cropRect.size.height);
+    scaledSizeForImage = CGSizeMake(boundsSize.width / cropRect.size.width, boundsSize.height / cropRect.size.height);
   } else {
     if (contentMode == UIViewContentModeScaleAspectFill)
-      scaledSizeForDestination = _ASSizeFillWithAspectRatio(sourceImageAspectRatio, scaledSizeForDestination);
+      scaledSizeForImage = _ASSizeFillWithAspectRatio(sourceImageAspectRatio, scaledSizeForImage);
     else if (contentMode == UIViewContentModeScaleAspectFit)
-      scaledSizeForDestination = _ASSizeFitWithAspectRatio(sourceImageAspectRatio, scaledSizeForDestination);
+      scaledSizeForImage = _ASSizeFitWithAspectRatio(sourceImageAspectRatio, scaledSizeForImage);
   }
 
   // Figure out the rectangle into which to draw the image.
   CGRect drawRect = CGRectZero;
   if (cropToRectDimensions) {
-    drawRect = CGRectMake(-cropRect.origin.x * scaledSizeForDestination.width,
-                          -cropRect.origin.y * scaledSizeForDestination.height,
-                          scaledSizeForDestination.width,
-                          scaledSizeForDestination.height);
+    drawRect = CGRectMake(-cropRect.origin.x * scaledSizeForImage.width,
+                          -cropRect.origin.y * scaledSizeForImage.height,
+                          scaledSizeForImage.width,
+                          scaledSizeForImage.height);
   } else {
     // We want to obey the origin of cropRect in aspect-fill mode.
     if (contentMode == UIViewContentModeScaleAspectFill) {
-      drawRect = CGRectMake(((destinationWidth - scaledSizeForDestination.width) * cropRect.origin.x),
-                            ((destinationHeight - scaledSizeForDestination.height) * cropRect.origin.y),
-                            scaledSizeForDestination.width,
-                            scaledSizeForDestination.height);
+      drawRect = CGRectMake(((destinationWidth - scaledSizeForImage.width) * cropRect.origin.x),
+                            ((destinationHeight - scaledSizeForImage.height) * cropRect.origin.y),
+                            scaledSizeForImage.width,
+                            scaledSizeForImage.height);
 
     }
     // And otherwise just center it.
     else {
-      drawRect = CGRectMake(((destinationWidth - scaledSizeForDestination.width) / 2.0),
-                            ((destinationHeight - scaledSizeForDestination.height) / 2.0),
-                            scaledSizeForDestination.width,
-                            scaledSizeForDestination.height);
+      drawRect = CGRectMake(((destinationWidth - scaledSizeForImage.width) / 2.0),
+                            ((destinationHeight - scaledSizeForImage.height) / 2.0),
+                            scaledSizeForImage.width,
+                            scaledSizeForImage.height);
     }
   }
 

Tests/ASImageNodeBackingSizeTests.mm

@@ -0,0 +1,80 @@
+//
+//  ASImageNodeBackingSizeTests.mm
+//  Texture
+//
+//  Copyright (c) Pinterest, Inc.  All rights reserved.
+//  Licensed under Apache 2.0: http://www.apache.org/licenses/LICENSE-2.0
+
+#import <XCTest/XCTest.h>
+#import <AsyncDisplayKit/ASImageNode+CGExtras.h>
+
+static CGSize _FitSizeWithAspectRatio(CGFloat imageRatio, CGSize backingSize);
+static CGSize _FillSizeWithAspectRatio(CGFloat imageRatio, CGSize backingSize);
+
+static CGSize _FitSizeWithAspectRatio(CGFloat imageRatio, CGSize backingSize)
+{
+  CGFloat backingRatio = backingSize.width / backingSize.height;
+  // fit size should be constrained in backing size
+  if (imageRatio > backingRatio) {
+    return CGSizeMake(backingSize.width, backingSize.width / imageRatio);
+  } else {
+    return CGSizeMake(backingSize.height * imageRatio, backingSize.height);
+  }
+}
+
+static CGSize _FillSizeWithAspectRatio(CGFloat imageRatio, CGSize backingSize)
+{
+  CGFloat backingRatio = backingSize.width / backingSize.height;
+  // backing size should be constrained in fill size
+  if (imageRatio > backingRatio) {
+    return CGSizeMake(backingSize.height * imageRatio, backingSize.height);
+  } else {
+    return CGSizeMake(backingSize.width, backingSize.width / imageRatio);
+  }
+}
+
+@interface ASImageNodeBackingSizeTests : XCTestCase
+
+@end
+
+@implementation ASImageNodeBackingSizeTests
+
+- (void)setUp {
+    // Put setup code here. This method is called before the invocation of each test method in the class.
+}
+
+- (void)tearDown {
+    // Put teardown code here. This method is called after the invocation of each test method in the class.
+}
+
+// ScaleAspectFit mode calculation.
+- (void)testScaleAspectFitModeBackingSizeCalculation {
+  CGSize imageSize     = CGSizeMake(100, 100);
+  CGSize boundsSize    = CGSizeMake(200, 400);
+
+  CGSize backingSize   = CGSizeZero;
+  CGRect imageDrawRect = CGRectZero;
+
+  ASCroppedImageBackingSizeAndDrawRectInBounds(imageSize, boundsSize, UIViewContentModeScaleAspectFit, CGRectZero, false, CGSizeZero, &backingSize, &imageDrawRect);
+  CGSize backingSizeShouldBe = _FillSizeWithAspectRatio(boundsSize.width / boundsSize.height, imageSize);
+  CGSize drawRectSizeShouldBe = _FitSizeWithAspectRatio(imageSize.width / imageSize.height, backingSizeShouldBe);
+  XCTAssertTrue(CGSizeEqualToSize(backingSizeShouldBe, backingSize));
+  XCTAssertTrue(CGSizeEqualToSize(drawRectSizeShouldBe, imageDrawRect.size));
+}
+
+// ScaleAspectFill mode calculation.
+- (void)testScaleAspectFillModeBackingSizeCalculation {
+  CGSize imageSize     = CGSizeMake(100, 100);
+  CGSize boundsSize    = CGSizeMake(200, 400);
+
+  CGSize backingSize   = CGSizeZero;
+  CGRect imageDrawRect = CGRectZero;
+
+  ASCroppedImageBackingSizeAndDrawRectInBounds(imageSize, boundsSize, UIViewContentModeScaleAspectFill, CGRectZero, false, CGSizeZero, &backingSize, &imageDrawRect);
+  CGSize backingSizeShouldBe = _FitSizeWithAspectRatio(boundsSize.width / boundsSize.height, imageSize);
+  CGSize drawRectSizeShouldBe = _FillSizeWithAspectRatio(imageSize.width / imageSize.height, backingSizeShouldBe);
+  XCTAssertTrue(CGSizeEqualToSize(backingSizeShouldBe, backingSize));
+  XCTAssertTrue(CGSizeEqualToSize(drawRectSizeShouldBe, imageDrawRect.size));
+}
+
+@end