Skip to content

Fix testDownsamplingWithEdgeCaseSize fail #2228

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

Conversation

kittisak-phetrungnapha
Copy link
Contributor

In Xcode 15.3 and Swift 5.10, below code does not return nil if kCGImageSourceThumbnailMaxPixelSize is 0. It returns some image size 64*64 . I am not sure why and it does not make any sense. Maybe a bug from Apple?

let downsampleOptions: [CFString : Any] = [
            kCGImageSourceCreateThumbnailFromImageAlways: true,
            kCGImageSourceShouldCacheImmediately: true,
            kCGImageSourceCreateThumbnailWithTransform: true,
            kCGImageSourceThumbnailMaxPixelSize: 0 // even we set it to 0
        ]

public func CGImageSourceCreateThumbnailAtIndex(_ isrc: CGImageSource, _ index: Int, _ options: CFDictionary?) -> CGImage?

Before fixing:
Screenshot 2567-04-05 at 21 18 30

I just fix the test fail by assert with image size 64*64 and it's passed.

@onevcat
Copy link
Owner

onevcat commented Apr 5, 2024

Thanks for it and I appreciate the effort you made for it.

I am preparing a major update currently and it has been also fixed here in the v8 branch:

d7505c6

As this PR seems to be duplicated to that fix and might fail the test if built with an earlier Xcode (which usually happens on CI), so maybe we can close this one without merging.

However, it is indeed a good idea to have this fix for the master branch as well. If you'd like, can you try to cherry pick that commit above and send another one to the master? Then we can merge it without worrying any further conflicting.

@kittisak-phetrungnapha
Copy link
Contributor Author

@onevcat done.

@onevcat onevcat merged commit 298cb26 into onevcat:master Apr 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants