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

Fixed scale-agnostic image size comparison #100

Merged
merged 1 commit into from
Sep 18, 2019
Merged

Conversation

Harmek
Copy link
Contributor

@Harmek Harmek commented Aug 28, 2019

Previously the code would compare UIImage size properties, which are not in pixel units but scale-dependent points.
This would cause issue if a test uses reference images generated using .none fileNameOptions and ran tests on 2x or 3x simulators: they would fail early because of the size comparison because of the scale difference.

This commit changes the logic to use CGImageGet{Width,Height} instead, which returns the actual pixel size (akin to use size * scale).
This change makes sense since the image comparison code actually uses CGImage pixel retrieving methods.

Previously the code would compare UIImage size properties, which are not in pixel units but scale-dependent points.
This would cause issue if a test uses reference images generated using .none fileNameOptions and ran tests on 2x or 3x simulators: they would fail early because of the size comparison because of the scale difference.
This commit changes the logic to use CGImageGet{Width,Height} instead, which returns the actual pixel size (akin to use size * scale).
This change makes sense since the image comparison code actually uses CGImage pixel retrieving methods.
@CLAassistant
Copy link

CLAassistant commented Aug 28, 2019

CLA assistant check
All committers have signed the CLA.

@reidmain
Copy link

Very nice catch @Harmek. This actually fixes #81 as well.

@reidmain reidmain merged commit 0bbf957 into uber:master Sep 18, 2019
reidmain pushed a commit that referenced this pull request Sep 18, 2019
Added PR #100 as the first change that will make it into the next release.
lukaskukacka pushed a commit to lukaskukacka/ios-snapshot-test-case that referenced this pull request Jan 3, 2020
Previously we would compare UIImage size properties, which are not in pixel units but scale-dependent points. This would cause issues if a test used reference images generated using .none fileNameOptions and ran tests on 2x or 3x simulators: they would fail early because of the size comparison because of the scale difference.

This commit changes the logic to use CGImageGet{Width,Height} instead, which returns the actual pixel size (akin to use size * scale) and is backwards compatible since the image comparison code actually uses CGImage pixel retrieving methods.
lukaskukacka pushed a commit to lukaskukacka/ios-snapshot-test-case that referenced this pull request Jan 3, 2020
Added PR uber#100 as the first change that will make it into the next release.
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.

None yet

3 participants