From 2c0588eaf6332c7fb8e41883f6aa94ca9f37415e Mon Sep 17 00:00:00 2001 From: Combiler01 Date: Mon, 26 Oct 2026 10:12:01 +0530 Subject: [PATCH] Self Reviewed Bug Fixes --- crates/edit/src/images.rs | 62 +++++++++++++++++--------- crates/edit/src/tests.rs | 70 ++++++++++++++++++++++++++++++ crates/edit/src/text.rs | 3 ++ crates/ui-egui/src/canvas.rs | 27 +++++++++--- crates/ui-egui/src/content_ui.rs | 19 +++++++- crates/ui-egui/src/edit_text_ui.rs | 11 +++-- crates/ui-egui/src/image_engine.rs | 15 +++++-- 7 files changed, 173 insertions(+), 34 deletions(-) diff --git a/crates/edit/src/images.rs b/crates/edit/src/images.rs index a12f48d3e9..cd47ac3875 100644 --- a/crates/edit/src/images.rs +++ b/crates/edit/src/images.rs @@ -556,10 +556,10 @@ pub(crate) fn is_entire_page_wrapped(ops: &[Op]) -> bool { if op.is("q") { depth = depth.saturating_add(1); } else if op.is("Q") { - depth = depth.saturating_sub(1); - if depth == 0 && i < ops.len().saturating_sub(1) { + if depth == 1 && i < ops.len().saturating_sub(1) { return false; } + depth = depth.saturating_sub(1); } } depth == 0 @@ -651,13 +651,18 @@ pub(crate) fn content_units(ops: &[Op], scan_start: usize, scan_end: usize) -> V j = j.saturating_add(1); found_end = true; break; - } else if j > i && matches!(next.op.as_slice(), b"q" | b"BT" | b"BI" | b"Do" | b"sh" | b"BDC" | b"BMC") { + } else if j > i + && matches!( + next.op.as_slice(), + b"q" | b"Q" | b"BT" | b"BI" | b"Do" | b"sh" | b"BDC" | b"BMC" | b"EMC" | b"Tj" | b"TJ" | b"'" | b"\"" + ) + { found_end = true; break; } j = j.saturating_add(1); } - let u_end = if found_end { j } else { scan_end }; + let u_end = if found_end { j } else { i.saturating_add(1) }; units.push(i..u_end); i = u_end; } @@ -684,38 +689,38 @@ pub(crate) struct ExtractedScopeState { } #[derive(Clone, Debug, Default, PartialEq)] -struct EffectiveGState { - ca: Option, - ca_stroke: Option, - bm: Option>, - smask: Option, +pub(crate) struct EffectiveGState { + pub(crate) ca: Option, + pub(crate) ca_stroke: Option, + pub(crate) bm: Option>, + pub(crate) smask: Option, } impl EffectiveGState { - fn effective_ca(&self) -> f64 { + pub(crate) fn effective_ca(&self) -> f64 { self.ca.unwrap_or(1.0) } - fn effective_ca_stroke(&self) -> f64 { + pub(crate) fn effective_ca_stroke(&self) -> f64 { self.ca_stroke.unwrap_or(1.0) } - fn effective_bm(&self) -> &[u8] { + pub(crate) fn effective_bm(&self) -> &[u8] { self.bm.as_deref().unwrap_or(b"Normal") } - fn effective_smask(&self) -> Option<&MaskIdentity> { + pub(crate) fn effective_smask(&self) -> Option<&MaskIdentity> { self.smask.as_ref() } - fn visually_differs(&self, other: &EffectiveGState) -> bool { + pub(crate) fn visually_differs(&self, other: &EffectiveGState) -> bool { (self.effective_ca() - other.effective_ca()).abs() > 1e-4 || (self.effective_ca_stroke() - other.effective_ca_stroke()).abs() > 1e-4 || self.effective_bm() != other.effective_bm() || self.effective_smask() != other.effective_smask() } - fn merge_dict(&mut self, dict: &Dict, doc: &Document, gs_name: Option<&[u8]>) { + pub(crate) fn merge_dict(&mut self, dict: &Dict, doc: &Document, gs_name: Option<&[u8]>) { if let Some(val) = dict.get(b"ca").map(|v| doc.resolve(v)).and_then(|v| v.as_f64()) { self.ca = Some(val); } @@ -749,7 +754,15 @@ impl EffectiveGState { } else if let Some(name) = gs_name { self.smask = Some(MaskIdentity::Named(name.to_vec())); } else { - self.smask = Some(MaskIdentity::Named(b"CustomMask".to_vec())); + use std::collections::hash_map::DefaultHasher; + use std::hash::{Hash, Hasher}; + let mut hasher = DefaultHasher::new(); + for (k, v) in mask_dict.iter() { + k.as_slice().hash(&mut hasher); + format!("{v:?}").hash(&mut hasher); + } + let id = format!("InlineMask_{:016x}", hasher.finish()); + self.smask = Some(MaskIdentity::Named(id.into_bytes())); } } else if let Some(r) = raw_smask.as_ref() { self.smask = Some(MaskIdentity::Ref(r.num, r.generation)); @@ -763,24 +776,31 @@ struct ExtendedGStateTracker { stack: Vec<(EffectiveGState, Vec<(Op, Matrix)>)>, current_state: EffectiveGState, active_gs_ops: Vec<(Op, Matrix)>, + overflow_depth: usize, } impl ExtendedGStateTracker { fn step(&mut self, op: &Op, ext_gstates: &Dict, doc: &Document, ctm: Matrix) -> Result<(), EditError> { match op.op.as_slice() { b"q" => { - if self.stack.len() >= 4096 { + if self.stack.len() >= 4096 || self.overflow_depth > 0 { + self.overflow_depth = self.overflow_depth.saturating_add(1); return Err(EditError::Invalid("the image graphics state is too deeply nested".into())); } self.stack.push((self.current_state.clone(), self.active_gs_ops.clone())); } b"Q" => { - if let Some((prev_state, prev_ops)) = self.stack.pop() { + if self.overflow_depth > 0 { + self.overflow_depth = self.overflow_depth.saturating_sub(1); + } else if let Some((prev_state, prev_ops)) = self.stack.pop() { self.current_state = prev_state; self.active_gs_ops = prev_ops; } } b"gs" => { + if self.overflow_depth > 0 { + return Ok(()); + } if let Some(name) = op.name(0) { if let Some(dict) = ext_gstates.get(name).map(|v| doc.resolve(v)).and_then(|v| v.as_dict().cloned()) { self.current_state.merge_dict(&dict, doc, Some(name)); @@ -1357,7 +1377,7 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran edits.push((dest_pos..dest_pos, serialized)); edits.push((target_span, removal_replacement)); } else { - return Err(EditError::Invalid("no change in image stacking order".into())); + return Err(EditError::Invalid("cannot arrange image into overlapping content range".into())); } let changed = pieces.splice(edits); @@ -1401,6 +1421,8 @@ fn image_exclusive_scope(ops: &[Op], target: usize) -> Result { @@ -1410,7 +1432,7 @@ fn image_exclusive_scope(ops: &[Op], target: usize) -> Result text = true, b"ET" => text = false, - b"m" | b"l" | b"c" | b"v" | b"y" | b"re" => path = true, + b"m" | b"l" | b"c" | b"v" | b"y" | b"re" | b"h" => path = true, b"W" | b"W*" => pending_clip = true, _ => {} } diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index fe2367f4e0..ad6c7e7fa6 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -8390,3 +8390,73 @@ fn a_run_of_spaces_in_a_subset_without_a_space_glyph_keeps_its_width() { let content = String::from_utf8_lossy(&page_content_bytes(&reopen(&doc), 0)).into_owned(); assert_eq!(content.matches("[-330] TJ").count(), 5, "{content}"); } + +#[test] +fn image_exclusive_scope_resets_path_on_scope_exit() { + // An earlier scope has a path operation closed by Q (with 'h' closepath). + // The subsequent image in its own scope must cleanly extract and allow crop removal. + let content = "q 0 0 m 10 10 l h Q q 100 100 200 100 re W n 200 0 0 100 100 100 cm /Im0 Do Q"; + let mut doc = image_page_with(content); + let before = images::page_images(&doc, 0).unwrap(); + assert_eq!(before.len(), 1); + images::remove_image_crop(&mut doc, 0, 0).unwrap(); +} + +#[test] +fn content_units_does_not_swallow_subsequent_units_on_standalone_gs() { + // /GS0 gs followed by Q and subsequent images must isolate each Do unit properly + let content = "q /GS0 gs Q q 200 0 0 100 100 100 cm /Im0 Do Q q 10 0 0 10 300 300 cm /Im0 Do Q"; + let pieces = pdfcraft_content::Pieces::join(&[content.as_bytes()]); + let ops = pieces.parse().ops; + let units = images::content_units(&ops, 0, ops.len()); + assert_eq!(units.len(), 2); +} + +#[test] +fn different_inline_soft_masks_have_distinct_identities() { + let doc = Document::new_empty(); + let mut dict1 = Dict::default(); + let mut mask1 = Dict::default(); + mask1.set(b"Type".to_vec(), Object::Name(b"Mask1".to_vec())); + dict1.set(b"SMask".to_vec(), Object::Dict(mask1)); + + let mut dict2 = Dict::default(); + let mut mask2 = Dict::default(); + mask2.set(b"Type".to_vec(), Object::Name(b"Mask2".to_vec())); + dict2.set(b"SMask".to_vec(), Object::Dict(mask2)); + + let mut state1 = images::EffectiveGState::default(); + state1.merge_dict(&dict1, &doc, None); + + let mut state2 = images::EffectiveGState::default(); + state2.merge_dict(&dict2, &doc, None); + + assert!(state1.visually_differs(&state2), "two different inline soft masks must visually differ"); +} + +#[test] +fn cid_to_gid_map_odd_length_is_trimmed_to_even() { + let mut m_bytes = vec![0u8, 1u8, 0u8, 2u8, 0u8]; + let needed_len = 5; + if m_bytes.len() > needed_len { + m_bytes.truncate(needed_len); + } + if m_bytes.len() % 2 != 0 { + m_bytes.pop(); + } + assert_eq!(m_bytes.len(), 4); + assert_eq!(m_bytes.len() % 2, 0); +} + +#[test] +fn is_entire_page_wrapped_with_nested_scopes() { + let wrapped = "q 1 0 0 1 0 0 cm q 0 0 10 10 re f Q 200 0 0 100 100 100 cm /Im0 Do Q"; + let pieces = pdfcraft_content::Pieces::join(&[wrapped.as_bytes()]); + let ops = pieces.parse().ops; + assert!(images::is_entire_page_wrapped(&ops), "page wrapped in outer q...Q with inner nested q...Q is wrapped"); + + let unwrapped = "q 1 0 0 1 0 0 cm Q q 200 0 0 100 100 100 cm /Im0 Do Q"; + let pieces2 = pdfcraft_content::Pieces::join(&[unwrapped.as_bytes()]); + let ops2 = pieces2.parse().ops; + assert!(!images::is_entire_page_wrapped(&ops2), "page with two separate top-level q...Q scopes is not wrapped"); +} diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index de2ef16cef..9d6bb88887 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -346,6 +346,9 @@ pub fn subset_document_fonts(doc: &mut Document) -> Result<(), EditError> { if m_bytes.len() > needed_len { m_bytes.truncate(needed_len); } + if m_bytes.len() % 2 != 0 { + m_bytes.pop(); + } for i in 0..m_bytes.len() / 2 { let s_idx = i.saturating_mul(2); if let Some(chunk) = m_bytes.get(s_idx..s_idx.saturating_add(2)) { diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 230872804f..59f524e8f0 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -99,16 +99,16 @@ pub fn image_selectable_bounds(im: &pdfcraft_engine::PageImage) -> [f64; 4] { /// the final selectable `egui::Rect` is the mathematical intersection of the image's /// full CTM-derived rect and the clipping path's rect. pub fn image_screen_rect(im: &pdfcraft_engine::PageImage, xf: &PageXform, info: &DocInfo, page: usize) -> Rect { - let full = xf.user_rect(info, page, im.rect.map(|v| v as f32)); + let full = xf.user_rect_f64(info, page, im.rect); if let Some(clip) = im.clip_rect { - let clip_r = xf.user_rect(info, page, clip.map(|v| v as f32)); + let clip_r = xf.user_rect_f64(info, page, clip); let inter = full.intersect(clip_r); if inter.is_positive() { return inter; } } if let Some(vr) = im.visible_rect { - let r = xf.user_rect(info, page, vr.map(|v| v as f32)); + let r = xf.user_rect_f64(info, page, vr); if r.is_positive() { return r; } @@ -2980,6 +2980,24 @@ impl PageXform { Rect::from_two_pos(self.norm_to_screen(a.0, a.1), self.norm_to_screen(b.0, b.1)) } + /// A PDF user-space rect with f64 precision (crop box, document `/Rotate`) -> screen rect. + pub fn user_rect_f64(&self, info: &DocInfo, page: usize, r: [f64; 4]) -> Rect { + let p = &info.pages[page]; + let [cx0, cy0, cx1, cy1] = p.crop.map(f64::from); + let (cw, ch) = ((cx1 - cx0).max(1.0), (cy1 - cy0).max(1.0)); + let norm = |x: f64, y: f64| -> (f64, f64) { + let (u, v) = ((x - cx0) / cw, (cy1 - y) / ch); + match p.rotation { + 90 => (1.0 - v, u), + 180 => (1.0 - u, 1.0 - v), + 270 => (v, 1.0 - u), + _ => (u, v), + } + }; + let (a, b) = (norm(r[0], r[1]), norm(r[2], r[3])); + Rect::from_two_pos(self.norm_to_screen(a.0 as f32, a.1 as f32), self.norm_to_screen(b.0 as f32, b.1 as f32)) + } + /// This transform resized to a raster of `px` device pixels (width, height before the view /// rotation), so each of its texels covers one screen pixel. The rectangle's corner must /// already be on a whole pixel ([`snap_to_pixels`]). @@ -5219,8 +5237,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { app.paste_image_at(page, at); } Some(PageMenuAction::RotateCCW(page)) => { - let _ = page; - app.views[index].rotate_view(false); + app.views[index].pending_edit = Some(pdfcraft_engine::Edit::RotatePages { pages: vec![page], degrees: -90 }); } Some(PageMenuAction::RotateCW(page)) => { let _ = page; diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index 70cbb49ba7..33ae6ce4f6 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -991,18 +991,33 @@ impl crate::PdfCraftApp { let target_rect = rect; - let before = self.session.get(id).map_or(0, |d| d.page_images(page).len()); if self.apply_edit(pdfcraft_engine::Edit::AddImage { page, rect: target_rect, name, bytes: std::sync::Arc::new(bytes) }) { self.last_copied_image_rect = None; let added_count = self.session.get(id).map_or(0, |d| d.added().iter().filter(|a| a.page == page).count()); let added_idx = added_count.saturating_sub(1); + let pasted_idx = self.session.get(id).map_or(0, |d| { + let images = d.page_images(page); + if let Some(target) = target_rect { + images + .iter() + .rposition(|im| { + (im.rect[0] - target[0]).abs() < 1e-2 + && (im.rect[1] - target[1]).abs() < 1e-2 + && (im.rect[2] - target[2]).abs() < 1e-2 + && (im.rect[3] - target[3]).abs() < 1e-2 + }) + .unwrap_or(images.len().saturating_sub(1)) + } else { + images.len().saturating_sub(1) + } + }); if let Some(v) = self.views.get_mut(i) { v.current = page; v.content.selected = Some((page, added_idx)); v.content.select_added = Some((page, added_idx)); v.image_selection = Some(crate::edit_text_ui::ImageSelection { page, - index: before, + index: pasted_idx, crop_mode: false, crop_rect: None, drag: None, diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index a147f97c0e..1a743241ae 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -300,7 +300,7 @@ pub fn toggle_numbered_list(text: &mut String) -> bool { && t[..dot_idx].chars().all(|c| c.is_ascii_digit()) && t[dot_idx..].starts_with(". ") { - new_text.push_str(&t[dot_idx + 2..]); + new_text.push_str(t.get(dot_idx + 2..).unwrap_or("")); continue; } new_text.push_str(line); @@ -735,6 +735,7 @@ impl LineEditor { self.look0 = self.look.clone(); self.extras0 = self.extras; self.original = self.text.clone(); + self.selection = None; self.focus = true; } @@ -1057,7 +1058,7 @@ pub(crate) fn quad_contains(quad: [Pos2; 4], p: Pos2) -> bool { } } } - true + sign != 0 } pub use crate::canvas::{ImageObb, PainterExt, project_delta}; @@ -1510,9 +1511,13 @@ pub(crate) fn image_input( info: &DocInfo, images: &[pdfcraft_engine::PageImage], view: &mut DocView, - _action: &mut Option, + action: &mut Option, allow_interaction: bool, ) -> bool { + // `action` is wired by the caller (canvas.rs) and forwarded to image_context_menu; it is not + // dispatched from within this function directly but must remain named (not prefixed with `_`) + // so that future in-function dispatch paths compile without a dead-wire rename. + let _ = &action; let boxes: Vec = images.iter().map(|im| crate::canvas::image_screen_rect(im, xf, info, page)).collect(); let painter = ui.painter(); let hover_pos = ptr.pos; diff --git a/crates/ui-egui/src/image_engine.rs b/crates/ui-egui/src/image_engine.rs index e04bd38652..3dd8d817b9 100644 --- a/crates/ui-egui/src/image_engine.rs +++ b/crates/ui-egui/src/image_engine.rs @@ -420,12 +420,12 @@ pub fn reset_crop(image_rect: Rect, uv_rect: Rect) -> (Rect, Rect) { let full_min_x = image_rect.min.x - uv_rect.min.x * px_per_uv_x; let full_min_y = image_rect.min.y - uv_rect.min.y * px_per_uv_y; - let full_w = px_per_uv_x; - let full_h = px_per_uv_y; + let full_uv = Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)); + let full_w = px_per_uv_x * full_uv.width(); + let full_h = px_per_uv_y * full_uv.height(); let new_rect = Rect::from_min_size(pos2(full_min_x, full_min_y), vec2(full_w, full_h)); - let new_uv = Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)); - (new_rect, new_uv) + (new_rect, full_uv) } // ============================================================================ @@ -776,5 +776,12 @@ mod tests { let _ = crate::edit_text_ui::quad_contains(quad, p); } } + + #[test] + fn degenerate_quad_does_not_contain_points() { + let zero_quad = [pos2(0.0, 0.0), pos2(0.0, 0.0), pos2(0.0, 0.0), pos2(0.0, 0.0)]; + assert!(!crate::edit_text_ui::quad_contains(zero_quad, pos2(10.0, 10.0))); + assert!(!crate::edit_text_ui::quad_contains(zero_quad, pos2(0.0, 0.0))); + } } }