From dae9adf307b36d97f3254e43c356818280d32ed0 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 09:02:41 -0700 Subject: [PATCH] Rename `failed_pixel_count_threshold` to `max_failed_pixels` (#8383) It was confusing that both tolerances had "threshold" in the name --------- Co-authored-by: Claude Opus 5 (1M context) --- crates/egui_kittest/README.md | 21 ++++-- crates/egui_kittest/src/config.rs | 112 ++++++++++++++++++++++------ crates/egui_kittest/src/snapshot.rs | 81 ++++++++++++++------ 3 files changed, 163 insertions(+), 51 deletions(-) diff --git a/crates/egui_kittest/README.md b/crates/egui_kittest/README.md index 7b97f4e1d..8711aeabb 100644 --- a/crates/egui_kittest/README.md +++ b/crates/egui_kittest/README.md @@ -44,25 +44,32 @@ All possible settings and their defaults: # path to the snapshot directory output_path = "tests/snapshots" -# default threshold for image comparison tests +# maximum weighted squared YIQ color distance between two corresponding pixels +# (a per-pixel color tolerance, applied to each pixel pair on its own) threshold = 0.6 -# default failed_pixel_count_threshold -failed_pixel_count_threshold = 0 +# how many pixels may exceed the `threshold` before the test fails +# (an absolute pixel count, not a fraction of the image) +max_failed_pixels = 0 [windows] threshold = 0.6 -failed_pixel_count_threshold = 0 +max_failed_pixels = 0 [macos] threshold = 0.6 -failed_pixel_count_threshold = 0 +max_failed_pixels = 0 [linux] threshold = 0.6 -failed_pixel_count_threshold = 0 +max_failed_pixels = 0 ``` +Raise `max_failed_pixels` only very carefully: a high value (more than ~10) is enough to hide a +real change, such as a moved separator, a shifted one-pixel border, or a small icon rendering +incorrectly. Prefer the smallest value that makes the test pass, and re-check it whenever you +update the snapshot. + ## Snapshot testing There is a snapshot testing feature. To create snapshot tests, enable the `snapshot` and `wgpu` features. Once enabled, you can call `Harness::snapshot` to render the ui and save the image to the `tests/snapshots` directory. @@ -105,7 +112,7 @@ However, especially when you're using custom rendering, you may observe images d First check whether the difference is due to a change in enabled rendering features, potentially due to difference in hardware (/software renderer) capabilities. Generally you should carefully enforcing the same set of features for all test runs, but this may happen nonetheless. -Once you validated that the differences are miniscule and hard to avoid, you can try to _carefully_ adjust the comparison tolerance setting (`SnapshotOptions::threshold`, TODO([#5683](https://github.com/emilk/egui/issues/5683)): as well as number of pixels allowed to differ) for the specific test. +Once you validated that the differences are miniscule and hard to avoid, you can try to _carefully_ adjust the comparison tolerances (`SnapshotOptions::threshold` and, as a last resort, `SnapshotOptions::max_failed_pixels`) for the specific test. See also TODO([#5683](https://github.com/emilk/egui/issues/5683)). ⚠️ **WARNING** ⚠️ Picking too high tolerances may mean that you are missing actual test failures. diff --git a/crates/egui_kittest/src/config.rs b/crates/egui_kittest/src/config.rs index a94075973..cdb74e00f 100644 --- a/crates/egui_kittest/src/config.rs +++ b/crates/egui_kittest/src/config.rs @@ -15,15 +15,20 @@ pub struct Config { /// Default is "tests/snapshots" (relative to the working directory / crate root). output_path: PathBuf, - /// The per-pixel threshold. + /// The maximum weighted squared YIQ color distance between two corresponding pixels. + /// + /// Pixels that differ by more than this are counted as failing. + /// This is an absolute, per-pixel value, and does not depend on the image dimensions. /// /// Default is 0.6. threshold: f32, - /// The number of pixels that can differ before the test is considered failed. + /// The number of pixels that may fail the [`Self::threshold`] before the test is + /// considered failed. /// /// Default is 0. - failed_pixel_count_threshold: usize, + #[serde(alias = "failed_pixel_count_threshold")] + max_failed_pixels: usize, windows: OsConfig, mac: OsConfig, @@ -35,7 +40,7 @@ impl Default for Config { Self { output_path: PathBuf::from("tests/snapshots"), threshold: 0.6, - failed_pixel_count_threshold: 0, + max_failed_pixels: 0, windows: Default::default(), mac: Default::default(), linux: Default::default(), @@ -48,8 +53,9 @@ pub struct OsConfig { /// Override the per-pixel threshold for this OS. threshold: Option, - /// Override the failed pixel count threshold for this OS. - failed_pixel_count_threshold: Option, + /// Override the maximum number of failing pixels for this OS. + #[serde(alias = "failed_pixel_count_threshold")] + max_failed_pixels: Option, } fn find_kittest_toml() -> io::Result { @@ -72,13 +78,44 @@ fn find_kittest_toml() -> io::Result { } } +/// The old name of `max_failed_pixels` is still accepted, but warned about. +fn warn_about_deprecated_keys(config_str: &str) { + let Ok(config) = toml::from_str::(config_str) else { + return; + }; + + let mut sections = vec![("", &config)]; + for name in ["windows", "mac", "linux"] { + if let Some(table) = config.get(name).and_then(toml::Value::as_table) { + sections.push((name, table)); + } + } + + for (section, table) in sections { + if table.contains_key("failed_pixel_count_threshold") { + let prefix = if section.is_empty() { + String::new() + } else { + format!("{section}.") + }; + log::warn!( + "`{prefix}failed_pixel_count_threshold` in kittest.toml is deprecated; \ + use `{prefix}max_failed_pixels` instead." + ); + } + } +} + fn load_config() -> Config { if let Ok(config_path) = find_kittest_toml() { match std::fs::read_to_string(&config_path) { - Ok(config_str) => match toml::from_str(&config_str) { - Ok(config) => config, - Err(e) => panic!("Failed to parse {}: {e}", config_path.display()), - }, + Ok(config_str) => { + warn_about_deprecated_keys(&config_str); + match toml::from_str(&config_str) { + Ok(config) => config, + Err(err) => panic!("Failed to parse {}: {err}", config_path.display()), + } + } Err(err) => { panic!("Failed to read {}: {}", config_path.display(), err); } @@ -127,30 +164,59 @@ impl Config { } } - pub fn os_failed_pixel_count_threshold(&self) -> crate::OsThreshold { - let fallback = self.failed_pixel_count_threshold; + pub fn os_max_failed_pixels(&self) -> crate::OsThreshold { + let fallback = self.max_failed_pixels; crate::OsThreshold { - windows: self - .windows - .failed_pixel_count_threshold - .unwrap_or(fallback), - macos: self.mac.failed_pixel_count_threshold.unwrap_or(fallback), - linux: self.linux.failed_pixel_count_threshold.unwrap_or(fallback), + windows: self.windows.max_failed_pixels.unwrap_or(fallback), + macos: self.mac.max_failed_pixels.unwrap_or(fallback), + linux: self.linux.max_failed_pixels.unwrap_or(fallback), fallback, } } - /// The threshold. + /// The maximum weighted squared YIQ color distance between two corresponding pixels. /// - /// Default is 1.0. + /// This is an absolute, per-pixel value, and does not depend on the image dimensions. + /// + /// Default is 0.6. pub fn threshold(&self) -> f32 { self.os_threshold().threshold() } - /// The number of pixels that can differ before the test is considered failed. + /// The number of pixels that may fail the [`Self::threshold`] before the test is + /// considered failed. /// /// Default is 0. - pub fn failed_pixel_count_threshold(&self) -> usize { - self.os_failed_pixel_count_threshold().threshold() + pub fn max_failed_pixels(&self) -> usize { + self.os_max_failed_pixels().threshold() + } +} + +#[cfg(test)] +mod tests { + use super::Config; + + #[test] + fn deprecated_failed_pixel_count_threshold_key_is_accepted() { + let config: Config = toml::from_str( + r" + failed_pixel_count_threshold = 1 + + [windows] + failed_pixel_count_threshold = 2 + + [mac] + failed_pixel_count_threshold = 3 + + [linux] + failed_pixel_count_threshold = 4 + ", + ) + .unwrap_or_else(|err| panic!("Failed to parse config: {err}")); + + assert_eq!(config.max_failed_pixels, 1); + assert_eq!(config.windows.max_failed_pixels, Some(2)); + assert_eq!(config.mac.max_failed_pixels, Some(3)); + assert_eq!(config.linux.max_failed_pixels, Some(4)); } } diff --git a/crates/egui_kittest/src/snapshot.rs b/crates/egui_kittest/src/snapshot.rs index 268e0749f..09ce986c8 100644 --- a/crates/egui_kittest/src/snapshot.rs +++ b/crates/egui_kittest/src/snapshot.rs @@ -11,18 +11,35 @@ pub type SnapshotResult = Result<(), SnapshotError>; #[non_exhaustive] #[derive(Clone, Debug)] pub struct SnapshotOptions { - /// The threshold for the image comparison. + /// How much a single pixel may differ before it is counted as failing: + /// the maximum weighted squared YIQ color distance between two corresponding pixels. + /// + /// This is a color tolerance, not an error budget for the image as a whole: + /// it is applied to each pixel pair on its own, and raising it makes every pixel + /// more forgiving. Use [`Self::max_failed_pixels`] to allow a number of pixels + /// to exceed it. /// /// Can be configured via kittest.toml. The fallback is `0.6` (which is enough for most egui /// tests to pass across different wgpu backends). pub threshold: f32, - /// The number of pixels that can differ before the snapshot is considered a failure. + /// The number of pixels that may fail the [`Self::threshold`] before the snapshot is + /// considered a failure. /// - /// Preferably, you should use `threshold` to control the sensitivity of the image comparison. + /// This is an absolute pixel count, not a fraction of the image, so the same value is + /// stricter for a large snapshot than for a small one. + /// + /// Preferably, you should use [`Self::threshold`] to control the sensitivity of the image + /// comparison. /// As a last resort, you can use this to allow a certain number of pixels to differ. + /// + /// Raise this only very carefully: a high value (more than ~10) is enough to hide a real + /// change, such as a moved separator, a shifted one-pixel border, or a small icon rendering + /// incorrectly. Prefer the smallest value that makes the test pass, and re-check it whenever + /// you update the snapshot. + /// /// Can be configured via kittest.toml. The fallback is `0` (meaning no pixels can differ). - pub failed_pixel_count_threshold: usize, + pub max_failed_pixels: usize, /// The path where the snapshots will be saved. /// @@ -33,12 +50,12 @@ pub struct SnapshotOptions { pub output_path: PathBuf, } -/// Helper struct to define the number of pixels that can differ before the snapshot is considered a failure. +/// Helper struct to define a per-OS comparison tolerance. /// -/// This is useful if you want to set different thresholds for different operating systems. +/// This is useful if you want to set different tolerances for different operating systems. /// /// [`OsThreshold::default`] gets the default from the config file (`kittest.toml`). -/// For `usize`, it's the `failed_pixel_count_threshold` value. +/// For `usize`, it's the `max_failed_pixels` value. /// For `f32`, it's the `threshold` value. /// /// Example usage: @@ -51,7 +68,7 @@ pub struct SnapshotOptions { /// "os_threshold_example", /// &SnapshotOptions::new() /// .threshold(OsThreshold::new(0.0).windows(10.0)) -/// .failed_pixel_count_threshold(OsThreshold::new(0).windows(10).macos(53) +/// .max_failed_pixels(OsThreshold::new(0).windows(10).macos(53) /// )) /// ``` #[derive(Debug, Clone, Copy)] @@ -63,11 +80,11 @@ pub struct OsThreshold { } impl Default for OsThreshold { - /// Returns the default `failed_pixel_count_threshold` as configured in `kittest.toml` + /// Returns the default `max_failed_pixels` as configured in `kittest.toml` /// /// The fallback is `0`. fn default() -> Self { - config().os_failed_pixel_count_threshold() + config().os_max_failed_pixels() } } @@ -158,7 +175,7 @@ impl Default for SnapshotOptions { Self { threshold: config().threshold(), output_path: config().output_path(), - failed_pixel_count_threshold: config().failed_pixel_count_threshold(), + max_failed_pixels: config().max_failed_pixels(), } } } @@ -169,7 +186,14 @@ impl SnapshotOptions { Default::default() } - /// Change the threshold for the image comparison. + /// Change how much a single pixel may differ before it is counted as failing: + /// the maximum weighted squared YIQ color distance between two corresponding pixels. + /// + /// This is a color tolerance, not an error budget for the image as a whole: + /// it is applied to each pixel pair on its own, and raising it makes every pixel + /// more forgiving. Use [`Self::max_failed_pixels`] to allow a number of pixels + /// to exceed it. + /// /// The default is `0.6` (which is enough for most egui tests to pass across different /// wgpu backends). #[inline] @@ -187,18 +211,33 @@ impl SnapshotOptions { self } - /// Change the number of pixels that can differ before the snapshot is considered a failure. + /// Change the number of pixels that may fail the [`Self::threshold`] before the snapshot is + /// considered a failure. + /// + /// This is an absolute pixel count, not a fraction of the image, so the same value is + /// stricter for a large snapshot than for a small one. /// /// Preferably, you should use [`Self::threshold`] to control the sensitivity of the image comparison. /// As a last resort, you can use this to allow a certain number of pixels to differ. + /// + /// Raise this only very carefully: a high value (more than ~10) is enough to hide a real + /// change, such as a moved separator, a shifted one-pixel border, or a small icon rendering + /// incorrectly. Prefer the smallest value that makes the test pass, and re-check it whenever + /// you update the snapshot. + #[inline] + pub fn max_failed_pixels(mut self, max_failed_pixels: impl Into>) -> Self { + self.max_failed_pixels = max_failed_pixels.into().threshold(); + self + } + + /// Renamed to [`Self::max_failed_pixels`]. + #[deprecated(since = "0.36.0", note = "Renamed to max_failed_pixels")] #[inline] pub fn failed_pixel_count_threshold( - mut self, - failed_pixel_count_threshold: impl Into>, + self, + max_failed_pixels: impl Into>, ) -> Self { - let failed_pixel_count_threshold = failed_pixel_count_threshold.into().threshold(); - self.failed_pixel_count_threshold = failed_pixel_count_threshold; - self + self.max_failed_pixels(max_failed_pixels) } } @@ -219,7 +258,7 @@ pub enum SnapshotError { /// /// Measured at [`THRESHOLD_SWEEP`], lowest threshold first. /// Use this to pick a [`SnapshotOptions::threshold`] and a - /// [`SnapshotOptions::failed_pixel_count_threshold`] from measurements, + /// [`SnapshotOptions::max_failed_pixels`] from measurements, /// instead of by trial and error. failing_pixels_by_threshold: Vec<(f32, i32)>, }, @@ -441,7 +480,7 @@ fn try_image_snapshot_options_impl( let SnapshotOptions { threshold, output_path, - failed_pixel_count_threshold, + max_failed_pixels, } = options; let parent_path = if let Some(parent) = PathBuf::from(&name).parent() { @@ -544,7 +583,7 @@ fn try_image_snapshot_options_impl( return Ok(()); // Difference below threshold }; - let below_threshold = num_wrong_pixels as i64 <= *failed_pixel_count_threshold as i64; + let below_threshold = num_wrong_pixels as i64 <= *max_failed_pixels as i64; if !below_threshold { diff_image