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

usdImagingGL idiff tests should be able to detect the difference between visibly distinct reference images #1866

Conversation

wrma6
Copy link

@wrma6 wrma6 commented May 17, 2022

Description of Change(s)

With the current parameters used for the image diff tests in usdImagingGL, the reference images cannot be distinguished from each other: idiff passes many image pairs as "similar enough" even though they are (visibly) distinctly different. Specifically, the Fail parameter of 1 (the most lenient value possible) usually passes every image pair, so tests using that rely solely on the additional Perceptual test. Using a Python script, all pairs of images referenced by each test were compared using idiff to determine reasonable Fail and FailPercent parameters. The proposed change would ensure that for the parameters and reference images of each test, the parameters can at least differentiate between any pair of reference images that differ.

Fixes Issue(s)

  • 22 tests have their Fail parameter changed to .05 and their FailPercent parameter changed to .03. This means to fail a test between two given images, if more than 0.03% of their pixels differ by more than 0.05 on the 1-255 colour scale (colour difference of about 12.5).
  • I have submitted a signed Contributor License Agreement

@pmolodo
Copy link
Contributor

pmolodo commented May 19, 2022

Noticed that this covers similar territory as this commit:
b5aa212
...but seems to fix different tests. Should the error threshholds we're altering here be updated to match the more stringent ones in that commit?

@jilliene
Copy link

Filed as internal issue #USD-7393

@wrma6 wrma6 force-pushed the pr/imagediff-detect-reference-diffs branch from 3f090c2 to 98174c5 Compare July 7, 2022 18:50
@pmolodo
Copy link
Contributor

pmolodo commented Jul 7, 2022

Just poking to see about getting this merged...

@spiffmon
Copy link
Member

spiffmon commented Jul 8, 2022

@pmolodo , this is in our queue, but our recent focus for 22.08, oss-imaging-wise, has been native silicon build support.

@pmolodo
Copy link
Contributor

pmolodo commented Jul 8, 2022

Got it, thanks for the info!

@pixar-oss pixar-oss merged commit dab79a0 into PixarAnimationStudios:dev Sep 1, 2022
pixar-oss pushed a commit that referenced this pull request Jun 7, 2023
This is a follow-up to change 2277652. That change tightened
image diff thresholds but were overfitted to internal
workstations and caused failures when run on other machines.

This change relaxes the thresholds to match values that were
used in usdImagingGL tests in change 2245969, which were
contributed in PR #1866. Using these values should minimize
false negatives on a wider range of machines while still
catching major differences. Some baseline images were
updated to fall within these new tolerances.

(Internal change: 2279351)
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.

5 participants