tests: Validate known-failure image tests against current Ruffle output

Author: moulins

tests/framework/src/runner/image_test.rs

@@ -1,9 +1,10 @@
 use crate::environment::RenderInterface;
 use crate::options::image_comparison::ImageComparison;
 use crate::util::{read_bytes, write_bytes};
-use anyhow::anyhow;
+use anyhow::{Context as _, anyhow};
 use image::{EncodableLayout, ImageBuffer, ImageFormat, Pixel, PixelWithColorType};
 use ruffle_core::Player;
+use std::borrow::Cow;
 use std::io::Cursor;
 use std::ops::Deref;
 use std::sync::{Arc, Mutex};
@@ -12,12 +13,10 @@ use vfs::VfsPath;
 pub fn capture_and_compare_image(
     base_path: &VfsPath,
     player: &Arc<Mutex<Player>>,
-    name: &String,
+    name: &str,
     image_comparison: ImageComparison,
     render_interface: Option<&dyn RenderInterface>,
 ) -> anyhow::Result<()> {
-    use anyhow::Context;
-
     let Some(render_interface) = render_interface else {
         return Ok(());
     };
@@ -30,69 +29,89 @@ pub fn capture_and_compare_image(
 
     let expected_image = {
         let path = base_path.join(format!("{name}.expected.png"))?;
-        if path.is_file()? {
+        if path.exists()? {
             image::load_from_memory(&read_bytes(&path)?)
                 .context("Failed to open expected image")?
                 .into_rgba8()
         } else if image_comparison.known_failure {
             // If we're expecting this to be wrong, don't save a likely wrong image
             return Err(anyhow!("Image '{name}': No image to compare to!"));
         } else {
-            write_image(&path, &actual_image, ImageFormat::Png)?;
+            write_image(&path, &actual_image)?;
             return Err(anyhow!(
                 "Image '{name}': No image to compare to! Saved actual image as expected."
             ));
         }
     };
 
-    let result = test(
-        &image_comparison,
-        name,
-        actual_image,
-        expected_image,
-        base_path,
-        render_interface.name(),
-        // If we're expecting failure, spamming files isn't productive.
-        !image_comparison.known_failure,
-    );
+    let diff = test(&image_comparison, name, &actual_image, expected_image)?;
+    let (failure, failure_name) = match (diff, image_comparison.known_failure) {
+        (None, false) => return Ok(()),
+        (None, true) => {
+            return Err(anyhow!(
+                "Image '{name}': Check was known to be failing, but now passes successfully. \
+                Please update the test, and remove `known_failure = true` and `{name}.ruffle.png`!",
+            ));
+        }
+        (Some(diff), false) => (diff, Cow::Borrowed(name)),
+        (Some(_), true) => {
+            let ruffle_name = format!("{name}.ruffle");
+            let path = base_path.join(format!("{ruffle_name}.png"))?;
+            let image = if path.exists()? {
+                image::load_from_memory(&read_bytes(&path)?)
+                    .context("Failed to open Ruffle-expected image")?
+                    .into_rgba8()
+            } else {
+                write_image(&path, &actual_image)?;
+                return Err(anyhow!(
+                    "Image '{ruffle_name}': No image to compare to! Saved actual image as Ruffle-expected."
+                ));
+            };
+
+            if let Some(diff) = test(&image_comparison, &ruffle_name, &actual_image, image)? {
+                (diff, Cow::Owned(ruffle_name))
+            } else {
+                return Ok(());
+            }
+        }
+    };
 
-    match (result, image_comparison.known_failure) {
-        (result, false) => result,
-        (Ok(()), true) => Err(anyhow!(
-            "Image '{name}': Check was known to be failing, but now passes successfully. \
-            Please update the test and remove `known_failure = true`!",
-        )),
-        (Err(_), true) => Ok(()),
+    // A comparison failed: write difference images and return an error.
+    let env_name = render_interface.name();
+    write_image(
+        &base_path.join(format!("{name}.actual-{env_name}.png"))?,
+        &actual_image,
+    )?;
+    write_image(
+        &base_path.join(format!("{failure_name}.difference-color-{env_name}.png"))?,
+        &failure.difference_color()?,
+    )?;
+    if let Some(diff_alpha) = failure.difference_alpha()? {
+        write_image(
+            &base_path.join(format!("{failure_name}.difference-alpha-{env_name}.png"))?,
+            &diff_alpha,
+        )?;
     }
+
+    Err(anyhow!(
+        "{failure_name} failed: \
+        Number of outliers ({}) is bigger than allowed limit of {}. \
+        Max difference is {}",
+        failure.outliers,
+        failure.max_outliers,
+        failure.max_difference,
+    ))
 }
 
-pub fn test(
+fn test(
     comparison: &ImageComparison,
     name: &str,
-    actual_image: image::RgbaImage,
+    actual_image: &image::RgbaImage,
     expected_image: image::RgbaImage,
-    test_path: &VfsPath,
-    environment_name: String,
-    save_failures: bool,
-) -> anyhow::Result<()> {
-    use anyhow::Context;
-
-    let save_actual_image = || {
-        if save_failures {
-            write_image(
-                &test_path.join(format!("{name}.actual-{environment_name}.png"))?,
-                &actual_image,
-                ImageFormat::Png,
-            )
-        } else {
-            Ok(())
-        }
-    };
-
+) -> anyhow::Result<Option<ImageDiff>> {
     if actual_image.width() != expected_image.width()
         || actual_image.height() != expected_image.height()
     {
-        save_actual_image()?;
         return Err(anyhow!(
             "'{}' image is not the right size. Expected = {}x{}, actual = {}x{}.",
             name,
@@ -106,7 +125,7 @@ pub fn test(
     let mut is_alpha_different = false;
 
     let difference_data: Vec<u8> =
-        calculate_difference_data(&actual_image, &expected_image, &mut is_alpha_different);
+        calculate_difference_data(actual_image, &expected_image, &mut is_alpha_different);
 
     let checks = comparison
         .checks()
@@ -138,63 +157,61 @@ pub fn test(
         }
 
         // The image failed a check :(
+        return Ok(Some(ImageDiff {
+            width: actual_image.width(),
+            height: actual_image.height(),
+            difference_data,
+            outliers,
+            max_outliers,
+            max_difference,
+            is_alpha_different,
+        }));
+    }
 
-        save_actual_image()?;
+    if !any_check_executed {
+        return Err(anyhow!("Image '{name}' failed: No checks executed."));
+    }
 
+    Ok(None)
+}
+
+struct ImageDiff {
+    width: u32,
+    height: u32,
+    difference_data: Vec<u8>,
+    outliers: usize,
+    max_outliers: usize,
+    max_difference: u8,
+    is_alpha_different: bool,
+}
+
+impl ImageDiff {
+    fn difference_color(&self) -> anyhow::Result<image::RgbImage> {
         let mut difference_color =
-            Vec::with_capacity(actual_image.width() as usize * actual_image.height() as usize * 3);
-        for p in difference_data.chunks_exact(4) {
+            Vec::with_capacity(self.width as usize * self.height as usize * 3);
+        for p in self.difference_data.chunks_exact(4) {
             difference_color.extend_from_slice(&p[..3]);
         }
 
-        if save_failures {
-            let difference_image = image::RgbImage::from_raw(
-                actual_image.width(),
-                actual_image.height(),
-                difference_color,
-            )
-            .context("Couldn't create color difference image")?;
-            write_image(
-                &test_path.join(format!("{name}.difference-color-{environment_name}.png"))?,
-                &difference_image,
-                ImageFormat::Png,
-            )?;
-        }
+        image::RgbImage::from_raw(self.width, self.height, difference_color)
+            .context("Couldn't create color difference image")
+    }
 
-        if is_alpha_different {
+    fn difference_alpha(&self) -> anyhow::Result<Option<image::GrayImage>> {
+        if self.is_alpha_different {
             let mut difference_alpha =
-                Vec::with_capacity(actual_image.width() as usize * actual_image.height() as usize);
-            for p in difference_data.chunks_exact(4) {
+                Vec::with_capacity(self.width as usize * self.height as usize);
+            for p in self.difference_data.chunks_exact(4) {
                 difference_alpha.push(p[3])
             }
 
-            if save_failures {
-                let difference_image = image::GrayImage::from_raw(
-                    actual_image.width(),
-                    actual_image.height(),
-                    difference_alpha,
-                )
-                .context("Couldn't create alpha difference image")?;
-                write_image(
-                    &test_path.join(format!("{name}.difference-alpha-{environment_name}.png"))?,
-                    &difference_image,
-                    ImageFormat::Png,
-                )?;
-            }
+            image::GrayImage::from_raw(self.width, self.height, difference_alpha)
+                .context("Couldn't create alpha difference image")
+                .map(Some)
+        } else {
+            Ok(None)
         }
-
-        return Err(anyhow!(
-            "{check_name} failed: \
-                Number of outliers ({outliers}) is bigger than allowed limit of {max_outliers}. \
-                Max difference is {max_difference}",
-        ));
     }
-
-    if !any_check_executed {
-        return Err(anyhow!("Image '{name}' failed: No checks executed.",));
-    }
-
-    Ok(())
 }
 
 fn calculate_difference_data(
@@ -248,15 +265,14 @@ fn calc_difference(lhs: u8, rhs: u8) -> u8 {
 fn write_image<P, Container>(
     path: &VfsPath,
     image: &ImageBuffer<P, Container>,
-    format: ImageFormat,
 ) -> anyhow::Result<()>
 where
     P: Pixel + PixelWithColorType,
     [P::Subpixel]: EncodableLayout,
     Container: Deref<Target = [P::Subpixel]>,
 {
     let mut buffer = vec![];
-    image.write_to(&mut Cursor::new(&mut buffer), format)?;
+    image.write_to(&mut Cursor::new(&mut buffer), ImageFormat::Png)?;
     write_bytes(path, &buffer)?;
     Ok(())
 }

tests/tests/swfs/visual/cache_as_bitmap/oversize/swf_10_too_big/test.toml

@@ -2,7 +2,7 @@ num_frames = 1
 
 [image_comparisons.output]
 known_failure = true
-tolerance = 0
+tolerance = 1 # Should be set to 0 when removing known_failure
 
 [player_options]
 with_renderer = { optional = false, sample_count = 4 }

tests/tests/swfs/visual/cache_as_bitmap/oversize/swf_9_too_big/test.toml

@@ -2,7 +2,7 @@ num_frames = 1
 
 [image_comparisons.output]
 known_failure = true
-tolerance = 0
+tolerance = 1 # Should be set to 0 when removing known_failure
 
 [player_options]
 with_renderer = { optional = false, sample_count = 4 }