Skip to content

Commit

Permalink
[ASImageNode]fix incorrect backing size calculation (facebookarchive#…
Browse files Browse the repository at this point in the history
…1189)

* fix backing size for image node which content mode is scaleAspectFit

* chore: update comments and naming

* add change log

* Update CHANGELOG.md

* Update CHANGELOG.md

Co-Authored-By: junjielu <348649634@qq.com>

* add unit test for backing size calculation

* correct license
  • Loading branch information
junjielu authored and garrettmoon committed Mar 29, 2019
1 parent fe1cb1c commit 8231599
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 23 deletions.
5 changes: 5 additions & 0 deletions AsyncDisplayKit.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -685,6 +686,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>"; };
Expand Down Expand Up @@ -1341,6 +1343,7 @@
058D0A30195D057000B7D73C /* ASDisplayNodeTestsHelper.h */,
058D0A31195D057000B7D73C /* ASDisplayNodeTestsHelper.mm */,
697B31591CFE4B410049936F /* ASEditableTextNodeTests.mm */,
471D04B0224CB98600649215 /* ASImageNodeBackingSizeTests.mm */,
056D21541ABCEF50001107EF /* ASImageNodeSnapshotTests.mm */,
ACF6ED551B178DC700DA7C62 /* ASInsetLayoutSpecSnapshotTests.mm */,
CCE4F9B21F0D60AC00062E4E /* ASIntegerMapTests.mm */,
Expand Down Expand Up @@ -2215,6 +2218,7 @@
developmentRegion = English;
hasScannedForEncodings = 0;
knownRegions = (
English,
en,
Base,
);
Expand Down Expand Up @@ -2323,6 +2327,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 */,
Expand Down
47 changes: 24 additions & 23 deletions Source/Private/ASImageNode+CGExtras.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
}

Expand Down
80 changes: 80 additions & 0 deletions Tests/ASImageNodeBackingSizeTests.mm
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 8231599

Please sign in to comment.