Skip to content

Compare image contexts in same colorspace#446

Merged
stephencelis merged 9 commits into
pointfreeco:mainfrom
dflems:colorspace
Jul 22, 2021
Merged

Compare image contexts in same colorspace#446
stephencelis merged 9 commits into
pointfreeco:mainfrom
dflems:colorspace

Conversation

@dflems

@dflems dflems commented Mar 26, 2021

Copy link
Copy Markdown
Contributor

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

dflems commented Mar 26, 2021

Copy link
Copy Markdown
Contributor Author

@stephencelis

@dflems

dflems commented Apr 16, 2021

Copy link
Copy Markdown
Contributor Author

@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

dflems commented May 13, 2021

Copy link
Copy Markdown
Contributor Author

@stephencelis 👋 again

@dflems

dflems commented Jul 15, 2021

Copy link
Copy Markdown
Contributor Author

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

@stephencelis stephencelis left a comment

Copy link
Copy Markdown
Member

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.

3 participants