diff --git a/tests/framework/src/environment.rs b/tests/framework/src/environment.rs index 9c3d76d27..f1be8cff1 100644 --- a/tests/framework/src/environment.rs +++ b/tests/framework/src/environment.rs @@ -14,13 +14,25 @@ pub trait Environment { false } - /// Creates a render backend for the given test. + /// Creates a render backend for a new test run. + /// + /// This method must return both a [RenderBackend] and [RenderInterface] as a pair, + /// and will be treated as a pair for the purposes of this test framework. + /// + /// All relevant methods in the [RenderInterface] will receive the same [RenderBackend] + /// that was provided here with that interface. /// /// If [Self::is_render_supported] returned false, this won't be attempted. - fn create_renderer(&self, _width: u32, _height: u32) -> Option> { + fn create_renderer( + &self, + _width: u32, + _height: u32, + ) -> Option<(Box, Box)> { None } +} +pub trait RenderInterface { /// Gets the name of this environment, for use in test reporting. /// /// This name may be used in file paths, so it should contain appropriate characters for such. @@ -28,6 +40,6 @@ pub trait Environment { /// Capture the stage rendered out by the given render backend. /// - /// The provided backend will have previously been created by [Environment::create_renderer]. - fn capture_renderer(&self, renderer: &mut Box) -> image::RgbaImage; + /// The provided backend is guaranteed to be the same one paired with this interface. + fn capture(&self, renderer: &mut Box) -> image::RgbaImage; } diff --git a/tests/framework/src/options.rs b/tests/framework/src/options.rs index 4b97d3b8c..304375a96 100644 --- a/tests/framework/src/options.rs +++ b/tests/framework/src/options.rs @@ -1,11 +1,12 @@ use crate::backends::TestAudioBackend; -use crate::environment::Environment; +use crate::environment::{Environment, RenderInterface}; use crate::image_trigger::ImageTrigger; use anyhow::{anyhow, Result}; use approx::assert_relative_eq; use regex::Regex; use ruffle_core::tag_utils::SwfMovie; use ruffle_core::{PlayerBuilder, ViewportDimensions}; +use ruffle_render::backend::RenderBackend; use ruffle_render::quality::StageQuality; use serde::Deserialize; use std::collections::{HashMap, HashSet}; @@ -136,30 +137,11 @@ pub struct PlayerOptions { } impl PlayerOptions { - pub fn setup( - &self, - mut player_builder: PlayerBuilder, - movie: &SwfMovie, - environment: &impl Environment, - ) -> Result { + pub fn setup(&self, mut player_builder: PlayerBuilder) -> Result { if let Some(max_execution_duration) = self.max_execution_duration { player_builder = player_builder.with_max_execution_duration(max_execution_duration); } - let (width, height) = if let Some(viewport_dimensions) = self.viewport_dimensions { - player_builder = player_builder.with_viewport_dimensions( - viewport_dimensions.width, - viewport_dimensions.height, - viewport_dimensions.scale_factor, - ); - (viewport_dimensions.width, viewport_dimensions.height) - } else { - ( - movie.width().to_pixels() as u32, - movie.height().to_pixels() as u32, - ) - }; - if let Some(render_options) = &self.with_renderer { player_builder = player_builder.with_quality(match render_options.sample_count { 16 => StageQuality::High16x16, @@ -168,10 +150,6 @@ impl PlayerOptions { 2 => StageQuality::Medium, _ => StageQuality::Low, }); - - if let Some(renderer) = environment.create_renderer(width, height) { - player_builder = player_builder.with_boxed_renderer(renderer); - } } if self.with_audio { @@ -197,6 +175,27 @@ impl PlayerOptions { } true } + + pub fn viewport_dimensions(&self, movie: &SwfMovie) -> ViewportDimensions { + self.viewport_dimensions + .unwrap_or_else(|| ViewportDimensions { + width: movie.width().to_pixels() as u32, + height: movie.height().to_pixels() as u32, + scale_factor: 1.0, + }) + } + + pub fn create_renderer( + &self, + environment: &impl Environment, + dimensions: ViewportDimensions, + ) -> Option<(Box, Box)> { + if self.with_renderer.is_some() { + environment.create_renderer(dimensions.width, dimensions.height) + } else { + None + } + } } #[derive(Deserialize, Default, Clone, Debug)] diff --git a/tests/framework/src/runner.rs b/tests/framework/src/runner.rs index 1a10d0bab..0b610d8f4 100644 --- a/tests/framework/src/runner.rs +++ b/tests/framework/src/runner.rs @@ -1,5 +1,5 @@ use crate::backends::{TestLogBackend, TestNavigatorBackend, TestUiBackend}; -use crate::environment::Environment; +use crate::environment::RenderInterface; use crate::fs_commands::{FsCommand, TestFsCommandProvider}; use crate::image_trigger::ImageTrigger; use crate::options::ImageComparison; @@ -15,7 +15,7 @@ use ruffle_input_format::{ AutomatedEvent, InputInjector, MouseButton as InputMouseButton, TextControlCode as InputTextControlCode, }; -use ruffle_render::backend::null::NullRenderer; +use ruffle_render::backend::{RenderBackend, ViewportDimensions}; use ruffle_socket_format::SocketEvent; use std::path::Path; use std::sync::{Arc, Mutex}; @@ -23,17 +23,19 @@ use std::time::Duration; /// Loads an SWF and runs it through the Ruffle core for a number of frames. /// Tests that the trace output matches the given expected output. +#[allow(clippy::too_many_arguments)] pub fn run_swf( test: &Test, + movie: SwfMovie, mut injector: InputInjector, socket_events: Option>, before_start: impl FnOnce(Arc>) -> Result<()>, before_end: impl FnOnce(Arc>) -> Result<()>, - environment: &impl Environment, + renderer: Option<(Box, Box)>, + viewport_dimensions: ViewportDimensions, ) -> Result { let base_path = Path::new(&test.output_path).parent().unwrap(); let mut executor = NullExecutor::new(); - let movie = SwfMovie::from_path(&test.swf_path, None).map_err(|e| anyhow!(e.to_string()))?; let mut frame_time = 1000.0 / movie.frame_rate().to_f64(); if let Some(tr) = test.options.tick_rate { frame_time = tr; @@ -50,35 +52,34 @@ pub fn run_swf( test.options.log_fetch.then(|| log.clone()), )?; - let builder = PlayerBuilder::new() + let mut builder = PlayerBuilder::new() .with_log(log.clone()) .with_navigator(navigator) .with_max_execution_duration(Duration::from_secs(300)) .with_fs_commands(Box::new(fs_command_provider)) .with_ui(TestUiBackend) .with_viewport_dimensions( - movie.width().to_pixels() as u32, - movie.height().to_pixels() as u32, - 1.0, + viewport_dimensions.width, + viewport_dimensions.height, + viewport_dimensions.scale_factor, ); + let render_interface = if let Some((interface, backend)) = renderer { + builder = builder.with_boxed_renderer(backend); + Some(interface) + } else { + None + }; + // Test player options may override anything set above let player = test .options .player_options - .setup(builder, &movie, environment)? + .setup(builder)? .with_movie(movie) .with_autoplay(true) //.tick() requires playback .build(); - // If we set anything but a null renderer, then the Environment must have created a valid renderer. - let has_renderer = player - .lock() - .unwrap() - .renderer() - .downcast_ref::() - .is_none(); - let mut images = test.options.image_comparisons.clone(); before_start(player.clone())?; @@ -148,8 +149,7 @@ pub fn run_swf( &name, image_comparison, test.options.known_failure, - environment, - has_renderer, + render_interface.as_deref(), )?; } else { return Err(anyhow!("Encountered fscommand to capture and compare image '{name}', but no [image_comparison] was set up for this.")); @@ -221,8 +221,7 @@ pub fn run_swf( &name, image_comparison, test.options.known_failure, - environment, - has_renderer, + render_interface.as_deref(), )?; } } @@ -242,8 +241,7 @@ pub fn run_swf( &name, image_comparison, test.options.known_failure, - environment, - has_renderer, + render_interface.as_deref(), )?; } @@ -272,16 +270,15 @@ fn capture_and_compare_image( name: &String, image_comparison: ImageComparison, known_failure: bool, - environment: &impl Environment, - has_renderer: bool, + render_interface: Option<&dyn RenderInterface>, ) -> Result<()> { use anyhow::Context; - if has_renderer { + if let Some(render_interface) = render_interface { let mut player_lock = player.lock().unwrap(); player_lock.render(); - let actual_image = environment.capture_renderer(player_lock.renderer_mut()); + let actual_image = render_interface.capture(player_lock.renderer_mut()); let expected_image_path = base_path.join(format!("{name}.expected.png")); if expected_image_path.is_file() { @@ -294,7 +291,7 @@ fn capture_and_compare_image( actual_image, expected_image, base_path, - environment.name(), + render_interface.name(), known_failure, )?; } else if !known_failure { diff --git a/tests/framework/src/test.rs b/tests/framework/src/test.rs index 6af391e1b..6d76a0cc5 100644 --- a/tests/framework/src/test.rs +++ b/tests/framework/src/test.rs @@ -4,6 +4,7 @@ use crate::runner::run_swf; use crate::set_logger; use anyhow::{anyhow, Context, Result}; use pretty_assertions::Comparison; +use ruffle_core::tag_utils::SwfMovie; use ruffle_core::Player; use ruffle_input_format::InputInjector; use ruffle_socket_format::SocketEvent; @@ -63,13 +64,22 @@ impl Test { } else { None }; + let movie = + SwfMovie::from_path(&self.swf_path, None).map_err(|e| anyhow!(e.to_string()))?; + let viewport_dimensions = self.options.player_options.viewport_dimensions(&movie); + let renderer = self + .options + .player_options + .create_renderer(environment, viewport_dimensions); let output = run_swf( self, + movie, injector, socket_events, before_start, before_end, - environment, + renderer, + viewport_dimensions, )?; self.compare_output(&output)?; Ok(()) diff --git a/tests/tests/environment.rs b/tests/tests/environment.rs index 7cf4a367a..e7b683d0d 100644 --- a/tests/tests/environment.rs +++ b/tests/tests/environment.rs @@ -1,33 +1,83 @@ -use image::RgbaImage; -use ruffle_test_framework::environment::{Environment, RenderBackend}; - -#[cfg(feature = "imgtests")] -use ruffle_test_framework::options::RenderOptions; - -#[cfg(feature = "imgtests")] -use ruffle_render_wgpu::{ - backend::{request_adapter_and_device, WgpuRenderBackend}, - descriptors::Descriptors, - target::TextureTarget, - wgpu, -}; - -#[cfg(feature = "imgtests")] -use {std::sync::Arc, std::sync::OnceLock}; +use ruffle_test_framework::environment::Environment; pub struct NativeEnvironment; -impl NativeEnvironment { +impl Environment for NativeEnvironment { #[cfg(feature = "imgtests")] - fn descriptors(&self) -> Option<&Arc> { - WGPU.get_or_init(build_wgpu_descriptors).as_ref() + fn is_render_supported( + &self, + requirements: &ruffle_test_framework::options::RenderOptions, + ) -> bool { + renderer::is_supported(requirements) + } + + #[cfg(feature = "imgtests")] + fn create_renderer( + &self, + width: u32, + height: u32, + ) -> Option<( + Box, + Box, + )> { + renderer::NativeRenderInterface::create_pair(width, height) } } -impl Environment for NativeEnvironment { - #[cfg(feature = "imgtests")] - fn is_render_supported(&self, requirements: &RenderOptions) -> bool { - if let Some(descriptors) = self.descriptors() { +#[cfg(feature = "imgtests")] +mod renderer { + use image::RgbaImage; + use ruffle_render_wgpu::backend::{request_adapter_and_device, WgpuRenderBackend}; + use ruffle_render_wgpu::descriptors::Descriptors; + use ruffle_render_wgpu::target::TextureTarget; + use ruffle_render_wgpu::wgpu; + use ruffle_test_framework::environment::{RenderBackend, RenderInterface}; + use ruffle_test_framework::options::RenderOptions; + use {std::sync::Arc, std::sync::OnceLock}; + + pub struct NativeRenderInterface; + + impl NativeRenderInterface { + pub fn create_pair( + width: u32, + height: u32, + ) -> Option<(Box, Box)> { + if let Some(descriptors) = descriptors() { + let target = TextureTarget::new(&descriptors.device, (width, height)).expect( + "WGPU Texture Target creation must not fail, everything was checked ahead of time", + ); + + Some( (Box::new(Self), Box::new( + WgpuRenderBackend::new(descriptors.clone(), target) + .expect("WGPU Render backend creation must not fail, everything was checked ahead of time"), + ))) + } else { + None + } + } + } + + impl RenderInterface for NativeRenderInterface { + fn name(&self) -> String { + if let Some(descriptors) = descriptors() { + let adapter_info = descriptors.adapter.get_info(); + format!("{}-{:?}", std::env::consts::OS, adapter_info.backend) + } else { + std::env::consts::OS.to_string() + } + } + + fn capture(&self, backend: &mut Box) -> RgbaImage { + let renderer = backend + .downcast_mut::>() + .unwrap(); + + renderer.capture_frame().expect("Failed to capture image") + } + } + + pub fn is_supported(requirements: &RenderOptions) -> bool { + if let Some(descriptors) = descriptors() { let adapter_info = descriptors.adapter.get_info(); let is_warp = cfg!(windows) && adapter_info.vendor == 5140 && adapter_info.device == 140; @@ -38,86 +88,42 @@ impl Environment for NativeEnvironment { } } - #[cfg(feature = "imgtests")] - fn create_renderer(&self, width: u32, height: u32) -> Option> { - if let Some(descriptors) = self.descriptors() { - let target = TextureTarget::new(&descriptors.device, (width, height)).expect( - "WGPU Texture Target creation must not fail, everything was checked ahead of time", - ); + static WGPU: OnceLock>> = OnceLock::new(); - Some(Box::new( - WgpuRenderBackend::new(descriptors.clone(), target) - .expect("WGPU Render backend creation must not fail, everything was checked ahead of time"), - )) + /* + It can be expensive to construct WGPU, much less Descriptors, so we put it off as long as we can + and share it across tests in the same process. + + Remember: + `cargo test` will run all tests in the same process. + `cargo nextest run` will create a different process per test. + + For `cargo test` it's relatively okay if we spend the time to construct descriptors once, + but for `cargo nextest run` it's a big cost per test if it's not going to use it. + */ + + fn create_wgpu_device() -> Option<(wgpu::Instance, wgpu::Adapter, wgpu::Device, wgpu::Queue)> { + let instance = wgpu::Instance::new(Default::default()); + futures::executor::block_on(request_adapter_and_device( + wgpu::Backends::all(), + &instance, + None, + Default::default(), + None, + )) + .ok() + .map(|(adapter, device, queue)| (instance, adapter, device, queue)) + } + + fn build_wgpu_descriptors() -> Option> { + if let Some((instance, adapter, device, queue)) = create_wgpu_device() { + Some(Arc::new(Descriptors::new(instance, adapter, device, queue))) } else { None } } - #[cfg(not(feature = "imgtests"))] - fn name(&self) -> String { - std::env::consts::OS.to_string() - } - - #[cfg(feature = "imgtests")] - fn name(&self) -> String { - if let Some(descriptors) = self.descriptors() { - let adapter_info = descriptors.adapter.get_info(); - format!("{}-{:?}", std::env::consts::OS, adapter_info.backend) - } else { - std::env::consts::OS.to_string() - } - } - - #[cfg(not(feature = "imgtests"))] - fn capture_renderer(&self, _backend: &mut Box) -> RgbaImage { - panic!("Cannot capture renderer as imgtests are not enabled") - } - - #[cfg(feature = "imgtests")] - fn capture_renderer(&self, backend: &mut Box) -> RgbaImage { - let renderer = backend - .downcast_mut::>() - .unwrap(); - - renderer.capture_frame().expect("Failed to capture image") - } -} - -#[cfg(feature = "imgtests")] -static WGPU: OnceLock>> = OnceLock::new(); - -/* - It can be expensive to construct WGPU, much less Descriptors, so we put it off as long as we can - and share it across tests in the same process. - - Remember: - `cargo test` will run all tests in the same process. - `cargo nextest run` will create a different process per test. - - For `cargo test` it's relatively okay if we spend the time to construct descriptors once, - but for `cargo nextest run` it's a big cost per test if it's not going to use it. -*/ - -#[cfg(feature = "imgtests")] -fn create_wgpu_device() -> Option<(wgpu::Instance, wgpu::Adapter, wgpu::Device, wgpu::Queue)> { - let instance = wgpu::Instance::new(Default::default()); - futures::executor::block_on(request_adapter_and_device( - wgpu::Backends::all(), - &instance, - None, - Default::default(), - None, - )) - .ok() - .map(|(adapter, device, queue)| (instance, adapter, device, queue)) -} - -#[cfg(feature = "imgtests")] -fn build_wgpu_descriptors() -> Option> { - if let Some((instance, adapter, device, queue)) = create_wgpu_device() { - Some(Arc::new(Descriptors::new(instance, adapter, device, queue))) - } else { - None + fn descriptors() -> Option<&'static Arc> { + WGPU.get_or_init(build_wgpu_descriptors).as_ref() } }