Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix image scale issue in thumbnail creation #793

Merged
merged 2 commits into from
Jul 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions Nuke.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
0C38DB2928568FE20027F9FF /* fixture-tiny.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 0C91B0F72438E84E007F9100 /* fixture-tiny.jpeg */; };
0C38DB2A28568FE20027F9FF /* baseline.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 0C70D9702089016800A49DAC /* baseline.jpeg */; };
0C38DB2B28568FE20027F9FF /* baseline.webp in Resources */ = {isa = PBXBuildFile; fileRef = 0C95FD532571B278008D4FC2 /* baseline.webp */; };
0C38DB2C28568FE20027F9FF /* left-orientation.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 0CF5456A25B39A0E00B45F1E /* left-orientation.jpeg */; };
0C38DB2C28568FE20027F9FF /* right-orientation.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 0CF5456A25B39A0E00B45F1E /* right-orientation.jpeg */; };
0C38DB2D28568FE20027F9FF /* s-rounded-corners.png in Resources */ = {isa = PBXBuildFile; fileRef = 0C7CE28A243933550018C8C3 /* s-rounded-corners.png */; };
0C38DB2E28568FE20027F9FF /* video.mp4 in Resources */ = {isa = PBXBuildFile; fileRef = 0CA4ECA226E67E3D00BAC8E5 /* video.mp4 */; };
0C38DB2F28568FE20027F9FF /* progressive.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 0C70D96F2089016700A49DAC /* progressive.jpeg */; };
Expand Down Expand Up @@ -217,7 +217,7 @@
0CB6449C28567E5400916267 /* ImageViewLoadingOptionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CB6449B28567E5400916267 /* ImageViewLoadingOptionsTests.swift */; };
0CB644AB28567EEA00916267 /* ImageViewExtensionsProgressiveDecodingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CB644AA28567EEA00916267 /* ImageViewExtensionsProgressiveDecodingTests.swift */; };
0CB644AC28567FC000916267 /* NukeExtensions.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 0C55FCFD28567875000FD2C9 /* NukeExtensions.framework */; };
0CB644BE2856807F00916267 /* left-orientation.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 0CF5456A25B39A0E00B45F1E /* left-orientation.jpeg */; };
0CB644BE2856807F00916267 /* right-orientation.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 0CF5456A25B39A0E00B45F1E /* right-orientation.jpeg */; };
0CB644BF2856807F00916267 /* cat.gif in Resources */ = {isa = PBXBuildFile; fileRef = 0C8DC722209B842600084AA6 /* cat.gif */; };
0CB644C02856807F00916267 /* progressive.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 0C70D96F2089016700A49DAC /* progressive.jpeg */; };
0CB644C12856807F00916267 /* baseline.webp in Resources */ = {isa = PBXBuildFile; fileRef = 0C95FD532571B278008D4FC2 /* baseline.webp */; };
Expand Down Expand Up @@ -260,7 +260,7 @@
0CF235E529A16006001BCA2F /* FetchImage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C55FD1D28567926000FD2C9 /* FetchImage.swift */; };
0CF235E629A16006001BCA2F /* LazyImageState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C38DB6F2856A4D30027F9FF /* LazyImageState.swift */; };
0CF4DE7D1D412A9E00170289 /* ImagePrefetcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CF4DE7C1D412A9E00170289 /* ImagePrefetcher.swift */; };
0CF5456B25B39A0E00B45F1E /* left-orientation.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 0CF5456A25B39A0E00B45F1E /* left-orientation.jpeg */; };
0CF5456B25B39A0E00B45F1E /* right-orientation.jpeg in Resources */ = {isa = PBXBuildFile; fileRef = 0CF5456A25B39A0E00B45F1E /* right-orientation.jpeg */; };
0CF58FF726DAAC3800D2650D /* ImageDownsampleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CF58FF626DAAC3800D2650D /* ImageDownsampleTests.swift */; };
2DFD93B0233A6AB300D84DB9 /* ImagePipelineProcessorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DFD93AF233A6AB300D84DB9 /* ImagePipelineProcessorTests.swift */; };
4480674C2A448C9F00DE7CF8 /* DataPublisherTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4480674B2A448C9F00DE7CF8 /* DataPublisherTests.swift */; };
Expand Down Expand Up @@ -529,7 +529,7 @@
0CF1754B22913F9800A8946E /* ImagePipeline+Configuration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ImagePipeline+Configuration.swift"; sourceTree = "<group>"; };
0CF4DE7C1D412A9E00170289 /* ImagePrefetcher.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ImagePrefetcher.swift; sourceTree = "<group>"; };
0CF52DCB26516F6B0094BC66 /* FetchImageTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FetchImageTests.swift; sourceTree = "<group>"; };
0CF5456A25B39A0E00B45F1E /* left-orientation.jpeg */ = {isa = PBXFileReference; lastKnownFileType = image.jpeg; path = "left-orientation.jpeg"; sourceTree = "<group>"; };
0CF5456A25B39A0E00B45F1E /* right-orientation.jpeg */ = {isa = PBXFileReference; lastKnownFileType = image.jpeg; path = "right-orientation.jpeg"; sourceTree = "<group>"; };
0CF58FF626DAAC3800D2650D /* ImageDownsampleTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageDownsampleTests.swift; sourceTree = "<group>"; };
2DFD93AF233A6AB300D84DB9 /* ImagePipelineProcessorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImagePipelineProcessorTests.swift; sourceTree = "<group>"; };
4480674B2A448C9F00DE7CF8 /* DataPublisherTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataPublisherTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -629,7 +629,7 @@
isa = PBXGroup;
children = (
0C70D9702089016800A49DAC /* baseline.jpeg */,
0CF5456A25B39A0E00B45F1E /* left-orientation.jpeg */,
0CF5456A25B39A0E00B45F1E /* right-orientation.jpeg */,
0C4B34112572E233000FDDBA /* grayscale.jpeg */,
0C70D96F2089016700A49DAC /* progressive.jpeg */,
0C95FD532571B278008D4FC2 /* baseline.webp */,
Expand Down Expand Up @@ -1353,7 +1353,7 @@
0C38DB2928568FE20027F9FF /* fixture-tiny.jpeg in Resources */,
0C38DB2A28568FE20027F9FF /* baseline.jpeg in Resources */,
0C38DB2B28568FE20027F9FF /* baseline.webp in Resources */,
0C38DB2C28568FE20027F9FF /* left-orientation.jpeg in Resources */,
0C38DB2C28568FE20027F9FF /* right-orientation.jpeg in Resources */,
0C38DB2D28568FE20027F9FF /* s-rounded-corners.png in Resources */,
0C38DB2E28568FE20027F9FF /* video.mp4 in Resources */,
0C38DB2F28568FE20027F9FF /* progressive.jpeg in Resources */,
Expand Down Expand Up @@ -1394,7 +1394,7 @@
0CB644C22856807F00916267 /* image-p3.jpg in Resources */,
0CB644C92856807F00916267 /* fixture.jpeg in Resources */,
0CB644C52856807F00916267 /* fixture-tiny.jpeg in Resources */,
0CB644BE2856807F00916267 /* left-orientation.jpeg in Resources */,
0CB644BE2856807F00916267 /* right-orientation.jpeg in Resources */,
0CB644C72856807F00916267 /* baseline.jpeg in Resources */,
0CB644BF2856807F00916267 /* cat.gif in Resources */,
);
Expand All @@ -1418,7 +1418,7 @@
0C91B0F82438E84E007F9100 /* fixture-tiny.jpeg in Resources */,
0C70D9742089016800A49DAC /* baseline.jpeg in Resources */,
0C95FD542571B278008D4FC2 /* baseline.webp in Resources */,
0CF5456B25B39A0E00B45F1E /* left-orientation.jpeg in Resources */,
0CF5456B25B39A0E00B45F1E /* right-orientation.jpeg in Resources */,
0C7CE28B243933550018C8C3 /* s-rounded-corners.png in Resources */,
0CA4ECA426E67ED500BAC8E5 /* video.mp4 in Resources */,
0C70D9712089016800A49DAC /* progressive.jpeg in Resources */,
Expand Down
12 changes: 7 additions & 5 deletions Sources/Nuke/Decoding/ImageDecoders+Default.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ extension ImageDecoders {
private var scanner = ProgressiveJPEGScanner()

private var isPreviewForGIFGenerated = false
private var scale: CGFloat?
private var scale: CGFloat = 1.0
private var thumbnail: ImageRequest.ThumbnailOptions?
private let lock = NSLock()

Expand All @@ -38,7 +38,7 @@ extension ImageDecoders {
/// Returns `nil` if progressive decoding is not allowed for the given
/// content.
public init?(context: ImageDecodingContext) {
self.scale = context.request.scale.map { CGFloat($0) }
self.scale = context.request.scale.map { CGFloat($0) } ?? self.scale
self.thumbnail = context.request.thumbnail

if !context.isCompleted && !isProgressiveDecodingAllowed(for: context.data) {
Expand All @@ -52,7 +52,9 @@ extension ImageDecoders {

func makeImage() -> PlatformImage? {
if let thumbnail {
return makeThumbnail(data: data, options: thumbnail)
return makeThumbnail(data: data,
options: thumbnail,
scale: scale)
}
return ImageDecoders.Default._decode(data, scale: scale)
}
Expand Down Expand Up @@ -162,11 +164,11 @@ private struct ProgressiveJPEGScanner: Sendable {
}

extension ImageDecoders.Default {
private static func _decode(_ data: Data, scale: CGFloat?) -> PlatformImage? {
private static func _decode(_ data: Data, scale: CGFloat) -> PlatformImage? {
#if os(macOS)
return NSImage(data: data)
#else
return UIImage(data: data, scale: scale ?? 1)
return UIImage(data: data, scale: scale)
#endif
}
}
Expand Down
34 changes: 33 additions & 1 deletion Sources/Nuke/Internal/Graphics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,20 @@ extension CGImagePropertyOrientation {
}
}
}
extension UIImage.Orientation {
init(_ cgOrientation: CGImagePropertyOrientation) {
switch cgOrientation {
case .up: self = .up
case .upMirrored: self = .upMirrored
case .down: self = .down
case .downMirrored: self = .downMirrored
case .left: self = .left
case .leftMirrored: self = .leftMirrored
case .right: self = .right
case .rightMirrored: self = .rightMirrored
}
}
}
#endif

#if canImport(UIKit)
Expand Down Expand Up @@ -351,7 +365,10 @@ extension Color {
}

/// Creates an image thumbnail. Uses significantly less memory than other options.
func makeThumbnail(data: Data, options: ImageRequest.ThumbnailOptions) -> PlatformImage? {
/// - parameter data: Data object from which to read the image.
/// - parameter options: Image loading options.
/// - parameter scale: The scale factor to assume when interpreting the image data, defaults to 1.
func makeThumbnail(data: Data, options: ImageRequest.ThumbnailOptions, scale: CGFloat = 1.0) -> PlatformImage? {
guard let source = CGImageSourceCreateWithData(data as CFData, [kCGImageSourceShouldCache: false] as CFDictionary) else {
return nil
}
Expand All @@ -366,7 +383,22 @@ func makeThumbnail(data: Data, options: ImageRequest.ThumbnailOptions) -> Platfo
guard let image = CGImageSourceCreateThumbnailAtIndex(source, 0, options as CFDictionary) else {
return nil
}

#if canImport(UIKit)
let orientation: UIImage.Orientation
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is needed to make sure the returned image has the same orientation as the original. Verified via tests below with .right and .up orientation images

if let imageProperties = CGImageSourceCopyPropertiesAtIndex(source, 0, nil) as? [AnyHashable: Any],
let orientationValue = imageProperties[kCGImagePropertyOrientation as String] as? UInt32,
let cgOrientation = CGImagePropertyOrientation(rawValue: orientationValue) {
orientation = UIImage.Orientation(cgOrientation)
} else {
orientation = .up
}
return PlatformImage(cgImage: image,
scale: scale,
orientation: orientation)
#else
return PlatformImage(cgImage: image)
#endif
}

private func getMaxPixelSize(for source: CGImageSource, options thumbnailOptions: ImageRequest.ThumbnailOptions) -> CGFloat {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ class ImagePipelineCacheTests: XCTestCase {
#if canImport(UIKit)
func testThatImageOrientationIsPreserved() throws {
// GIVEN opaque jpeg with orientation
let image = Test.image(named: "left-orientation", extension: "jpeg")
let image = Test.image(named: "right-orientation", extension: "jpeg")
XCTAssertTrue(image.cgImage!.isOpaque)
XCTAssertEqual(image.imageOrientation, .right)

Expand All @@ -477,7 +477,7 @@ class ImagePipelineCacheTests: XCTestCase {

func testThatImageOrientationIsPreservedForProcessedImages() throws {
// GIVEN opaque jpeg with orientation
let image = Test.image(named: "left-orientation", extension: "jpeg")
let image = Test.image(named: "right-orientation", extension: "jpeg")
XCTAssertTrue(image.cgImage!.isOpaque)
XCTAssertEqual(image.imageOrientation, .right)

Expand Down
24 changes: 20 additions & 4 deletions Tests/NukeTests/ImageProcessorsTests/ImageDownsampleTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,35 @@ class ImageThumbnailTest: XCTestCase {
// Given an image with `right` orientation. From the user perspective,
// the image a landscape image with s size 640x480px. The raw pixel
// data, on the other hand, is 480x640px.
let input = try XCTUnwrap(Test.data(name: "left-orientation", extension: "jpeg"))
let input = try XCTUnwrap(Test.data(name: "right-orientation", extension: "jpeg"))
XCTAssertEqual(PlatformImage(data: input)?.imageOrientation, .right)

// When we resize the image to fit 320x480px frame, we expect the processor
// to take image orientation into the account and produce a 320x240px.
let options = ImageRequest.ThumbnailOptions(size: CGSize(width: 320, height: 1000), unit: .pixels, contentMode: .aspectFit)
let output = try XCTUnwrap(options.makeThumbnail(with: input))

// Then the output thumbnail is rotated
// Then the output has orientation of the original image
XCTAssertEqual(output.imageOrientation, .right)

//verify size of the image in points and pixels (using scale)
XCTAssertEqual(output.sizeInPixels, CGSize(width: 320, height: 240))
XCTAssertEqual(output.size, CGSize(width: 120, height: 160))
}

func testResizeImageWithOrientationUp() throws {
let input = try XCTUnwrap(Test.data(name: "baseline", extension: "jpeg"))
XCTAssertEqual(PlatformImage(data: input)?.imageOrientation, .up)

let options = ImageRequest.ThumbnailOptions(maxPixelSize: 300)
let output = try XCTUnwrap(options.makeThumbnail(with: input))

// Then the output has orientation of the original image
XCTAssertEqual(output.imageOrientation, .up)
// Then the image is resized according to orientation
XCTAssertEqual(output.size, CGSize(width: 320, height: 240))

//verify size of the image in points and pixels (using scale)
XCTAssertEqual(output.sizeInPixels, CGSize(width: 300, height: 200))
XCTAssertEqual(output.size, CGSize(width: 150, height: 100))
}
#endif
}
6 changes: 3 additions & 3 deletions Tests/NukeTests/ImageProcessorsTests/ResizeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class ImageProcessorsResizeTests: XCTestCase {
// the image a landscape image with s size 640x480px. The raw pixel
// data, on the other hand, is 480x640px. macOS, however, automatically
// changes image orientaiton to `up` so that you don't have to worry about it
let input = try XCTUnwrap(Test.image(named: "left-orientation.jpeg"))
let input = try XCTUnwrap(Test.image(named: "right-orientation.jpeg"))

// When we resize the image to fit 320x480px frame, we expect the processor
// to take image orientation into the account and produce a 320x240px.
Expand All @@ -143,7 +143,7 @@ class ImageProcessorsResizeTests: XCTestCase {
// Given an image with `right` orientation. From the user perspective,
// the image a landscape image with s size 640x480px. The raw pixel
// data, on the other hand, is 480x640px.
let input = try XCTUnwrap(Test.image(named: "left-orientation.jpeg"))
let input = try XCTUnwrap(Test.image(named: "right-orientation.jpeg"))
XCTAssertEqual(input.imageOrientation, .right)

// When we resize the image to fit 320x480px frame, we expect the processor
Expand All @@ -162,7 +162,7 @@ class ImageProcessorsResizeTests: XCTestCase {
// Given an image with `right` orientation. From the user perspective,
// the image a landscape image with s size 640x480px. The raw pixel
// data, on the other hand, is 480x640px.
let input = try XCTUnwrap(Test.image(named: "left-orientation.jpeg"))
let input = try XCTUnwrap(Test.image(named: "right-orientation.jpeg"))
XCTAssertEqual(input.imageOrientation, .right)

// When
Expand Down
Loading