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

Compare image contexts in same colorspace #446

Merged
merged 9 commits into from Jul 22, 2021

Conversation

dflems
Copy link
Contributor

@dflems dflems commented Mar 26, 2021

I ran some of our snapshots through a lossless image optimizer (ImageOptim) and it made a lot of them significantly smaller (removing unnecessary alpha channel, converting to monochrome or 8-bit color, etc). Unfortunately, after this conversion, image comparison failed in swift-snapshot-testing. I think this is because the contexts were compared against one-another using different color spaces.

This PR converts both the snapshot and the reference to the same colorspace when comparing bytes: SRGB with alpha channel. I don't know if this is the best or the most efficient but it seemed consistent enough and worked against our 1,600 snapshot tests. If anyone has some better suggestions, I'm open to hearing them (generally unfamiliar with this kind of stuff)

@dflems
Copy link
Contributor Author

dflems commented Mar 26, 2021

@stephencelis

@dflems
Copy link
Contributor Author

dflems commented Apr 16, 2021

@stephencelis 👋

zenangst added a commit to zenangst/swift-snapshot-testing that referenced this pull request Apr 21, 2021
zenangst added a commit to zenangst/swift-snapshot-testing that referenced this pull request Apr 21, 2021
@dflems
Copy link
Contributor Author

dflems commented May 13, 2021

@stephencelis 👋 again

@dflems
Copy link
Contributor Author

dflems commented Jul 15, 2021

@stephencelis ready to give up on this PR... any feedback?

Copy link
Member

@stephencelis stephencelis left a comment

Choose a reason for hiding this comment

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

Ha sorry about the delay! We have a lot of projects in-flight but this one has had the stablest API, so it's gotten a little less attention. We reviewed it this morning and it looks good to merge!

@stephencelis stephencelis merged commit d5962c2 into pointfreeco:main Jul 22, 2021
@dflems dflems deleted the colorspace branch July 22, 2021 23:13
mac-gallagher pushed a commit to mac-gallagher/swift-snapshot-testing that referenced this pull request Aug 22, 2021
Co-authored-by: Stephen Celis <stephen@stephencelis.com>
Co-authored-by: Brandon Williams <135203+mbrandonw@users.noreply.github.com>
niil-qb pushed a commit to quickbit/swift-snapshot-testing that referenced this pull request Feb 23, 2023
Co-authored-by: Stephen Celis <stephen@stephencelis.com>
Co-authored-by: Brandon Williams <135203+mbrandonw@users.noreply.github.com>
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