From 85a4d3573310d9e7ea4f4b6146a4aa9bb79dfa8e Mon Sep 17 00:00:00 2001 From: Combiler01 Date: Tue, 27 Oct 2026 08:46:08 +0530 Subject: [PATCH] Review Fixes --- crates/automation/tests/it/automation.rs | 18 +- crates/edit/src/images.rs | 72 ++--- crates/edit/src/tests.rs | 55 ++++ crates/edit/src/text.rs | 271 +----------------- crates/engine/src/edit.rs | 5 +- crates/engine/src/session.rs | 7 +- crates/engine/src/text_fonts.rs | 4 +- crates/engine/tests/it/document_fonts.rs | 47 --- crates/ui-egui/src/canvas.rs | 15 +- crates/ui-egui/src/content_ui.rs | 72 +++-- crates/ui-egui/src/create_ui.rs | 20 +- crates/ui-egui/src/edit_text_ui.rs | 32 ++- crates/ui-egui/src/editing.rs | 18 +- crates/ui-egui/src/format_text_ui.rs | 3 +- crates/ui-egui/tests/it/editing.rs | 94 +++++- crates/ui-egui/tests/it/main.rs | 1 + crates/ui-egui/tests/{ => it}/pinned_tools.rs | 0 parity/acrobat-features.toml | 22 +- 18 files changed, 307 insertions(+), 449 deletions(-) rename crates/ui-egui/tests/{ => it}/pinned_tools.rs (100%) diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index a7ef13a1eb..3d464fa420 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -632,17 +632,7 @@ fn explicit_font_and_disjoint_ranges_save_reopen_through_tools() { ok(&mut a, "doc_save", json!({"doc":id,"path":"edited.pdf"})); let reopened = ok(&mut a, "doc_open", json!({"path":"edited.pdf"}))["doc"].as_u64().unwrap(); let saved = ok(&mut a, "text_lines", json!({"doc":reopened,"page":1})); - let mut normalized_saved = saved.clone(); - if let Some(arr) = normalized_saved["lines"].as_array_mut() { - for l in arr { - if let Some(font) = l["font"].as_str() - && let Some((_, base)) = font.split_once('+') - { - l["font"] = json!(base); - } - } - } - assert_eq!(lines, normalized_saved); + assert_eq!(lines, saved); assert_eq!(rendered, a.call("page_render", &json!({"doc":reopened,"page":1,"dpi":72})).unwrap()); let before = saved.clone(); assert!(a.call("text_edit", &json!({"doc":reopened,"page":1,"paragraph":1,"text":"é test","ranges":[{"range":[1,2],"size":18}]})).is_err()); @@ -3352,6 +3342,12 @@ fn arranging_page_images_through_tools() { assert_eq!(r["undo"], "Bring to front"); let r = ok(&mut a, "image_edit", json!({ "doc": made, "page": 1, "image": 2, "action": "send_to_back" })); assert_eq!(r["undo"], "Send to back"); + ok(&mut a, "doc_save", json!({ "doc": made, "path": "arranged.pdf" })); + let reopened = ok(&mut a, "doc_open", json!({ "path": "arranged.pdf" }))["doc"].as_u64().unwrap(); + let after_reopen = ok(&mut a, "page_images", json!({ "doc": reopened, "page": 1 })); + assert_eq!(after_reopen["count"], 2); + assert_eq!(after_reopen["images"][0]["rect"], json!([0.0, 0.0, 50.0, 75.0])); + assert_eq!(after_reopen["images"][1]["rect"], json!([50.0, 50.0, 100.0, 100.0])); } #[test] diff --git a/crates/edit/src/images.rs b/crates/edit/src/images.rs index 9d2d3fe518..3272663c3c 100644 --- a/crates/edit/src/images.rs +++ b/crates/edit/src/images.rs @@ -662,7 +662,7 @@ pub(crate) fn content_units(ops: &[Op], scan_start: usize, scan_end: usize) -> V } j = j.saturating_add(1); } - let u_end = if found_end { j } else { i.saturating_add(1) }; + let u_end = if found_end { j } else { scan_end.max(i.saturating_add(1)) }; units.push(i..u_end); i = u_end; } @@ -771,11 +771,16 @@ impl EffectiveGState { } } +const MAX_ACTIVE_GS_OPS: usize = 128; + +type ActiveGsOp = (Vec, Op, Matrix); +type ActiveGsOps = std::rc::Rc>; + #[derive(Clone, Default)] struct ExtendedGStateTracker { - stack: Vec<(EffectiveGState, Vec<(Op, Matrix)>)>, + stack: Vec<(EffectiveGState, ActiveGsOps)>, current_state: EffectiveGState, - active_gs_ops: Vec<(Op, Matrix)>, + active_gs_ops: ActiveGsOps, overflow_depth: usize, } @@ -787,7 +792,7 @@ impl ExtendedGStateTracker { 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())); + self.stack.push((self.current_state.clone(), std::rc::Rc::clone(&self.active_gs_ops))); } b"Q" => { if self.overflow_depth > 0 { @@ -801,12 +806,20 @@ impl ExtendedGStateTracker { 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)); + let valid_gs = op + .name(0) + .and_then(|name| ext_gstates.get(name).map(|v| doc.resolve(v)).and_then(|v| v.as_dict().cloned()).map(|dict| (name, dict))); + if let Some((name, dict)) = valid_gs { + self.current_state.merge_dict(&dict, doc, Some(name)); + let mut next = (*self.active_gs_ops).clone(); + if let Some(pos) = next.iter().position(|(k, _, _)| k.as_slice() == name) { + if let Some(entry) = next.get_mut(pos) { + *entry = (name.to_vec(), op.clone(), ctm); + } + } else if next.len() < MAX_ACTIVE_GS_OPS { + next.push((name.to_vec(), op.clone(), ctm)); } - self.active_gs_ops.retain(|(o, _)| o.name(0) != Some(name)); - self.active_gs_ops.push((op.clone(), ctm)); + self.active_gs_ops = std::rc::Rc::new(next); } } _ => {} @@ -1198,7 +1211,7 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran } if i == target { source_effective_state = tracker.current_state.clone(); - source_gs_ops = tracker.active_gs_ops.clone(); + source_gs_ops = tracker.active_gs_ops.iter().map(|(_, op, ctm)| (op.clone(), *ctm)).collect(); } if i == scan_target { dest_effective_state = tracker.current_state.clone(); @@ -1342,7 +1355,7 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran unit_ops.push(extracted.do_op); unit_ops.push(Op::new("Q", vec![])); // An unbalanced sequence had no EMC; it doesn't gain one. - if marked_open.is_some() && (has_other_paint || target_unit.end.checked_sub(1).and_then(|i| ops.get(i)).is_some_and(|o| o.is("EMC"))) { + if marked_open.is_some() && target_unit.end.checked_sub(1).and_then(|i| ops.get(i)).is_some_and(|o| o.is("EMC")) { unit_ops.push(Op::new("EMC", vec![])); } @@ -1468,21 +1481,19 @@ fn image_exclusive_scope(ops: &[Op], target: usize) -> Result Result, EditError> { - let inverse = scope_matrix.invert().ok_or_else(|| EditError::Invalid("unsupported graphics state: non-invertible CTM".into()))?; - let placement = draw.then(&inverse); - if !placement.0.iter().all(|v| v.is_finite()) { - return Err(EditError::Invalid("unsupported graphics state: invalid transform coordinates".into())); - } +fn extract_image_drawing_ops( + ops: &[Op], + start: usize, + end: usize, + _target: usize, + _scope_matrix: Matrix, + _draw: Matrix, +) -> Result, EditError> { let is_path = |o: &Op| matches!(o.op.as_slice(), b"m" | b"l" | b"c" | b"v" | b"y" | b"h" | b"re"); let mut drawing = Vec::new(); let mut i = start.saturating_add(1); while i < end { let Some(op) = ops.get(i) else { break }; - if op.is("cm") { - i = i.saturating_add(1); - continue; - } if is_path(op) { let mut k = i; while ops.get(k).is_some_and(is_path) { @@ -1493,9 +1504,6 @@ fn extract_image_drawing_ops(ops: &[Op], start: usize, end: usize, target: usize continue; } } - if i == target && placement != Matrix::IDENTITY { - drawing.push(Op::new("cm", placement.0.map(pdfcraft_content::num).to_vec())); - } drawing.push(op.clone()); i = i.saturating_add(1); } @@ -1815,23 +1823,19 @@ fn crop_image_impl( .chain([Op::new("h", vec![]), Op::new("W", vec![]), Op::new("n", vec![])]) .collect() }; - // The image's own earlier crop (a clipping path right after the scope's `q`, skipping any - // initial `cm` view transform) gives way to the new one, so a crop can grow again as well as shrink. - let mut clip_start = start + 1; - while ops.get(clip_start).is_some_and(|o| o.is("cm")) { - clip_start += 1; - } + // The image's own earlier crop (a clipping path right after the scope's `q`, in the CTM before + // the scope) gives way to the new one, so a crop can grow again as well as shrink. let old_clip: Option> = (start != end) .then(|| { - let mut k = clip_start; + let mut k = start + 1; while ops.get(k).is_some_and(|o| matches!(o.op.as_slice(), b"m" | b"l" | b"c" | b"v" | b"y" | b"h" | b"re")) { k += 1; } - (k > clip_start && ops.get(k).is_some_and(|o| matches!(o.op.as_slice(), b"W" | b"W*")) && ops.get(k + 1).is_some_and(|o| o.is("n"))) - .then_some(clip_start..k + 2) + (k > start + 1 && ops.get(k).is_some_and(|o| matches!(o.op.as_slice(), b"W" | b"W*")) && ops.get(k + 1).is_some_and(|o| o.is("n"))) + .then_some(start + 1..k + 2) }) .flatten(); - let insert_at = old_clip.as_ref().map_or(clip_start, |r| r.start); + let insert_at = old_clip.as_ref().map_or(start + 1, |r| r.start); let changed = crate::text::splice(&pieces, &ops, |index| { if start == end && index == target { // No scope of its own yet: one around the drawing, with the crop inside. diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 2dbf5eafc1..48d9aa43fa 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -8542,3 +8542,58 @@ fn is_entire_page_wrapped_with_nested_scopes() { let ops2 = pieces2.parse().ops; assert!(!images::is_entire_page_wrapped(&ops2), "page with two separate top-level q...Q scopes is not wrapped"); } + +#[test] +fn hostile_page_with_many_gs_and_nested_q_does_not_hang_or_oom() { + use std::fmt::Write as _; + let mut stream = String::new(); + for i in 0..10_000 { + let _ = write!(stream, "/G{i} gs "); + } + for _ in 0..500 { + stream.push_str("q "); + } + stream.push_str("100 0 0 100 0 0 cm /Im0 Do "); + for _ in 0..500 { + stream.push_str("Q "); + } + let doc = image_page_with(&stream); + let start = std::time::Instant::now(); + let imgs = images::page_images(&doc, 0).unwrap(); + assert_eq!(imgs.len(), 1); + assert!(start.elapsed().as_secs() < 5, "page_images on hostile 10k gs must finish in under 5s, took {:?}", start.elapsed()); +} + +#[test] +fn crop_image_with_leading_cm_in_scope_places_clip_before_internal_cm() { + let content = "q 10 0 0 10 50 50 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::crop_image(&mut doc, 0, 0, [50.0, 50.0, 55.0, 55.0]).unwrap(); + let doc = reopen(&doc); + let after = images::page_images(&doc, 0).unwrap(); + assert_eq!(after.len(), 1); + assert!(after[0].visible_rect.is_some()); + let (v0, v1, v2, v3) = + (after[0].visible_rect.unwrap()[0], after[0].visible_rect.unwrap()[1], after[0].visible_rect.unwrap()[2], after[0].visible_rect.unwrap()[3]); + assert!((v0 - 50.0).abs() < 1e-3 && (v2 - 55.0).abs() < 1e-3, "visible rect must match crop coordinates: {:?}", after[0].visible_rect); + assert!((v1 - 50.0).abs() < 1e-3 && (v3 - 55.0).abs() < 1e-3, "visible rect must match crop coordinates: {:?}", after[0].visible_rect); +} + +#[test] +fn remove_crop_preserves_cm_before_extgstate() { + let content = "q 2 0 0 2 0 0 cm /GS0 gs 0 0 100 100 re W n 10 0 0 10 50 50 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(); + let doc = reopen(&doc); + let bytes = page_content_bytes(&doc, 0); + let s = String::from_utf8_lossy(&bytes); + assert!(!s.contains("W n"), "clipping path must be removed: {s}"); + // Verify that the initial cm appears before /GS0 gs + let cm_pos = s.find("2 0 0 2 0 0 cm").expect("cm must be preserved"); + let gs_pos = s.find("/GS0 gs").expect("gs must be preserved"); + assert!(cm_pos < gs_pos, "cm must precede /GS0 gs so soft mask space is preserved: {s}"); +} diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index 9d6bb88887..d1624490bb 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -10,11 +10,11 @@ //! otherwise it is set in Helvetica (WinAnsi), and the result says the font was substituted. //! Reflowing a paragraph to a new width is not done here (see `edit.text-reflow`). -use std::collections::{BTreeSet, HashMap}; +use std::collections::HashMap; use std::rc::Rc; use pdfcraft_content::{Matrix, Op, Pieces, parse, serialize_ops}; -use pdfcraft_cos::{DecodeBudget, Dict, Document, ObjRef, Object, PdfString, Stream}; +use pdfcraft_cos::{DecodeBudget, Dict, Document, Object, PdfString, Stream}; use pdfcraft_fonts::pdf::Metrics; use pdfcraft_fonts::{CraftFont, GlyphError, japanese_glyph_from}; @@ -162,273 +162,6 @@ pub fn register_text_font(doc: &mut Document, page: usize, bytes: &[u8]) -> Resu Ok(name) } -fn page_content_bytes_for_subset(doc: &Document, page: &pdfcraft_model::Page) -> Vec { - let Some(c) = page.dict.get(b"Contents") else { return Vec::new() }; - match doc.resolve(c).as_ref() { - Object::Stream(s) => s.decoded_within(16 << 20).unwrap_or_default(), - Object::Array(a) => a - .iter() - .flat_map(|x| match doc.resolve(x).as_ref() { - Object::Stream(s) => s.decoded_within(16 << 20).unwrap_or_default(), - _ => Vec::new(), - }) - .collect(), - _ => Vec::new(), - } -} - -/// Subset any added TrueType fonts in `doc` (marked with `/PCFullFont`) to only the glyphs used, -/// update their BaseFont/FontName with an `ABCDEF+` subset prefix, and remove `/PCFullFont`. -pub fn subset_document_fonts(doc: &mut Document) -> Result<(), EditError> { - let mut added_programs: Vec = Vec::new(); - for (r, obj) in doc.scan_objects() { - if let Object::Stream(s) = obj.as_ref() - && s.dict.get(b"PCFullFont").is_some() - { - added_programs.push(r); - } - } - if added_programs.is_empty() { - return Ok(()); - } - - let mut desc_for_program: HashMap = HashMap::new(); - let mut cid_for_desc: HashMap = HashMap::new(); - let mut type0_for_cid: HashMap> = HashMap::new(); - - for (r, obj) in doc.scan_objects() { - if let Object::Dict(d) = obj.as_ref() { - if d.name(b"Type") == Some(b"FontDescriptor") { - if let Some(Object::Ref(p_ref)) = d.get(b"FontFile2").or_else(|| d.get(b"FontFile3")) - && added_programs.contains(p_ref) - { - desc_for_program.insert(*p_ref, r); - } - } else if d.name(b"Type") == Some(b"Font") { - if d.name(b"Subtype") == Some(b"CIDFontType2") || d.name(b"Subtype") == Some(b"CIDFontType0") { - if let Some(Object::Ref(d_ref)) = d.get(b"FontDescriptor") { - cid_for_desc.insert(*d_ref, r); - } - } else if d.name(b"Subtype") == Some(b"Type0") { - let desc_opt = d.get(b"DescendantFonts").map(|o| doc.resolve(o)); - if let Some(desc_arc) = desc_opt - && let Object::Array(descendants) = desc_arc.as_ref() - && let Some(Object::Ref(c_ref)) = descendants.first() - { - type0_for_cid.entry(*c_ref).or_default().push(r); - } - } - } - } - } - - let mut type0_to_program: HashMap = HashMap::new(); - for (&prog, &desc) in &desc_for_program { - if let Some(&cid) = cid_for_desc.get(&desc) - && let Some(type0s) = type0_for_cid.get(&cid) - { - for &t0 in type0s { - type0_to_program.insert(t0, prog); - } - } - } - - let pages = pdfcraft_model::pages(doc); - let mut used_cids_by_program: HashMap> = HashMap::new(); - - for page in &pages { - let Some(res) = page.dict.get(b"Resources").and_then(|r| doc.dict(r)) else { continue }; - let mut font_res_to_program: HashMap, ObjRef> = HashMap::new(); - if let Some(fonts) = res.get(b"Font").and_then(|f| doc.dict(f)) { - for (name, val) in fonts.iter() { - if let Object::Ref(r) = val - && let Some(&prog) = type0_to_program.get(r) - { - font_res_to_program.insert(name.clone(), prog); - } - } - } - if font_res_to_program.is_empty() { - continue; - } - - let content = page_content_bytes_for_subset(doc, page); - let parsed = parse(&content); - let mut current_font: Option> = None; - for op in parsed.ops { - if op.is("Tf") { - current_font = op.operands.first().and_then(|o| o.as_name()).map(|n| n.to_vec()); - } else if let Some(cur) = current_font.as_ref() - && let Some(&prog) = font_res_to_program.get(cur) - { - let cids = used_cids_by_program.entry(prog).or_default(); - if op.is("Tj") || op.is("'") { - if let Some(s) = op.operands.first().and_then(|o| o.as_string()) { - for chunk in s.bytes.as_chunks::<2>().0 { - cids.insert(u16::from_be_bytes(*chunk)); - } - } - } else if op.is("\"") { - if let Some(s) = op.operands.get(2).and_then(|o| o.as_string()) { - for chunk in s.bytes.as_chunks::<2>().0 { - cids.insert(u16::from_be_bytes(*chunk)); - } - } - } else if op.is("TJ") - && let Some(arr) = op.operands.first().and_then(|o| o.as_array()) - { - for item in arr { - if let Some(s) = item.as_string() { - for chunk in s.bytes.as_chunks::<2>().0 { - cids.insert(u16::from_be_bytes(*chunk)); - } - } - } - } - } - } - } - - for prog_ref in added_programs { - let Some(&desc_ref) = desc_for_program.get(&prog_ref) else { continue }; - let Some(&cid_ref) = cid_for_desc.get(&desc_ref) else { continue }; - let empty_set = BTreeSet::new(); - let used_cids = used_cids_by_program.get(&prog_ref).unwrap_or(&empty_set); - - let Some(cid_dict) = doc.dict(&Object::Ref(cid_ref)) else { continue }; - let cid_to_gid_obj = cid_dict.get(b"CIDToGIDMap").cloned(); - let mut map_bytes: Option<(ObjRef, Vec)> = None; - let mut used_gids = BTreeSet::new(); - used_gids.insert(0u16); - - if let Some(Object::Ref(map_ref)) = cid_to_gid_obj { - if let Object::Stream(map_stream) = doc.get(map_ref).as_ref() - && let Ok(bytes) = map_stream.decoded_within(1 << 17) - { - for &cid in used_cids { - let offset = usize::from(cid).saturating_mul(2); - if let Some(chunk) = bytes.get(offset..offset.saturating_add(2)) - && chunk.len() == 2 - { - let gid = u16::from_be_bytes([chunk[0], chunk[1]]); - used_gids.insert(gid); - } - } - map_bytes = Some((map_ref, bytes)); - } - } else { - for &cid in used_cids { - used_gids.insert(cid); - } - } - - let Object::Stream(prog_stream) = doc.get(prog_ref).as_ref().clone() else { continue }; - let Ok(font_bytes) = prog_stream.decoded_within(16 << 20) else { continue }; - - let prefix = pdfcraft_fonts::embedding::subset_tag(u64::from(prog_ref.num)); - let mut desc_dict = doc.dict(&Object::Ref(desc_ref)).unwrap_or_default(); - let orig_name = - desc_dict.name(b"FontName").or_else(|| cid_dict.name(b"BaseFont")).and_then(|n| std::str::from_utf8(n).ok()).unwrap_or("Font"); - let clean_name = orig_name.rsplit('+').next().unwrap_or(orig_name); - let subset_name = format!("{prefix}{clean_name}"); - - let used_gids_vec: Vec = used_gids.into_iter().collect(); - - if let Ok((subset_bytes, old_to_new)) = pdfcraft_fonts::embedding::subset_truetype(&font_bytes, &used_gids_vec, &subset_name) { - let mut new_prog_dict = prog_stream.dict.clone(); - new_prog_dict.remove(b"PCFullFont"); - new_prog_dict.set(b"Length1".to_vec(), Object::Int(subset_bytes.len() as i64)); - doc.set(prog_ref, Object::Stream(Stream::flate(new_prog_dict, &subset_bytes))); - - if let Some((map_ref, mut m_bytes)) = map_bytes { - let max_cid = used_cids.iter().copied().next_back().unwrap_or(0); - let needed_len = usize::from(max_cid).saturating_add(1).saturating_mul(2); - 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)) { - let old_gid = u16::from_be_bytes([chunk[0], chunk[1]]); - let new_gid = old_to_new.get(&old_gid).copied().unwrap_or(0); - if let Some(target) = m_bytes.get_mut(s_idx..s_idx.saturating_add(2)) { - target.copy_from_slice(&new_gid.to_be_bytes()); - } - } - } - doc.set(map_ref, Object::Stream(Stream::flate(Dict::new(), &m_bytes))); - } - } else { - let mut new_prog_dict = prog_stream.dict.clone(); - new_prog_dict.remove(b"PCFullFont"); - doc.set(prog_ref, Object::Stream(Stream::flate(new_prog_dict, &font_bytes))); - } - - desc_dict.set(b"FontName".to_vec(), Object::name(&subset_name)); - doc.set(desc_ref, Object::Dict(desc_dict)); - - let mut new_cid_dict = cid_dict.clone(); - new_cid_dict.set(b"BaseFont".to_vec(), Object::name(&subset_name)); - - let first_t0 = type0_for_cid.get(&cid_ref).and_then(|t0s| t0s.first()).and_then(|r| doc.dict(&Object::Ref(*r))); - let m_opt = first_t0.as_ref().map(|d| Metrics::from_dict(doc, d)); - - if let Some(ref m) = m_opt { - let mut w_array = Vec::new(); - for &cid in used_cids { - let w = m.width(u32::from(cid)) * 1000.0; - w_array.push(Object::Int(i64::from(cid))); - w_array.push(Object::Array(vec![Object::Real(w)])); - } - if !w_array.is_empty() { - new_cid_dict.set(b"W".to_vec(), Object::Array(w_array)); - } - - let mut cmap = String::from( - "/CIDInit /ProcSet findresource begin\n12 dict begin\nbegincmap\n/CMapName /PCUnicode def\n/CMapType 2 def\n1 begincodespacerange\n<0000> \nendcodespacerange\n", - ); - let used_cids_vec: Vec = used_cids.iter().copied().collect(); - for chunk in used_cids_vec.chunks(100) { - use std::fmt::Write as _; - let _ = writeln!(cmap, "{} beginbfchar", chunk.len()); - for &cid in chunk { - let text = m.decode(&cid.to_be_bytes()); - let hex: String = if text.is_empty() { format!("{cid:04X}") } else { text.encode_utf16().map(|u| format!("{u:04X}")).collect() }; - let _ = writeln!(cmap, "<{cid:04X}> <{hex}>"); - } - cmap.push_str("endbfchar\n"); - } - cmap.push_str("endcmap\nCMapName currentdict /CMap defineresource pop\nend\nend\n"); - - if let Some(type0s) = type0_for_cid.get(&cid_ref) { - for &t0_ref in type0s { - if let Some(t0_dict) = doc.dict(&Object::Ref(t0_ref)) - && let Some(Object::Ref(to_uni_ref)) = t0_dict.get(b"ToUnicode") - { - doc.set(*to_uni_ref, Object::Stream(Stream::flate(Dict::new(), cmap.as_bytes()))); - } - } - } - } - - doc.set(cid_ref, Object::Dict(new_cid_dict)); - - if let Some(type0s) = type0_for_cid.get(&cid_ref) { - for &t0_ref in type0s { - if let Some(mut t0_dict) = doc.dict(&Object::Ref(t0_ref)) { - t0_dict.set(b"BaseFont".to_vec(), Object::name(&subset_name)); - doc.set(t0_ref, Object::Dict(t0_dict)); - } - } - } - } - - Ok(()) -} - /// Read-only draft bounds, measured with the same PDF rewrite used by Apply/Save. #[derive(Clone, Debug, PartialEq)] pub struct TextEditLayout { diff --git a/crates/engine/src/edit.rs b/crates/engine/src/edit.rs index b8a300937c..2a538a27dc 100644 --- a/crates/engine/src/edit.rs +++ b/crates/engine/src/edit.rs @@ -1449,10 +1449,11 @@ pub(crate) fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut E ); if should_promote { - let _ = doc.update_dict(added_item.obj, |d| { + doc.update_dict(added_item.obj, |d| { d.remove(b"PCAdded"); d.remove(b"PCMark"); - }); + }) + .map_err(|e| EditError::Edit(pdfcraft_edit::EditError::Invalid(e.to_string())))?; } else { ai_to_update = Some((*added_idx, ai)); } diff --git a/crates/engine/src/session.rs b/crates/engine/src/session.rs index 5c8373afb8..730b5d1eec 100644 --- a/crates/engine/src/session.rs +++ b/crates/engine/src/session.rs @@ -534,6 +534,7 @@ impl Session { | Edit::DeleteContent { .. } | Edit::ReplaceImage { .. } | Edit::PasteContent { .. } + | Edit::EditPageImage { .. } ) || matches!(e, Edit::MoveObjects { objects, .. } if objects.iter().any(|o| o.kind == ObjectKind::Added)) }); let touches_marks = edit.is_none_or(|e| { @@ -676,8 +677,7 @@ impl Session { // file so that revision stays in the chain. A full rewrite (redaction, new encryption, // a reconstructed file) still writes the current graph on its own. let rewrite = editor.cos.full_save_required() || editor.cos.encryption_changed() || editor.cos.revisions().is_empty(); - let mut cos = writable(&editor.cos)?; - pdfcraft_edit::text::subset_document_fonts(cos.to_mut())?; + let cos = writable(&editor.cos)?; guard(|| { if on_saved || rewrite { write_incremental(&cos, &opts) @@ -700,8 +700,7 @@ impl Session { } let editor = doc.editor.as_ref().ok_or_else(|| EditError::ReadOnly(doc.read_only_reason.clone().unwrap_or_default()))?; let opts = SaveOptions { mod_date: self.now().map(pdfcraft_cos::pdf_date), ..SaveOptions::default() }; - let mut cos = writable(&editor.cos)?; - pdfcraft_edit::text::subset_document_fonts(cos.to_mut())?; + let cos = writable(&editor.cos)?; guard(|| write_full(&cos, &opts)).map_err(EditError::Write)?.map(Arc::new).map_err(|e| EditError::Write(e.to_string())) } diff --git a/crates/engine/src/text_fonts.rs b/crates/engine/src/text_fonts.rs index e99c766bbb..8fc1275ed7 100644 --- a/crates/engine/src/text_fonts.rs +++ b/crates/engine/src/text_fonts.rs @@ -68,8 +68,8 @@ pub fn discover_budgeted(roots: &[PathBuf], mut budget: u64) -> Vec { } budget = budget.saturating_sub(meta.len()); let mut bytes = Vec::new(); - let read = std::fs::File::open(&path).and_then(|f| f.take((16 << 20) + 1).read_to_end(&mut bytes)); - if read.is_err() { + let read = std::fs::File::open(&path).and_then(|f| f.take(meta.len().saturating_add(1)).read_to_end(&mut bytes)); + if read.is_err() || bytes.len() != meta.len() as usize { continue; } if pdfcraft_fonts::embedding::validate_document_input(&bytes).is_err() { diff --git a/crates/engine/tests/it/document_fonts.rs b/crates/engine/tests/it/document_fonts.rs index bf9415ce91..e4212f4a3a 100644 --- a/crates/engine/tests/it/document_fonts.rs +++ b/crates/engine/tests/it/document_fonts.rs @@ -96,50 +96,3 @@ fn contextual_arabic_marks_ligatures_and_mixed_bidi_keep_logical_text_after_reop } exercise(font.bytes.to_vec(), "سَلَام 123", "NotoSansArabic-Regular"); } - -#[test] -fn added_font_is_subset_to_used_glyphs_on_save() { - let font_bytes = std::fs::read(std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../assets/fonts/Inter-Regular.ttf")).unwrap(); - let font_len = font_bytes.len(); - assert!(font_len > 300_000, "source font is full sized"); - - let mut s = Session::new(); - let id = s.open("source.pdf", None, document(), None).unwrap(); - s.apply(id, Edit::RegisterTextFont { page: 0, bytes: Arc::new(font_bytes) }).unwrap(); - let resource = s.get(id).unwrap().text_editor_fonts(0).into_iter().find(|f| f.face == "Inter-Regular").unwrap().resource; - let style = BlockStyle { resource: Some(resource), width: Some(240.0), ..Default::default() }; - let text = "Three words here"; - s.apply(id, Edit::EditTextBlock { page: 0, block: 0, text: text.into(), style }).unwrap(); - - let saved = s.save_bytes(id).unwrap(); - assert!(saved.len() < 50_000, "saved PDF with 3 words must be subset and small, was {} bytes", saved.len()); - - let saved_doc = pdfcraft_cos::Document::open(saved.clone()).unwrap(); - let mut subset_found = false; - for (_, obj) in saved_doc.scan_objects() { - if let pdfcraft_cos::Object::Dict(d) = obj.as_ref() - && d.name(b"Type") == Some(b"Font") - && d.name(b"Subtype") == Some(b"Type0") - && let Some(base) = d.name(b"BaseFont") - { - let base_str = String::from_utf8_lossy(base); - if base_str.ends_with("Inter-Regular") { - assert!(base_str.len() >= 7 && base_str.as_bytes()[6] == b'+', "font base name {base_str} must have subset prefix"); - subset_found = true; - } - } - if let pdfcraft_cos::Object::Stream(stream) = obj.as_ref() - && stream.dict.get(b"PCFullFont").is_some() - { - panic!("saved PDF must not retain PCFullFont flag"); - } - } - assert!(subset_found, "Type0 subset font must be present in saved document"); - - // Also verify reopening and rendering matches: - let re = s.open("saved.pdf", None, saved, None).unwrap(); - assert_eq!(s.get(re).unwrap().text_blocks(0)[0].editor_text, text); - let re_pixels = pixels(s.get(re).unwrap().display.clone()); - let orig_pixels = pixels(s.get(id).unwrap().display.clone()); - assert_eq!(re_pixels, orig_pixels, "reopened subset font renders identically to original"); -} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 80d7aea492..8c881ae33a 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1065,7 +1065,7 @@ pub enum ViewAction { CameraNext(&'static str), } -/// Default page right-click menu actions (matching Adobe Acrobat). +/// Default page right-click menu actions. #[derive(Clone, Debug, PartialEq)] pub(crate) enum PageMenuAction { RotateCCW(usize), @@ -1103,12 +1103,10 @@ pub(crate) fn default_page_context_menu(ui: &mut egui::Ui, page: usize, at: [f64 // Top horizontal icon bar: Rotate CCW, Print, Select tool, Hand tool, Marquee Zoom ui.horizontal(|ui| { ui.spacing_mut().item_spacing.x = 4.0; - ui.add_enabled_ui(perms.can_assemble, |ui| { - if crate::icons::button(ui, "rotate-ccw", 24.0, false, tl!("Rotate Counterclockwise")).clicked() { - *action = Some(PageMenuAction::RotateCCW(page)); - ui.close(); - } - }); + if crate::icons::button(ui, "rotate-ccw", 24.0, false, tl!("Rotate view counterclockwise")).clicked() { + *action = Some(PageMenuAction::RotateCCW(page)); + ui.close(); + } ui.add_enabled_ui(perms.can_print, |ui| { if crate::icons::button(ui, "printer", 24.0, false, tl!("Print")).clicked() { *action = Some(PageMenuAction::Print); @@ -5396,7 +5394,8 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { app.paste_image_at(page, at); } Some(PageMenuAction::RotateCCW(page)) => { - app.views[index].pending_edit = Some(pdfcraft_engine::Edit::RotatePages { pages: vec![page], degrees: -90 }); + let _ = page; + app.views[index].rotate_view(false); } 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 38f4d1e114..1c6df76300 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -953,6 +953,15 @@ impl crate::PdfCraftApp { /// Paste an image from the clipboard onto `page`. pub fn paste_image(&mut self, page: usize, rect: Option<[f64; 4]>) -> bool { + self.paste_image_internal(page, rect, None) + } + + /// Paste an image centered at context menu point `at: [f64; 2]` on `page`. + pub fn paste_image_at(&mut self, page: usize, at: [f64; 2]) -> bool { + self.paste_image_internal(page, None, Some(at)) + } + + fn paste_image_internal(&mut self, page: usize, rect: Option<[f64; 4]>, at: Option<[f64; 2]>) -> bool { let Some((i, id)) = self.active_ids() else { return false; }; @@ -980,7 +989,20 @@ impl crate::PdfCraftApp { None => { #[cfg(not(target_arch = "wasm32"))] let sys_result = if self.system_clipboard { - crate::os_clipboard::with(|cb| cb.get_image()).ok().map(|img| (img.width, img.height, img.bytes.into_owned())) + crate::os_clipboard::with(|cb| cb.get_image()).ok().and_then(|img| { + let expected = img.width.checked_mul(img.height).and_then(|p| p.checked_mul(4)); + if img.width > 0 + && img.height > 0 + && img.width <= crate::create_ui::MAX_IMAGE_SIDE + && img.height <= crate::create_ui::MAX_IMAGE_SIDE + && expected == Some(img.bytes.len()) + && img.bytes.len() <= crate::create_ui::MAX_IMAGE_BYTES + { + Some((img.width, img.height, img.bytes.into_owned())) + } else { + None + } + }) } else { None }; @@ -1018,7 +1040,18 @@ impl crate::PdfCraftApp { return false; }; - let target_rect = rect; + let target_rect = if let Some(r) = rect { + Some(r) + } else if let Some(at) = at { + let (raw_w, raw_h) = self.last_copied_image.as_ref().map(|(w, h, _)| (*w as f64, *h as f64)).unwrap_or((100.0, 100.0)); + let (pw, ph) = + self.session.get(id).and_then(|d| d.info.pages.get(page)).map(|p| (p.width as f64, p.height as f64)).unwrap_or((612.0, 792.0)); + let k = ((pw * 0.8) / raw_w.max(1.0)).min((ph * 0.8) / raw_h.max(1.0)).min(1.0); + let (w, h) = (raw_w * k, raw_h * k); + Some([at[0] - w * 0.5, at[1] - h * 0.5, at[0] + w * 0.5, at[1] + h * 0.5]) + } else { + None + }; 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; @@ -1062,41 +1095,9 @@ impl crate::PdfCraftApp { false } } - - /// Paste an image centered at context menu point `at: [f64; 2]` on `page`. - pub fn paste_image_at(&mut self, page: usize, at: [f64; 2]) -> bool { - let (raw_w, raw_h) = if let Some(crate::create_ui::Clip::Image { width, height, .. }) = self.clipboard_override.as_ref() { - (*width as f64, *height as f64) - } else { - #[cfg(not(target_arch = "wasm32"))] - let sys_img = - if self.system_clipboard { crate::os_clipboard::with(|cb| cb.get_image()).ok().map(|img| (img.width, img.height)) } else { None }; - #[cfg(target_arch = "wasm32")] - let sys_img: Option<(usize, usize)> = None; - - if let Some((img_width, img_height)) = sys_img { - (img_width as f64, img_height as f64) - } else if let Some((w, h, _)) = self.last_copied_image { - (w as f64, h as f64) - } else { - (100.0, 100.0) - } - }; - - let (_, id) = match self.active_ids() { - Some(ids) => ids, - None => return false, - }; - let (pw, ph) = self.session.get(id).and_then(|d| d.info.pages.get(page)).map(|p| (p.width as f64, p.height as f64)).unwrap_or((612.0, 792.0)); - let k = ((pw * 0.8) / raw_w.max(1.0)).min((ph * 0.8) / raw_h.max(1.0)).min(1.0); - let (w, h) = (raw_w * k, raw_h * k); - let rect = Some([at[0] - w * 0.5, at[1] - h * 0.5, at[0] + w * 0.5, at[1] + h * 0.5]); - self.paste_image(page, rect) - } } /// What the image tools ask for. -#[allow(dead_code)] pub(crate) enum ImageAction { Update(pdfcraft_engine::AddedContent), Replace, @@ -1104,13 +1105,10 @@ pub(crate) enum ImageAction { // Lucide names a flip icon after the axis it draws, Acrobat names the flip after the direction the // picture moves: Flip horizontal (left and right swap) shows Lucide's vertical axis. -#[allow(dead_code)] const FLIP_HORIZONTAL_ICON: &str = "flip-vertical-2"; -#[allow(dead_code)] const FLIP_VERTICAL_ICON: &str = "flip-horizontal-2"; /// Edit image: rotate, flip, crop, replace (shown while an added image is selected). -#[allow(dead_code)] pub(crate) fn image_panel(ui: &mut egui::Ui, t: &Tokens, img: &pdfcraft_engine::AddedImage) -> Option { let mut out = None; widgets::section_title(ui, tl!("Edit image")); diff --git a/crates/ui-egui/src/create_ui.rs b/crates/ui-egui/src/create_ui.rs index 2838d27482..38e400a771 100644 --- a/crates/ui-egui/src/create_ui.rs +++ b/crates/ui-egui/src/create_ui.rs @@ -57,11 +57,24 @@ pub enum Clip { } /// An image on the system clipboard, or else its text. +pub(crate) const MAX_IMAGE_SIDE: usize = 16384; +pub(crate) const MAX_IMAGE_BYTES: usize = 64 * 1024 * 1024; + #[cfg(not(target_arch = "wasm32"))] pub(crate) fn read_clipboard() -> Option { crate::os_clipboard::with(|cb| { if let Ok(img) = cb.get_image() { - return Ok(Some(Clip::Image { width: img.width, height: img.height, rgba: img.bytes.into_owned() })); + let expected_len = img.width.checked_mul(img.height).and_then(|px| px.checked_mul(4)); + if img.width > 0 + && img.height > 0 + && img.width <= MAX_IMAGE_SIDE + && img.height <= MAX_IMAGE_SIDE + && expected_len == Some(img.bytes.len()) + && img.bytes.len() <= MAX_IMAGE_BYTES + { + return Ok(Some(Clip::Image { width: img.width, height: img.height, rgba: img.bytes.into_owned() })); + } + return Ok(None); } Ok(cb.get_text().ok().filter(|t| !t.trim().is_empty()).map(Clip::Text)) }) @@ -71,8 +84,9 @@ pub(crate) fn read_clipboard() -> Option { pub(crate) fn png_from_rgba(width: usize, height: usize, rgba: &[u8]) -> Result, String> { let (w, h) = (u32::try_from(width).map_err(|e| e.to_string())?, u32::try_from(height).map_err(|e| e.to_string())?); - if w == 0 || h == 0 || rgba.len() != width * height * 4 { - return Err("the clipboard image is empty or malformed".into()); + let expected = width.checked_mul(height).and_then(|p| p.checked_mul(4)); + if w == 0 || h == 0 || width > MAX_IMAGE_SIDE || height > MAX_IMAGE_SIDE || expected != Some(rgba.len()) || rgba.len() > MAX_IMAGE_BYTES { + return Err("the clipboard image is empty, malformed or exceeds size limits".into()); } let mut out = Vec::new(); let mut enc = png::Encoder::new(&mut out, w, h); diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index ffa2a358ea..1438a8cdd1 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -1015,9 +1015,6 @@ impl CropMode { pub(crate) struct Matrix(pub(crate) [f64; 6]); impl Matrix { - #[allow(dead_code)] - pub(crate) const IDENTITY: Self = Matrix([1.0, 0.0, 0.0, 1.0, 0.0, 0.0]); - pub(crate) fn apply(&self, x: f64, y: f64) -> (f64, f64) { let [a, b, c, d, e, f] = self.0; (a * x + c * y + e, b * x + d * y + f) @@ -1570,13 +1567,15 @@ pub(crate) fn image_input( } let hovered_box = hover_pos.and_then(|p| { boxes.iter().enumerate().rposition(|(i, b)| { - b.is_positive() && (b.contains(p) || quad_contains(ImageObb::from_image(&images[i], xf, info, page).corners(), p)) && image_contains(i, p) + b.is_positive() + && images.get(i).is_some_and(|img| b.contains(p) || quad_contains(ImageObb::from_image(img, xf, info, page).corners(), p)) + && image_contains(i, p) }) }); if !faint_all { for i in selected.filter(|_| moving.is_none()).into_iter().chain(hovered_box.filter(|h| Some(*h) != selected)) { - if let Some(b) = boxes.get(i).filter(|b| b.is_positive()) { - let obb = ImageObb::from_image(&images[i], xf, info, page); + if let (Some(b), Some(img)) = (boxes.get(i).filter(|b| b.is_positive()), images.get(i)) { + let obb = ImageObb::from_image(img, xf, info, page); if obb.is_rotated { let pts = obb.corners(); for k in 0..4 { @@ -1589,8 +1588,8 @@ pub(crate) fn image_input( } } for i in selected.into_iter().chain(hovered_box).collect::>() { - let Some(b) = boxes.get(i).copied().filter(|b| b.is_positive()) else { continue }; - let obb = ImageObb::from_image(&images[i], xf, info, page); + let (Some(b), Some(img)) = (boxes.get(i).copied().filter(|b| b.is_positive()), images.get(i)) else { continue }; + let obb = ImageObb::from_image(img, xf, info, page); if selected == Some(i) { if !dragging { if obb.is_rotated { @@ -1607,8 +1606,8 @@ pub(crate) fn image_input( } // The selected image: frame, corner handles, dragging. if let Some(i) = selected.filter(|i| boxes.get(*i).is_some_and(|b| b.is_positive())) { - let b = boxes[i]; - let image = &images[i]; + let Some(b) = boxes.get(i).copied() else { return false }; + let Some(image) = images.get(i) else { return false }; let obb = ImageObb::from_image(image, xf, info, page); // Until the page is rendered again its raster has an applied move or resize's picture // where it was: it stays blanked there and drawn where it went. @@ -2050,7 +2049,9 @@ pub(crate) fn image_input( let menu_key = image_menu_id(); let Some(p) = hover_pos.filter(|p| xf.rect.contains(*p)) else { return false }; let Some(hit) = boxes.iter().enumerate().rposition(|(i, b)| { - b.is_positive() && (b.contains(p) || quad_contains(ImageObb::from_image(&images[i], xf, info, page).corners(), p)) && image_contains(i, p) + b.is_positive() + && images.get(i).is_some_and(|img| b.contains(p) || quad_contains(ImageObb::from_image(img, xf, info, page).corners(), p)) + && image_contains(i, p) }) else { if resp.secondary_clicked() || resp.clicked() { view.image_selection = None; @@ -2058,15 +2059,16 @@ pub(crate) fn image_input( } return false; }; - let hit_obb = ImageObb::from_image(&images[hit], xf, info, page); + let Some(hit_img) = images.get(hit) else { return false }; + let hit_obb = ImageObb::from_image(hit_img, xf, info, page); if selected != Some(hit) { if hit_obb.is_rotated { let pts = hit_obb.corners(); for k in 0..4 { painter.line_segment([pts[k], pts[(k + 1) % 4]], Stroke::new(1.5, ACCENT.gamma_multiply(0.7))); } - } else { - painter.rect_stroke(boxes[hit], CornerRadius::ZERO, Stroke::new(1.5, ACCENT.gamma_multiply(0.7)), egui::StrokeKind::Outside); + } else if let Some(b) = boxes.get(hit).copied() { + painter.rect_stroke(b, CornerRadius::ZERO, Stroke::new(1.5, ACCENT.gamma_multiply(0.7)), egui::StrokeKind::Outside); } } let corner = hit_obb.handles().iter().position(|(_, c)| c.distance(p) < 10.0); @@ -2088,7 +2090,7 @@ pub(crate) fn image_input( true } -/// The right-click context menu for a selected image matching Adobe Acrobat. +/// The right-click context menu for a selected image. #[allow(clippy::too_many_arguments)] pub(crate) fn image_context_menu( ui: &mut egui::Ui, diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 9551564f3f..c2c1435204 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -54,7 +54,11 @@ impl PdfCraftApp { let target = doc.and_then(|d| d.page_images(*page).get(*index).cloned()); let ambiguous = doc.is_some_and(|d| { if let Some(t) = &target { - d.page_images(*page).iter().filter(|img| img.name == t.name && img.object == t.object && img.rect == t.rect).count() > 1 + d.page_images(*page) + .iter() + .filter(|img| img.name == t.name && img.object == t.object && rect_matches(img.rect, t.rect)) + .count() + > 1 } else { false } @@ -260,7 +264,8 @@ impl PdfCraftApp { let doc = self.session.get(id)?; let images = doc.page_images(s.page); let target = images.get(s.index).cloned()?; - let ambiguous = images.iter().filter(|img| img.name == target.name && img.object == target.object && img.rect == target.rect).count() > 1; + let ambiguous = + images.iter().filter(|img| img.name == target.name && img.object == target.object && rect_matches(img.rect, target.rect)).count() > 1; Some((s.page, target, ambiguous)) }); let had_selection = self.views.get(i).and_then(|v| v.image_selection.as_ref()).is_some(); @@ -1074,6 +1079,10 @@ impl PdfCraftApp { } } +fn rect_matches(a: [f64; 4], b: [f64; 4]) -> bool { + a.iter().zip(b.iter()).all(|(x, y)| (x - y).abs() < 1e-3) +} + fn re_resolve_image_selection( view: &mut crate::DocView, page: usize, @@ -1084,7 +1093,7 @@ fn re_resolve_image_selection( let exact_matches: Vec = page_images .iter() .enumerate() - .filter(|(_, img)| img.name == target.name && img.object == target.object && img.rect == target.rect) + .filter(|(_, img)| img.name == target.name && img.object == target.object && rect_matches(img.rect, target.rect)) .map(|(idx, _)| idx) .collect(); let new_idx = if !ambiguous_before && exact_matches.len() == 1 { @@ -1172,9 +1181,6 @@ fn is_form_edit(edit: &Edit) -> bool { /// `Packages\Microsoft.OutlookForWindows_` package folder for the new Outlook. The patterns /// follow those layouts, so an ordinary folder that merely shares a name doesn't match. Saving there either fails, /// because Outlook can still hold the file, or appears to work while the edits stay out of the -/// email and are deleted with the cache. Matched on either separator and in any case, so it can -/// be tested on every platform. -#[allow(dead_code)] /// email and are deleted with the cache. Apple Mail opens one from `Mail Downloads//` in /// its sandbox container, where other apps can't write, so saving there fails (#2751). Matched on /// either separator and in any case, so it can be tested on every platform. diff --git a/crates/ui-egui/src/format_text_ui.rs b/crates/ui-egui/src/format_text_ui.rs index 865c4f133f..e5b48a5a52 100644 --- a/crates/ui-egui/src/format_text_ui.rs +++ b/crates/ui-egui/src/format_text_ui.rs @@ -116,7 +116,8 @@ pub(crate) fn panel(ui: &mut egui::Ui, t: &Tokens, look: &mut AddedText, extras: let search_id = ui.scope_id().with("installed-font-search"); let mut query = ui.data(|d| d.get_temp::(search_id)).unwrap_or_default(); ui.horizontal(|ui| { - ui.label(egui::RichText::new("🔍").size(12.0)); + let (icon_rect, _) = ui.allocate_exact_size(egui::vec2(16.0, 16.0), egui::Sense::hover()); + crate::icons::paint(ui, icon_rect, "search", 13.0, ui.visuals().text_color().gamma_multiply(0.6)); let resp = ui .add(egui::TextEdit::singleline(&mut query).hint_text(tl!("Search…")).desired_width(ui.available_width() - 8.0)); if resp.changed() { diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index c44dc8c9a8..4cad1fcff4 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -4140,8 +4140,8 @@ fn default_page_context_menu_when_not_in_edit_mode() { h.event(egui::Event::PointerButton { pos: c, button: egui::PointerButton::Secondary, pressed: false, modifiers: Default::default() }); h.run_steps(2); - // Verify Acrobat page context menu items (Screenshot 3) - h.get_by_label("Rotate Counterclockwise"); + // Verify default page context menu items + h.get_by_label("Rotate view counterclockwise"); h.get_by_label("Print"); h.get_by_label("Select Tool"); h.get_by_label("Hand Tool"); @@ -7800,8 +7800,8 @@ fn clicking_after_image_context_menu_action_resets_menu_state_and_updates_select h.run_steps(2); let r = h.state().views[0].page_screen_rect(0).expect("on screen"); - let user_to_screen = |u: [f64; 2]| { - let info = &h.state().session.get(id).unwrap().info.pages[0]; + let info = h.state().session.get(id).unwrap().info.pages[0].clone(); + let user_to_screen = move |u: [f64; 2]| { let v = info.user_to_view(u[0] as f32, u[1] as f32); let scale = r.width() / info.width; egui::pos2(r.left() + v[0] * scale, r.top() + v[1] * scale) @@ -7995,3 +7995,89 @@ fn a_paragraph_selected_by_its_frame_deletes_with_one_key() { assert_eq!(blocks(&h).iter().map(|b| b.text.as_str()).collect::>(), ["Page 1"], "undo restores it"); } } + +fn two_image_fixture() -> Vec { + let content = "q 20 0 0 20 20 20 cm /Im0 Do Q q 20 0 0 20 100 100 cm /Im0 Do Q"; + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 300] /Contents 4 0 R /Resources << /XObject << /Im0 5 0 R >> >> >>".into(), + format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()), + "<< /Type /XObject /Subtype /Image /Width 2 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 8 /Length 4 >>\nstream\n\0\0\0\0\nendstream" + .into(), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +#[test] +fn duplicate_xobject_at_different_rects_arrange_and_delete_keeps_other_instance_through_menu() { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("dup.pdf", None, two_image_fixture()).expect("opens fixture"); + app + }); + h.run_steps(4); + let id = h.state().views[0].id; + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + + let r = h.state().views[0].page_screen_rect(0).expect("on screen"); + let info = h.state().session.get(id).unwrap().info.pages[0].clone(); + let user_to_screen = move |u: [f64; 2]| { + let v = info.user_to_view(u[0] as f32, u[1] as f32); + let scale = r.width() / info.width; + egui::pos2(r.left() + v[0] * scale, r.top() + v[1] * scale) + }; + + let p1 = user_to_screen([110.0, 110.0]); + + // 1. Right-click B (image 1) to open context menu + h.hover_at(p1); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: p1, button: egui::PointerButton::Secondary, pressed: true, modifiers: Default::default() }); + h.event(egui::Event::PointerButton { pos: p1, button: egui::PointerButton::Secondary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); + + // 2. Click "Bring to front" in context menu + h.get_by_label("Bring to front").click(); + h.run_steps(3); + + // 3. Selection must be preserved after arrange + let sel = h.state().views[0].image_selection.as_ref().map(|s| s.index); + assert!(sel.is_some(), "selection must be preserved after arrange"); + + // 4. Right-click on the selected image again to open context menu + let sel_idx = sel.unwrap(); + let b_rect = h.state().session.get(id).unwrap().page_images(0)[sel_idx].rect; + let p_b = user_to_screen([(b_rect[0] + b_rect[2]) / 2.0, (b_rect[1] + b_rect[3]) / 2.0]); + h.hover_at(p_b); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: p_b, button: egui::PointerButton::Secondary, pressed: true, modifiers: Default::default() }); + h.event(egui::Event::PointerButton { pos: p_b, button: egui::PointerButton::Secondary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); + + // 5. Click "Delete image" in context menu + h.get_by_label("Delete image").click(); + h.run_steps(3); + + // 6. Save, reopen and verify: A remains at [20, 20, 40, 40], B is deleted + let saved = h.state().session.save_bytes(id).unwrap(); + let mut reopened = pdfcraft_engine::Session::new(); + let rid = reopened.open("again.pdf", None, saved, None).unwrap(); + let imgs = reopened.get(rid).unwrap().page_images(0); + assert_eq!(imgs.len(), 1, "exactly one image must remain after deleting B"); + assert!((imgs[0].rect[0] - 20.0).abs() < 1e-2, "remaining image must be A at [20, 20, 40, 40], was {:?}", imgs[0].rect); +} diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index a5c3192358..10a2c7ec36 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -59,6 +59,7 @@ mod pageboxes; mod panel_snapshots; mod pdf_links; mod pickers; +mod pinned_tools; mod pointer; mod preferences_layout; mod prepare; diff --git a/crates/ui-egui/tests/pinned_tools.rs b/crates/ui-egui/tests/it/pinned_tools.rs similarity index 100% rename from crates/ui-egui/tests/pinned_tools.rs rename to crates/ui-egui/tests/it/pinned_tools.rs diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index c0bd743822..d5c9d487ad 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1799,11 +1799,17 @@ title = "Customise the quick-action toolbar and pinned tools" area = "B" tier = "P2" milestone = "M14" -status = "planned" +status = "partial" commands = [] tools = [] -evidence = [] -notes = "" +evidence = [ + "crates/ui-egui/tests/it/pinned_tools.rs::state_operations_and_validation", + "crates/ui-egui/tests/it/pinned_tools.rs::persistence_round_trip_and_sanitization", + "crates/ui-egui/tests/it/pinned_tools.rs::home_shows_pinned_tools_section_and_placeholder", + "crates/ui-egui/tests/it/pinned_tools.rs::pin_tool_dialog_workflow", + "crates/ui-egui/tests/it/pinned_tools.rs::sidebar_pin_toggle_unpins_and_pins", +] +notes = "Pin and unpin tools on the Home page via tool card hover button, All Tools row toggle, or 'Pin a tool' search dialog. Pinned tool order persists across sessions. Floating quick-action toolbar customisation remains planned." [[feature]] id = "view.restriction-notice" @@ -2154,7 +2160,7 @@ status = "shipped" commands = ["edit.edit_text"] tools = ["image_edit", "image_hit"] evidence = ["crates/edit/src/tests.rs::private_image_crop_with_intervening_state_resizes_across_streams", "crates/automation/tests/it/automation.rs::private_crops_with_nested_state_resize_save_reopen_through_tools", "crates/edit/src/tests.rs::form_artwork_edits_only_the_selected_placement", "crates/ui-egui/tests/it/editing.rs::editing_existing_form_artwork_on_the_page", "crates/automation/tests/it/automation.rs::editing_form_artwork_through_tools", "crates/edit/src/tests.rs::page_images_move_turn_replace_and_delete", "crates/ui-egui/tests/it/editing.rs::editing_existing_images_on_the_page", "crates/automation/tests/it/automation.rs::editing_page_images_through_tools", "crates/edit/src/tests.rs::curved_image_clips_have_analytic_bounds_and_holes_do_not_capture_points", "crates/edit/src/tests.rs::cropped_image_resizing_moves_private_clips_and_keeps_shared_ones", "crates/ui-egui/tests/it/editing.rs::cropped_image_corner_resize_keeps_opposite_corner_and_scales_crop", "crates/edit/src/tests.rs::image_selection_bounds_respect_clips_and_restore_graphics_state", "crates/ui-egui/tests/it/editing.rs::cropped_images_do_not_intercept_text_and_stay_inside_each_page", "crates/automation/tests/it/automation.rs::clipped_image_bounds_and_text_edit_save_reopen_through_tools", "crates/edit/src/tests.rs::resizing_an_image_under_a_shared_clip_moves_its_own_clip"] -notes = "existing raster images and grouped Form artwork: select, drag to move, drag a corner to resize keeping proportions; Delete removes one placement. Forms include their nested content as a group; individual internal elements and inline images are not supported. See docs/editing-images.md; added images too Since #1116: raster images and grouped Forms move/resize/turn as placements; selected grips own the gesture and opposite anchor, with no PDF writes during movement. Curved path frames use analytic Bezier extrema and hit testing evaluates actual nonzero/even-odd fill rules, excluding holes; intersections can retain a loose frame. Selection and resize use the visible (clipped) frame; exclusive image clips move with their pixels across nested states, intervening operators and content streams, also inside a clip the picture shares with other content (its own clip scales, the shared one stays); shared clips stay fixed. Inline images and Form internals remain unsupported; see docs/editing-images.md" +notes = "existing raster images and grouped Form artwork: select, drag to move, 8-point handles to resize (corners keep proportions, edges scale along axis, Shift inverts aspect lock); Delete removes one placement. Forms include their nested content as a group; individual internal elements and inline images are not supported. See docs/editing-images.md; added images too Since #1116: raster images and grouped Forms move/resize/turn as placements; selected grips own the gesture and opposite anchor, with no PDF writes during movement. Curved path frames use analytic Bezier extrema and hit testing evaluates actual nonzero/even-odd fill rules, excluding holes; intersections can retain a loose frame. Selection and resize use the visible (clipped) frame; exclusive image clips move with their pixels across nested states, intervening operators and content streams, also inside a clip the picture shares with other content (its own clip scales, the shared one stays); shared clips stay fixed. Inline images and Form internals remain unsupported; see docs/editing-images.md" [[feature]] id = "edit.image-extract" @@ -2189,8 +2195,12 @@ milestone = "M7" status = "partial" commands = [] tools = ["image_edit"] -evidence = ["crates/edit/src/tests.rs::image_arrange_stacking_order_and_remove_crop"] -notes = "Image stacking order in content streams: Bring to Front, Send to Back, Bring Forward, Send Backward with undo/redo and PDF export persistence. Image-level crop and 8-point handle transforms. Still missing: arbitrary vector paths, inline images, and multi-object grouping." +evidence = [ + "crates/edit/src/tests.rs::image_arrange_stacking_order_and_remove_crop", + "crates/automation/tests/it/automation.rs::arranging_page_images_through_tools", + "crates/ui-egui/tests/it/editing.rs::duplicate_xobject_at_different_rects_arrange_and_delete_keeps_other_instance_through_menu", +] +notes = "Image stacking order in content streams: Bring to Front, Send to Back, Bring Forward, Send Backward with undo/redo and persistence. Refuses arranging across shared marked content or when clipping path transforms cannot be cleanly preserved. Stacking for arbitrary vector paths and inline images remains planned." [[feature]] id = "edit.align-distribute-objects"