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

Normalize image color spaces before comparison #665

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ejensen
Copy link
Contributor

@ejensen ejensen commented Oct 22, 2022

Overview

Use the sRGB converted snapshot when doing the perceptual comparison. This reduces the chances of failures when comparing snapshots using different color spaces.

This colorspace normalization technique was originally introduced in #446 and this PR extends it to both the reference and new images when performing perceptual image comparison.

Unit tests were added to verify that images in the P3 and sRGB colors spaces match after colorspace normalization.

Related Issues

Use the sRGB converted snapshot when doing the perceptual comparison

This reduces the chances of failures when comparing snapshots in different color spaces
@pilot34
Copy link

pilot34 commented Oct 2, 2023

Hey. Should we merge this PR after the rebase? Looks like it was approved, but never merged

@Kaspik
Copy link

Kaspik commented Oct 11, 2023

@stephencelis Please, is there a plan to merge this?

# Conflicts:
#	Sources/SnapshotTesting/Snapshotting/NSImage.swift
#	Sources/SnapshotTesting/Snapshotting/UIImage.swift
#	Tests/SnapshotTestingTests/SnapshotTestingTests.swift
@ogiba
Copy link

ogiba commented Apr 22, 2024

@stephencelis I will bump this topic cause there was no answer from few months

Could you provide information what is blocking you from merging it into the main branch?

It will be nice feature/fix that could reduce lots of issues.

Thanks!

@brucerune
Copy link

+1 please merge :)

@adarhef
Copy link

adarhef commented Oct 28, 2024

After a while I figured out the difference between my CI machine and the dev machine was the color space, running on macOS.
Right now I simply normalize the NSWindow used for comparison by setting its color space to sRGB in my snapshotting strategy.
I imagine it won't be necessary if this PR is merged, but I imagine it might not be merged.
While users not understanding failures are due to color space differences is problematic (like just happened to me), I can imagine users might be surprised if images look different in the screenshots they take compared with what the interface looks like when just running on their dev machine.
It might be better to inform people about color space normalization so they can be intentional with their color space choice.
Just a thought

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.

8 participants