diff --git a/src/cli/render.rs b/src/cli/render.rs index f0a9f72..5c0940f 100644 --- a/src/cli/render.rs +++ b/src/cli/render.rs @@ -466,55 +466,20 @@ pub fn run_render( // Render the resolved sprite let (mut image, render_warnings) = render_resolved(&render_sprite_data); - // Apply transforms from sprite.transform if present - if let Some(ref transform_specs) = sprite.transform { - use crate::models::TransformSpec; - use crate::transforms::{apply_image_transform, parse_transform_str}; - - for spec in transform_specs { - let transform_result = match spec { - TransformSpec::String(s) => parse_transform_str(s), - TransformSpec::Object { op, params } => { - // Convert object to JSON and parse - let mut obj = serde_json::Map::new(); - obj.insert("op".to_string(), serde_json::Value::String(op.clone())); - for (k, v) in params { - obj.insert(k.clone(), v.clone()); - } - crate::transforms::parse_transform_value(&serde_json::Value::Object( - obj, - )) - } - }; - - match transform_result { - Ok(transform) => { - // Skip animation transforms (they don't apply to images) - if crate::transforms::is_animation_transform(&transform) { - continue; - } - match apply_image_transform(&image, &transform, Some(&final_palette)) { - Ok(transformed) => image = transformed, - Err(e) => { - let msg = - format!("sprite '{}': transform error: {}", sprite.name, e); - if strict { - eprintln!("Error: {}", msg); - return ExitCode::from(EXIT_ERROR); - } - all_warnings.push(msg); - } - } - } - Err(e) => { - let msg = format!("sprite '{}': invalid transform: {}", sprite.name, e); - if strict { - eprintln!("Error: {}", msg); - return ExitCode::from(EXIT_ERROR); - } - all_warnings.push(msg); - } + if let Some(ref specs) = sprite.transform { + let (transformed, warnings) = crate::transforms::apply_image_transform_specs( + image, + specs, + Some(&final_palette), + ); + image = transformed; + for warning in warnings { + let msg = format!("sprite '{}': {}", sprite.name, warning); + if strict { + eprintln!("Error: {}", msg); + return ExitCode::from(EXIT_ERROR); } + all_warnings.push(msg); } } @@ -890,7 +855,7 @@ fn run_composition_render( } /// Render a composition to an image buffer -/// TRF-9: Now uses SpriteRegistry to resolve sprites with transforms applied +/// Resolve sprite geometry and apply image transforms before caching composition inputs. #[allow(clippy::too_many_arguments)] fn render_composition_to_image( comp: &Composition, @@ -924,7 +889,7 @@ fn render_composition_to_image( // First check if sprite exists in the raw sprites map (for @include: handling) let original_sprite = sprites.get(sprite_name); - // Use sprite registry to resolve the sprite with transforms applied + // Resolve inherited sprite geometry; image transforms are applied below. let resolved_sprite = match sprite_registry.resolve(sprite_name, palette_registry, strict) { Ok(resolved) => { // Collect any sprite warnings @@ -993,8 +958,19 @@ fn render_composition_to_image( regions: resolved_sprite.regions.clone(), }; - // Render the resolved sprite (transforms already applied) - let (image, render_warnings) = render_resolved(&render_sprite_data); + // Registry resolution inherits regions and size, not transformed pixels. + let (mut image, render_warnings) = render_resolved(&render_sprite_data); + if let Some(specs) = original_sprite.and_then(|sprite| sprite.transform.as_deref()) { + let (transformed, warnings) = crate::transforms::apply_image_transform_specs( + image, + specs, + Some(&render_sprite_data.palette), + ); + image = transformed; + for warning in warnings { + all_warnings.push(format!("sprite '{}': {}", sprite_name, warning)); + } + } // Collect render warnings for warning in render_warnings { diff --git a/src/composition/error.rs b/src/composition/error.rs index 5fc3dc2..762a946 100644 --- a/src/composition/error.rs +++ b/src/composition/error.rs @@ -18,6 +18,9 @@ impl Warning { #[derive(Debug, Clone, PartialEq, Error)] #[non_exhaustive] pub enum CompositionError { + /// A layer transform could not be parsed or applied in strict mode. + #[error("Layer transform in composition '{composition_name}': {message}")] + LayerTransform { composition_name: String, message: String }, /// Sprite dimensions exceed cell size #[error("Sprite '{sprite_name}' ({sprite_w}x{sprite_h}) exceeds cell size ({cell_w}x{cell_h}) in composition '{composition_name}'", sprite_w = sprite_size.0, sprite_h = sprite_size.1, cell_w = cell_size.0, cell_h = cell_size.1)] SizeMismatch { diff --git a/src/composition/render.rs b/src/composition/render.rs index f82223b..1185599 100644 --- a/src/composition/render.rs +++ b/src/composition/render.rs @@ -3,7 +3,7 @@ use image::{Rgba, RgbaImage}; use std::collections::HashMap; -use crate::models::Composition; +use crate::models::{Composition, CompositionLayer}; use crate::registry::CompositionRegistry; use crate::variables::VariableRegistry; @@ -12,6 +12,34 @@ use super::context::RenderContext; use super::error::{CompositionError, Warning}; use super::resolve::{resolve_blend_mode, resolve_opacity}; +/// Transform a layer's input before placement, without mutating cached sprites +/// or nested compositions reused by other layers. +fn transform_layer_sprite<'a>( + image: &'a RgbaImage, + layer: &CompositionLayer, + composition_name: &str, + strict: bool, + warnings: &mut Vec, +) -> Result, CompositionError> { + let Some(specs) = layer.transform.as_deref().filter(|specs| !specs.is_empty()) else { + return Ok(std::borrow::Cow::Borrowed(image)); + }; + let (image, diagnostics) = + crate::transforms::apply_image_transform_specs(image.clone(), specs, None); + for diagnostic in diagnostics { + let message = + format!("layer '{}': {}", layer.name.as_deref().unwrap_or("unnamed"), diagnostic); + if strict { + return Err(CompositionError::LayerTransform { + composition_name: composition_name.to_string(), + message, + }); + } + warnings.push(Warning::new(message)); + } + Ok(std::borrow::Cow::Owned(image)) +} + /// Render a composition to an RGBA image buffer. /// /// Takes a composition and a map of sprite name -> rendered image. @@ -228,7 +256,15 @@ pub fn render_composition( } }; - // Check for size mismatch (Task 2.5) + let sprite_image = transform_layer_sprite( + sprite_image, + layer, + &comp.name, + strict, + &mut warnings, + )?; + + // Check the transformed dimensions for size mismatch (Task 2.5) let sprite_width = sprite_image.width(); let sprite_height = sprite_image.height(); if sprite_width > cell_size[0] || sprite_height > cell_size[1] { @@ -252,7 +288,7 @@ pub fn render_composition( let y = (row_idx as u32) * cell_size[1]; // Blit sprite onto canvas with blend mode and opacity (ATF-10) - blit_sprite_blended(&mut canvas, sprite_image, x, y, blend_mode, opacity); + blit_sprite_blended(&mut canvas, &sprite_image, x, y, blend_mode, opacity); } } } @@ -437,6 +473,14 @@ fn render_composition_inner( continue; }; + let sprite_image = transform_layer_sprite( + &sprite_image, + layer, + &comp.name, + strict, + &mut warnings, + )?; + let x = (col_idx as u32) * cell_size[0]; let y = (row_idx as u32) * cell_size[1]; diff --git a/src/transforms/mod.rs b/src/transforms/mod.rs index c6c99db..e1d0b7d 100644 --- a/src/transforms/mod.rs +++ b/src/transforms/mod.rs @@ -37,3 +37,35 @@ pub use types::{explain_transform, Transform, TransformError}; /// Result type alias for transform operations. pub type Result = std::result::Result; + +/// Apply image transform specifications in order, retaining valid operations in +/// lenient rendering. Callers turn the returned diagnostics into strict errors. +pub(crate) fn apply_image_transform_specs( + mut image: image::RgbaImage, + specs: &[crate::models::TransformSpec], + palette: Option<&std::collections::HashMap>, +) -> (image::RgbaImage, Vec) { + use crate::models::TransformSpec; + + let mut warnings = Vec::new(); + for spec in specs { + let parsed = match spec { + TransformSpec::String(s) => parse_transform_str(s), + TransformSpec::Object { op, params } => { + let mut object: serde_json::Map = + params.clone().into_iter().collect(); + object.insert("op".to_string(), serde_json::Value::String(op.clone())); + parse_transform_value(&serde_json::Value::Object(object)) + } + }; + match parsed { + Ok(transform) if is_animation_transform(&transform) => continue, + Ok(transform) => match apply_image_transform(&image, &transform, palette) { + Ok(transformed) => image = transformed, + Err(error) => warnings.push(format!("transform error: {}", error)), + }, + Err(error) => warnings.push(format!("invalid transform: {}", error)), + } + } + (image, warnings) +} diff --git a/tests/cli_composition_transforms.rs b/tests/cli_composition_transforms.rs new file mode 100644 index 0000000..03b6654 --- /dev/null +++ b/tests/cli_composition_transforms.rs @@ -0,0 +1,59 @@ +//! Composition transforms must agree with direct sprite rendering in PNG and GIF. + +use image::AnimationDecoder; +use std::fs::File; +use std::io::BufReader; +use std::path::Path; +use std::process::Command; + +const INPUT: &str = "tests/fixtures/valid/composition_transforms.pxl"; + +fn render(output: &Path, options: &[&str]) { + let result = Command::new(env!("CARGO_BIN_EXE_pxl")) + .args(["render", INPUT, "--strict"]) + .args(options) + .arg("-o") + .arg(output) + .output() + .unwrap(); + assert!(result.status.success(), "{}", String::from_utf8_lossy(&result.stderr)); +} + +fn expected(x: u32, y: u32) -> image::RgbaImage { + let mut image = image::RgbaImage::new(8, 8); + for dy in 0..2 { + for dx in 0..3 { + image.put_pixel(x + dx, y + dy, image::Rgba([227, 218, 201, 255])); + } + } + image +} + +#[test] +fn png_compositions_match_direct_transformed_sprites() { + let dir = tempfile::tempdir().unwrap(); + render(dir.path(), &[]); + let shifted = image::open(dir.path().join("jaw_shift.png")).unwrap().to_rgba8(); + assert_eq!(shifted, expected(2, 4)); + for name in ["layer_shift", "source_shift", "base_shift"] { + let actual = image::open(dir.path().join(format!("{name}.png"))).unwrap().to_rgba8(); + assert_eq!(actual, shifted, "{name}"); + } + let combined = image::open(dir.path().join("combined.png")).unwrap().to_rgba8(); + assert_eq!(combined, expected(3, 4)); + assert_eq!(combined, image::open(dir.path().join("jaw_combined.png")).unwrap().to_rgba8()); +} + +#[test] +fn gif_composition_frames_apply_source_then_layer_transforms() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("poses.gif"); + render(&path, &["--gif", "--animation", "poses"]); + let decoder = + image::codecs::gif::GifDecoder::new(BufReader::new(File::open(path).unwrap())).unwrap(); + let frames = decoder.into_frames().collect_frames().unwrap(); + assert_eq!(frames.len(), 3); + for (frame, x) in frames.iter().zip([2, 2, 3]) { + assert_eq!(frame.buffer(), &expected(x, 4)); + } +} diff --git a/tests/composition_layer_transforms.rs b/tests/composition_layer_transforms.rs new file mode 100644 index 0000000..aef3b9b --- /dev/null +++ b/tests/composition_layer_transforms.rs @@ -0,0 +1,111 @@ +//! Layer transforms apply before placement and never alter shared cached inputs. + +use image::{Rgba, RgbaImage}; +use pixelsrc::composition::{render_composition, render_composition_nested, RenderContext}; +use pixelsrc::models::Composition; +use pixelsrc::registry::CompositionRegistry; +use serde_json::json; +use std::collections::HashMap; + +fn sprites() -> HashMap { + let mut image = RgbaImage::new(4, 1); + image.put_pixel(0, 0, Rgba([255, 0, 0, 255])); + HashMap::from([("dot".into(), image)]) +} + +#[test] +fn transforms_run_in_order_before_each_cell_placement() { + let comp: Composition = serde_json::from_value(json!({ + "name": "pair", "size": [8, 1], "cell_size": [4, 1], + "sprites": {"A": "dot"}, + "layers": [{"map": ["AA"], "transform": ["shift:1,0", "mirror-h"]}] + })) + .unwrap(); + let sprites = sprites(); + let (image, warnings) = render_composition(&comp, &sprites, true, None).unwrap(); + assert!(warnings.is_empty()); + for x in 0..8 { + assert_eq!(image.get_pixel(x, 0)[3] > 0, x == 2 || x == 6); + } + assert_eq!(sprites["dot"].get_pixel(0, 0)[3], 255); +} + +#[test] +fn nested_layer_transforms_leave_cached_compositions_unchanged() { + let inner: Composition = serde_json::from_value(json!({ + "name": "inner", "size": [4, 1], "cell_size": [4, 1], + "sprites": {"A": "dot"}, "layers": [{"map": ["A"]}] + })) + .unwrap(); + let outer: Composition = serde_json::from_value(json!({ + "name": "outer", "size": [8, 1], "cell_size": [4, 1], + "sprites": {"N": "inner"}, + "layers": [ + {"map": ["NN"], "transform": ["shift:1,0", "mirror-h"]}, + {"map": ["N"]} + ] + })) + .unwrap(); + let mut registry = CompositionRegistry::new(); + registry.register(inner); + let sprites = sprites(); + let mut context = RenderContext::new(); + let (image, warnings) = + render_composition_nested(&outer, &sprites, Some(®istry), &mut context, true, None) + .unwrap(); + assert!(warnings.is_empty()); + for x in 0..8 { + assert_eq!(image.get_pixel(x, 0)[3] > 0, [0, 2, 6].contains(&x)); + } + assert_eq!(context.get_cached("inner").unwrap(), &sprites["dot"]); +} + +#[test] +fn invalid_layer_transforms_warn_or_fail_in_both_renderers() { + let comp: Composition = serde_json::from_value(json!({ + "name": "bad", "size": [4, 1], "cell_size": [4, 1], + "sprites": {"A": "dot"}, + "layers": [{"name": "pose", "map": ["A"], + "transform": ["unknown-operation", {"op": "shift", "x": 1, "y": 0}]}] + })) + .unwrap(); + for nested in [false, true] { + for strict in [false, true] { + let sprites = sprites(); + let result = if nested { + render_composition_nested( + &comp, + &sprites, + None, + &mut RenderContext::new(), + strict, + None, + ) + } else { + render_composition(&comp, &sprites, strict, None) + }; + if strict { + let error = result.unwrap_err().to_string(); + assert!(error.contains("pose") && error.contains("invalid transform")); + } else { + let (image, warnings) = result.unwrap(); + assert_eq!(warnings.len(), 1); + assert!(warnings[0].message.contains("invalid transform")); + assert_eq!(image.get_pixel(0, 0)[3], 0); + assert_eq!(image.get_pixel(1, 0)[3], 255); + } + } + } +} + +#[test] +fn transformed_sprite_dimensions_are_validated() { + let comp: Composition = serde_json::from_value(json!({ + "name": "large", "size": [4, 1], "cell_size": [4, 1], + "sprites": {"A": "dot"}, + "layers": [{"map": ["A"], "transform": ["pad:1"]}] + })) + .unwrap(); + let error = render_composition(&comp, &sprites(), true, None).unwrap_err(); + assert!(error.to_string().contains("exceeds cell size")); +} diff --git a/tests/fixtures/valid/composition_transforms.pxl b/tests/fixtures/valid/composition_transforms.pxl new file mode 100644 index 0000000..c4f776f --- /dev/null +++ b/tests/fixtures/valid/composition_transforms.pxl @@ -0,0 +1,9 @@ +{"type":"palette","name":"p","colors":{"_":"transparent","bone":"#e3dac9"}} +{"type":"sprite","name":"jaw","size":[8,8],"palette":"p","regions":{"bone":{"rect":[2,1,3,2]}}} +{"type":"sprite","name":"jaw_shift","source":"jaw","palette":"p","transform":["shift:0,3"]} +{"type":"sprite","name":"jaw_combined","source":"jaw","palette":"p","transform":["shift:0,3",{"op":"shift","x":1,"y":0}]} +{"type":"composition","name":"layer_shift","size":[8,8],"cell_size":[8,8],"sprites":{"J":"jaw"},"layers":[{"map":["J"],"transform":["shift:0,3"]}]} +{"type":"composition","name":"source_shift","size":[8,8],"cell_size":[8,8],"sprites":{"J":"jaw_shift"},"layers":[{"map":["J"]}]} +{"type":"composition","name":"combined","size":[8,8],"cell_size":[8,8],"sprites":{"J":"jaw_shift"},"layers":[{"map":["J"],"transform":[{"op":"shift","x":1,"y":0}]}]} +{"type":"composition","name":"base_shift","base":"jaw_shift","layers":[],"sprites":{}} +{"type":"animation","name":"poses","frames":["layer_shift","source_shift","combined"],"duration":100}