Skip to content

Feature: add pixel_threshold for tests #7092

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

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

Conversation

bircni
Copy link
Contributor

@bircni bircni commented May 26, 2025

I thought about this - so we have two options here:

  1. adding it to SnapshotOptions
  2. adding it to every function which I do not like as this would be a huge breaking change

Summary

This pull request introduces a new feature to the SnapshotOptions struct in the egui_kittest crate, allowing users to specify a permissible percentage of pixel differences (diff_percentage) before a snapshot comparison is considered a failure. This feature provides more flexibility in handling minor visual discrepancies during snapshot testing.

Additions to SnapshotOptions:

  • Added a new field diff_percentage of type Option<f64> to the SnapshotOptions struct. This field allows users to define a tolerance for pixel differences, with a default value of None (interpreted as 0% tolerance).
  • Updated the Default implementation of SnapshotOptions to initialize diff_percentage to None.

Integration into snapshot comparison logic:

  • Updated the try_image_snapshot_options function to handle the new diff_percentage field. If a diff_percentage is specified, the function calculates the percentage of differing pixels and allows the snapshot to pass if the difference is within the specified tolerance. [1] [2]

  • Closes Add pixel_threshold and improve threshold docs #5683

  • I have followed the instructions in the PR template

@bircni bircni requested a review from lucasmerlin as a code owner May 26, 2025 15:46
@bircni
Copy link
Contributor Author

bircni commented May 26, 2025

To discuss:
Should this be an Option or just have a default value like 0.01 ?

I would be fine with both but I think per default it should be 0

Copy link

Preview available at https://egui-pr-preview.github.io/pr/7092-patch5683
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

Copy link
Collaborator

@lucasmerlin lucasmerlin left a comment

Choose a reason for hiding this comment

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

I'd prefer if this was a threshold of broken pixels instead of percentage, which would be more convenient since it's also what the error prints on test failure.
e.g. in

'easymarkeditor' Image did not match snapshot. Diff: 420, "/Users/runner/work/egui/egui/crates/egui_demo_app/tests/snapshots/easymarkeditor.diff.png". Run `UPDATE_SNAPSHOTS=1 cargo test --all-features` to update the snapshots.

420 means 420 pixels were above the threshold.
If we had a pixel_threshold option you could just use the 420 instead of having to calculate or guess the percentage.

Also, it'd be nice to have a helper to add a per-platform threshold. It could just be a struct with {windows: f32, linux: f32, ...} and impl Into so you could use it directly on the threshold param.

@bircni
Copy link
Contributor Author

bircni commented Jun 2, 2025

@lucasmerlin done :-)

@bircni bircni changed the title Feature: add diff_percentage for tests Feature: add pixel_threshold for tests Jun 3, 2025
@lucasmerlin lucasmerlin added feature New feature or request egui_kittest labels Jun 5, 2025
Copy link
Collaborator

@lucasmerlin lucasmerlin left a comment

Choose a reason for hiding this comment

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

Awesome!

@bircni
Copy link
Contributor Author

bircni commented Jun 5, 2025

@lucasmerlin fixed

@bircni bircni requested a review from lucasmerlin June 11, 2025 18:13
@bircni bircni force-pushed the patch5683 branch 2 times, most recently from be59e33 to 1a4a6a1 Compare June 12, 2025 20:12
@bircni
Copy link
Contributor Author

bircni commented Jun 14, 2025

Rebased to keep it ready to merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
egui_kittest feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pixel_threshold and improve threshold docs
2 participants