Self Reviewed Bug Fixes

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<f64>,
-    ca_stroke: Option<f64>,
-    bm: Option<Vec<u8>>,
-    smask: Option<MaskIdentity>,
+pub(crate) struct EffectiveGState {
+    pub(crate) ca: Option<f64>,
+    pub(crate) ca_stroke: Option<f64>,
+    pub(crate) bm: Option<Vec<u8>>,
+    pub(crate) smask: Option<MaskIdentity>,
 }
 
 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<Option<(usize, usi
                     }
                     matrix = scope.matrix;
                     clipped = scope.inherited_clip;
+                    path = scope.inherited_path;
+                    pending_clip = false;
                 }
             }
             b"cm" => {
@@ -1410,7 +1432,7 @@ fn image_exclusive_scope(ops: &[Op], target: usize) -> Result<Option<(usize, usi
             }
             b"BT" => 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,
             _ => {}
         }

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");
+}

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)) {

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;

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,

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<ImageAction>,
+    action: &mut Option<ImageAction>,
     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<Rect> = images.iter().map(|im| crate::canvas::image_screen_rect(im, xf, info, page)).collect();
     let painter = ui.painter();
     let hover_pos = ptr.pos;

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)));
+        }
     }
 }