-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
base: main
Are you sure you want to change the base?
Conversation
To discuss: I would be fine with both but I think per default it should be 0 |
Preview available at https://egui-pr-preview.github.io/pr/7092-patch5683 |
There was a problem hiding this 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.
@lucasmerlin done :-) |
diff_percentage
for testspixel_threshold
for tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
@lucasmerlin fixed |
be59e33
to
1a4a6a1
Compare
Rebased to keep it ready to merge |
I thought about this - so we have two options here:
SnapshotOptions
Summary
This pull request introduces a new feature to the
SnapshotOptions
struct in theegui_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
:diff_percentage
of typeOption<f64>
to theSnapshotOptions
struct. This field allows users to define a tolerance for pixel differences, with a default value ofNone
(interpreted as 0% tolerance).Default
implementation ofSnapshotOptions
to initializediff_percentage
toNone
.Integration into snapshot comparison logic:
Updated the
try_image_snapshot_options
function to handle the newdiff_percentage
field. If adiff_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