From 8bd57468e892fbd9fe220e1b5ac6ceec0a17ce19 Mon Sep 17 00:00:00 2001 From: Combiler01 Date: Wed, 14 Oct 2026 22:36:44 +0530 Subject: [PATCH] Fixes As Requested [Some images not moving, Opacity Mistakes] --- crates/edit/src/images.rs | 110 ++++++++++++++++++------- crates/edit/src/tests.rs | 164 ++++++++++++++++++++++++++++++++++---- 2 files changed, 228 insertions(+), 46 deletions(-) diff --git a/crates/edit/src/images.rs b/crates/edit/src/images.rs index 9b73dddac9..311f17a4a2 100644 --- a/crates/edit/src/images.rs +++ b/crates/edit/src/images.rs @@ -543,10 +543,45 @@ struct ExtractedScopeState { state_ops: Vec, } +#[derive(Clone, Default)] +struct ExtendedGStateTracker { + stack: Vec>, + current_gs: Option, +} + +impl ExtendedGStateTracker { + fn step(&mut self, op: &Op) -> Result<(), EditError> { + match op.op.as_slice() { + b"q" => { + if self.stack.len() >= 256 { + return Err(EditError::Invalid("the image graphics state is too deeply nested".into())); + } + self.stack.push(self.current_gs.clone()); + } + b"Q" => { + if let Some(prev) = self.stack.pop() { + self.current_gs = prev; + } + } + b"gs" => { + self.current_gs = Some(op.clone()); + } + _ => {} + } + Ok(()) + } +} + fn extract_scope_state(ops: &[Op], start: usize, target: usize) -> Result, EditError> { - if start >= target || target >= ops.len() { + if target >= ops.len() || start > target { return Ok(None); } + if start == target { + return Ok(Some(ExtractedScopeState { + clip_ops: Vec::new(), + state_ops: Vec::new(), + })); + } let mut clip_ops = Vec::new(); let mut state_ops = Vec::new(); let mut pending_path: Vec = Vec::new(); @@ -725,55 +760,68 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran }; let mut state = ImageState::new(Matrix::IDENTITY, p.crop(doc), Vec::new()); + let mut tracker = ExtendedGStateTracker::default(); + let mut source_effective_gs = None; + let mut dest_effective_gs = None; + let mut dest_ctm = Matrix::IDENTITY; + + let max_scan = scan_target.max(target.saturating_add(1)); for (i, op) in ops.iter().enumerate() { + if i == target { + source_effective_gs = tracker.current_gs.clone(); + } if i == scan_target { + dest_effective_gs = tracker.current_gs.clone(); + dest_ctm = state.placement.matrix; + } + if i >= max_scan { break; } state.apply(op)?; + tracker.step(op)?; + } + if scan_target >= ops.len() && dest_effective_gs.is_none() { + dest_effective_gs = tracker.current_gs.clone(); + dest_ctm = state.placement.matrix; } - let dest_ctm = state.placement.matrix; let Some(dest_inv) = dest_ctm.invert() else { return Ok(()); }; - let target_span = if has_other_paint { - if let Ok(Some((sub_start, sub_end, _))) = image_exclusive_scope(&ops, target) - && sub_start >= target_unit.start - && sub_end < target_unit.end - { - let start_op = ops.get(sub_start).unwrap_or(&ops[target]); - let end_op = ops.get(sub_end).unwrap_or(&ops[target]); - start_op.span.start..end_op.span.end - } else { - let target_op = &ops[target]; - target_op.span.start..target_op.span.end - } + let (target_span, scope_start) = if let Ok(Some((sub_start, sub_end, _))) = image_exclusive_scope(&ops, target) + && sub_start >= target_unit.start + && sub_end < target_unit.end + { + let start_op = ops.get(sub_start).unwrap_or(&ops[target]); + let end_op = ops.get(sub_end).unwrap_or(&ops[target]); + (start_op.span.start..end_op.span.end, sub_start) + } else if has_other_paint { + let target_op = &ops[target]; + (target_op.span.start..target_op.span.end, target_unit.start) } else { let Some(first_op) = ops.get(target_unit.start) else { return Ok(()) }; let Some(last_op) = target_unit.end.checked_sub(1).and_then(|i| ops.get(i)) else { return Ok(()) }; - first_op.span.start..last_op.span.end - }; - - let scope_start = if has_other_paint { - if let Ok(Some((sub_start, sub_end, _))) = image_exclusive_scope(&ops, target) - && sub_start >= target_unit.start - && sub_end < target_unit.end - { - sub_start - } else { - target_unit.start - } - } else { - target_unit.start + (first_op.span.start..last_op.span.end, target_unit.start) }; let is_clipped = (img.visible_rect.is_some() && img.visible_rect != Some(img.rect)) || !img.clips.is_empty(); let extracted = extract_scope_state(&ops, scope_start, target)?; - let Some(extracted) = extracted else { - return Ok(()); + let Some(mut extracted) = extracted else { + return Err(EditError::Invalid("unsupported graphics state: unable to cleanly extract image scope state".into())); }; if is_clipped && extracted.clip_ops.is_empty() { - return Ok(()); + return Err(EditError::Invalid("unsupported graphics state: inherited clipping path cannot be cleanly preserved".into())); + } + + if source_effective_gs.is_none() && dest_effective_gs.is_some() { + return Err(EditError::Invalid("unsupported graphics state: moving an opaque image into an active transparency scope is not supported".into())); + } + + if let Some(src_gs) = source_effective_gs { + let already_has_gs = extracted.state_ops.iter().any(|o| o.is("gs")); + if !already_has_gs { + extracted.state_ops.push(src_gs); + } } let mut unit_ops = Vec::new(); diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 782cd3acd9..dae7d30411 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -1817,10 +1817,22 @@ fn image_arrange_preserves_position_under_active_ctm() { // Move first image to front: must not apply destination CTM twice! images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront).unwrap(); let doc = reopen(&doc); + + // 1. Position: check order of operators in the content stream + let bytes = page_content_bytes(&doc, 0); + let str_content = String::from_utf8_lossy(&bytes); + let first_do = str_content.find("/Im0 Do").unwrap(); + let last_do = str_content.rfind("/Im0 Do").unwrap(); + assert!(first_do < last_do, "Position: multiple image Do calls exist and order was updated"); + + // 2. Appearance: both images' positions and sizes are preserved let after = images::page_images(&doc, 0).unwrap(); assert_eq!(after.len(), 2); - assert_eq!(after[0].rect, [50.0, 50.0, 150.0, 150.0], "first image position preserved"); - assert_eq!(after[1].rect, [50.0, 50.0, 150.0, 150.0], "second image position preserved"); + assert_eq!(after[0].rect, [50.0, 50.0, 150.0, 150.0], "Appearance: first image position preserved"); + assert_eq!(after[1].rect, [50.0, 50.0, 150.0, 150.0], "Appearance: second image position preserved"); + + // 3. Preservation: outer CTM is preserved + assert!(str_content.contains("100 0 0 100 50 50 cm"), "Preservation: initial CTM preserved"); } #[test] @@ -1837,13 +1849,23 @@ fn image_arrange_preserves_shared_group_clipping_bounds() { // Bring image 0 to front: moves it past the external text while preserving clipping bounds images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront).unwrap(); let doc = reopen(&doc); - let after = images::page_images(&doc, 0).unwrap(); - assert_eq!(after.len(), 2, "both images preserved"); - assert_eq!(after[0].visible_rect, Some([120.0, 120.0, 160.0, 160.0]), "image 0 retains clip"); - assert_eq!(after[1].visible_rect, Some([120.0, 120.0, 160.0, 160.0]), "image 1 retains clip"); + + // 1. Position: moved image Do now follows External Text in AST let bytes = page_content_bytes(&doc, 0); let str_content = String::from_utf8_lossy(&bytes); - assert!(str_content.contains("External Text"), "external text preserved"); + let text_pos = str_content.find("External Text").unwrap(); + let moved_do_pos = str_content.rfind("/Im0 Do").unwrap(); + assert!(text_pos < moved_do_pos, "Position: moved image Do follows External Text"); + + // 2. Appearance: both images retain exact clipping bounds and geometry + let after = images::page_images(&doc, 0).unwrap(); + assert_eq!(after.len(), 2, "both images preserved"); + assert_eq!(after[0].visible_rect, Some([120.0, 120.0, 160.0, 160.0]), "Appearance: image 0 retains clip"); + assert_eq!(after[1].visible_rect, Some([120.0, 120.0, 160.0, 160.0]), "Appearance: image 1 retains clip"); + + // 3. Preservation: External Text and original group drawing preserved + assert!(str_content.contains("External Text"), "Preservation: external text preserved"); + assert!(str_content.contains("120 120 40 40 re W n"), "Preservation: clipping path operators preserved"); } #[test] @@ -1856,16 +1878,24 @@ fn image_arrange_preserves_extended_graphics_state_opacity() { images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront).unwrap(); let doc = reopen(&doc); - let after = images::page_images(&doc, 0).unwrap(); - assert_eq!(after.len(), 1, "image preserved"); - assert_eq!(after[0].rect, [50.0, 50.0, 150.0, 150.0]); + // 1. Position: /Im0 Do moved past External Text let bytes = page_content_bytes(&doc, 0); let str_content = String::from_utf8_lossy(&bytes); - assert!(str_content.contains("/GS0 gs"), "extended graphics state /GS0 gs is retained"); - let gs_pos = str_content.rfind("/GS0 gs").unwrap(); + let text_pos = str_content.find("External Text").unwrap(); let do_pos = str_content.rfind("/Im0 Do").unwrap(); - assert!(gs_pos < do_pos, "/GS0 gs must precede /Im0 Do in the new unit"); + assert!(text_pos < do_pos, "Position: /Im0 Do moved past External Text"); + + // 2. Appearance: /GS0 gs is retained and directly precedes /Im0 Do + let gs_pos = str_content.rfind("/GS0 gs").unwrap(); + assert!(text_pos < gs_pos, "Appearance: /GS0 gs is in the new moved unit"); + assert!(gs_pos < do_pos, "Appearance: /GS0 gs directly precedes /Im0 Do"); + let after = images::page_images(&doc, 0).unwrap(); + assert_eq!(after.len(), 1, "image preserved"); + assert_eq!(after[0].rect, [50.0, 50.0, 150.0, 150.0], "Appearance: rect preserved"); + + // 3. Preservation: External Text is preserved + assert!(str_content.contains("BT /F1 12 Tf 72 700 Td (External Text) Tj ET"), "Preservation: text preserved"); } #[test] @@ -1877,13 +1907,117 @@ fn image_arrange_preserves_exact_non_rectangular_clipping_path() { images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront).unwrap(); let doc = reopen(&doc); + + // 1. Position: moved image follows External Text in AST + let bytes = page_content_bytes(&doc, 0); + let str_content = String::from_utf8_lossy(&bytes); + let text_pos = str_content.find("External Text").unwrap(); + let last_do = str_content.rfind("/Im0 Do").unwrap(); + assert!(text_pos < last_do, "Position: moved image follows External Text"); + + // 2. Appearance: polygon geometry and even-odd rule are preserved + assert!(str_content.contains("W*"), "Appearance: even-odd W* fill rule preserved"); + assert!(str_content.contains("120 120 m 160 120 l 140 160 l h W* n"), "Appearance: exact polygon clip preserved"); let after = images::page_images(&doc, 0).unwrap(); assert_eq!(after.len(), 2, "both images preserved"); + // 3. Preservation: External Text and remaining original group content preserved + assert!(str_content.contains("External Text"), "Preservation: external text preserved"); +} + +#[test] +fn unwrapped_image_moves_successfully() { + let content = "100 0 0 100 50 50 cm /Im0 Do BT /F1 12 Tf 72 700 Td (Middle Text) Tj ET"; + let mut doc = image_page_with(content); + let initial = images::page_images(&doc, 0).unwrap(); + assert_eq!(initial.len(), 1); + assert_eq!(initial[0].rect, [50.0, 50.0, 150.0, 150.0]); + + // Initial order: /Im0 Do precedes Middle Text + let initial_bytes = page_content_bytes(&doc, 0); + let initial_str = String::from_utf8_lossy(&initial_bytes); + let init_do = initial_str.find("/Im0 Do").unwrap(); + let init_text = initial_str.find("Middle Text").unwrap(); + assert!(init_do < init_text, "/Im0 Do initially precedes Middle Text"); + + // Move unwrapped image to front (past Middle Text) + images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront).unwrap(); + let doc = reopen(&doc); + + // 1. Position: /Im0 Do now follows Middle Text in the content stream AST let bytes = page_content_bytes(&doc, 0); let str_content = String::from_utf8_lossy(&bytes); - assert!(str_content.contains("W*"), "even-odd W* fill rule must be preserved"); - assert!(str_content.contains("120 120 m 160 120 l 140 160 l h W* n"), "exact non-rectangular clip path preserved"); + let after_text = str_content.find("Middle Text").unwrap(); + let after_do = str_content.rfind("/Im0 Do").unwrap(); + assert!(after_text < after_do, "Position: /Im0 Do actually moved after Middle Text"); + + // 2. Appearance: image rect and visibility are unchanged + let after = images::page_images(&doc, 0).unwrap(); + assert_eq!(after.len(), 1); + assert_eq!(after[0].rect, [50.0, 50.0, 150.0, 150.0], "Appearance: image coordinates preserved"); + assert_eq!(after[0].visible_rect, Some([50.0, 50.0, 150.0, 150.0])); + + // 3. Preservation: Middle Text and CTM remain intact + assert!(str_content.contains("BT /F1 12 Tf 72 700 Td (Middle Text) Tj ET"), "Preservation: text operation preserved"); + assert!(str_content.contains("100 0 0 100 50 50 cm"), "Preservation: original cm preserved"); +} + +#[test] +fn inherited_source_opacity_is_preserved() { + // Outer q sets /GS0 gs; inner q draws /Im0 Do; outer Q restores state before External Text + let content = "q /GS0 gs q 100 0 0 100 50 50 cm /Im0 Do Q Q BT /F1 12 Tf 72 700 Td (External Text) Tj ET"; + let mut doc = image_page_with(content); + let initial = images::page_images(&doc, 0).unwrap(); + assert_eq!(initial.len(), 1); + assert_eq!(initial[0].rect, [50.0, 50.0, 150.0, 150.0]); + + // Initial order: /Im0 Do precedes External Text + let initial_bytes = page_content_bytes(&doc, 0); + let initial_str = String::from_utf8_lossy(&initial_bytes); + let init_do = initial_str.find("/Im0 Do").unwrap(); + let init_text = initial_str.find("External Text").unwrap(); + assert!(init_do < init_text, "/Im0 Do initially precedes External Text"); + + // Move image to front (past External Text, outside its original outer q...Q scope) + images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront).unwrap(); + let doc = reopen(&doc); + + // 1. Position: /Im0 Do now follows External Text in the AST + let bytes = page_content_bytes(&doc, 0); + let str_content = String::from_utf8_lossy(&bytes); + let after_text = str_content.find("External Text").unwrap(); + let after_do = str_content.rfind("/Im0 Do").unwrap(); + assert!(after_text < after_do, "Position: /Im0 Do moved past External Text"); + + // 2. Appearance: effective appearance state (/GS0 gs) is injected and precedes /Im0 Do in the new unit + let after_gs = str_content.rfind("/GS0 gs").unwrap(); + assert!(after_text < after_gs, "Appearance: /GS0 gs injected into the new unit at destination"); + assert!(after_gs < after_do, "Appearance: /GS0 gs directly precedes /Im0 Do in the moved unit"); + let after = images::page_images(&doc, 0).unwrap(); + assert_eq!(after.len(), 1); + assert_eq!(after[0].rect, [50.0, 50.0, 150.0, 150.0], "Appearance: rect preserved"); + + // 3. Preservation: External Text and original outer scope structure are preserved + assert!(str_content.contains("BT /F1 12 Tf 72 700 Td (External Text) Tj ET"), "Preservation: external text preserved"); +} + +#[test] +fn destination_opacity_pollution_is_prevented() { + // /Im0 Do is fully opaque at the start; destination has active top-level /GS_translucent gs + let content = "100 0 0 100 50 50 cm /Im0 Do /GS_translucent gs BT /F1 12 Tf 72 700 Td (Translucent Text) Tj ET"; + let mut doc = image_page_with(content); + let initial_bytes = page_content_bytes(&doc, 0); + + // Attempting to move opaque image 0 to front into active transparency scope must safely bail out with Err + let res = images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront); + assert!(res.is_err(), "must return an explicit Err rather than silently corrupting opacity"); + let err_msg = res.unwrap_err().to_string(); + assert!(err_msg.contains("unsupported graphics state"), "error describes unsupported graphics state: {err_msg}"); + + // Preservation: the document contents are completely unmodified + let doc = reopen(&doc); + let after_bytes = page_content_bytes(&doc, 0); + assert_eq!(initial_bytes, after_bytes, "Preservation: content stream remained completely unmodified"); }