diff --git a/crates/edit/src/added.rs b/crates/edit/src/added.rs index d7dbcee752..d292c0869f 100644 --- a/crates/edit/src/added.rs +++ b/crates/edit/src/added.rs @@ -1257,6 +1257,21 @@ pub fn delete_content(doc: &mut Document, page: usize, index: usize) -> Result<( Ok(()) } +/// Leave only item `index` on `page`: its contents become the item's own stream and its +/// annotations go, so a render of the page (on a transparent backdrop) is the item's pixels +/// alone. For previews on a copy of the document; the page's resources stay as they are. +pub fn isolate_content(doc: &mut Document, page: usize, index: usize) -> Result<(), EditError> { + let a = find(doc, page, index)?; + let p = page_list(doc).swap_remove(page); + // An item's stream starts from the page's initial graphics state: `place_tagged` closes + // everything drawn before it, so it draws the same on its own. + doc.update_dict(p.obj, |d| { + d.set(b"Contents".to_vec(), Object::Array(vec![Object::Ref(a.obj)])); + d.remove(b"Annots"); + })?; + Ok(()) +} + /// The Type 3 fonts an Arabic item's stream shows text with. Each item has its own (`draw_shaped` /// names them so), so they go when the item is replaced or deleted. fn own_fonts(doc: &Document, obj: ObjRef) -> Vec> { diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index f324a565eb..ef8977a858 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -1044,8 +1044,8 @@ pub use text::{ }; pub mod added; pub use added::{ - Added, AddedImage, AddedText, Align, Content, Family, MarkerScan, add_content, delete_content, first_undrawable, list_added, scan_markers, - update_content, + Added, AddedImage, AddedText, Align, Content, Family, MarkerScan, add_content, delete_content, first_undrawable, isolate_content, list_added, + scan_markers, update_content, }; #[cfg(test)] diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 2ba817750f..0f8adc6514 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -1009,6 +1009,28 @@ fn added_text_and_images_are_page_content_that_stays_editable() { assert!(marks_present(&doc).is_empty()); } +/// #4464: a page left with one added item draws that item and nothing else. +#[test] +fn an_isolated_item_is_all_that_is_left_on_its_page() { + let mut doc = fixture(); + let first = AddedText { rect: [72.0, 600.0, 300.0, 700.0], text: "First".into(), size: 14.0, ..AddedText::default() }; + let second = AddedText { rect: [72.0, 400.0, 300.0, 500.0], text: "Second".into(), ..first.clone() }; + add_content(&mut doc, 0, &Content::Text(first)).unwrap(); + add_content(&mut doc, 0, &Content::Text(second)).unwrap(); + let before = streams(&doc, 0).join("\n"); + assert!(before.contains("(First) Tj") && before.contains("(Second) Tj"), "{before}"); + assert!(isolate_content(&mut doc, 0, 2).is_err(), "no third item"); + assert!(isolate_content(&mut doc, 9, 0).is_err(), "no such page"); + let page = pdfcraft_model::pages(&doc).swap_remove(0); + doc.update_dict(page.obj, |d| d.set(b"Annots".to_vec(), Object::Array(Vec::new()))).unwrap(); + isolate_content(&mut doc, 0, 1).unwrap(); + let after = streams(&doc, 0).join("\n"); + assert!(after.contains("(Second) Tj") && !after.contains("(First) Tj"), "{after}"); + let page = pdfcraft_model::pages(&doc).swap_remove(0); + assert!(page.dict.get(b"Annots").is_none(), "no annotations are drawn over it"); + assert_eq!(list_added(&doc).len(), 1); +} + /// Edit ▸ Paste (#2636): where a pasted item lands. #[test] fn a_paste_lands_where_the_copy_was_offset_per_paste_and_stays_on_the_page() { diff --git a/crates/engine/src/text_preview.rs b/crates/engine/src/text_preview.rs index d533faf7e0..290231cf73 100644 --- a/crates/engine/src/text_preview.rs +++ b/crates/engine/src/text_preview.rs @@ -89,4 +89,17 @@ impl Document { Ok(TextEditPreview { page, bytes, config: self.config.clone() }) })? } + + /// An added item alone, on a transparent page: what follows the pointer while it is dragged, + /// so the page under its first place doesn't come along. Read-only, like + /// [`Self::added_content_preview`]. + pub fn added_content_alone(&self, page: usize, index: usize) -> Result { + guard(|| { + let editor = self.editor.as_ref().ok_or("the document can't be read")?; + let mut cos = editor.cos.clone(); + pdfcraft_edit::isolate_content(&mut cos, page, index).map_err(|e| e.to_string())?; + let bytes = Arc::new(pdfcraft_cos::write_incremental(&cos, &pdfcraft_cos::SaveOptions::default()).map_err(|e| e.to_string())?); + Ok(TextEditPreview { page, bytes, config: RenderConfig { transparent: true, ..self.config.clone() } }) + })? + } } diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 81b3fe2729..02bc000ed6 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -3413,8 +3413,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } view.text_background.paint(painter, &xf, i, view.line_editor.as_ref(), view.content.draft.as_ref()); if editing_content { - let page_tex = view.pages.get(&i).map(|p| p.tex.id()); - view.move_background.paint(painter, &xf, i, info, &view.content, page_tex, pointer); + view.move_background.paint(painter, &xf, i, info, &view.content, pointer); } if view.show_page_gaps { painter.rect_stroke(r, CornerRadius::ZERO, Stroke::new(0.5, t.border.gamma_multiply(0.8)), egui::StrokeKind::Outside); diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index 87ec76e77b..0d86f7dd8f 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -118,6 +118,10 @@ pub(crate) struct MoveBackground { key: Option<(u64, usize, usize)>, renderer: Option, texture: Option, + /// The item alone on a transparent page: the pixels that follow the pointer. The page's own + /// raster has whatever the item covers in its box too, which must stay where it is (#4464). + item_renderer: Option, + item: Option, tag: Option, /// The item's display-space box when the gesture started. from: Option<[f64; 4]>, @@ -130,7 +134,12 @@ pub(crate) struct MoveBackground { impl MoveBackground { pub fn pending(&self) -> bool { - self.renderer.is_some() && self.texture.is_none() + self.renderer.is_some() && self.item_renderer.is_some() && (self.texture.is_none() || self.item.is_none()) + } + + /// Both rasters are in: the page without the item, and the item alone. + fn ready(&self) -> bool { + self.texture.is_some() && self.item.is_some() } pub fn prepare(&mut self, ctx: &egui::Context, doc: &pdfcraft_engine::Document, cv: &ContentView, scale: f32) { @@ -145,6 +154,7 @@ impl MoveBackground { *self = Self { key, ..Default::default() }; if let Some((_, page, index)) = key { self.renderer = doc.added_content_preview(page, index).ok().map(|p| p.renderer()); + self.item_renderer = doc.added_content_alone(page, index).ok().map(|p| p.renderer()); self.from = on_page(doc.added(), page).into_iter().find(|(i, _)| *i == index).map(|(_, a)| a.content.rect()); self.picture = picture(page, index); } @@ -152,39 +162,60 @@ impl MoveBackground { if self.settling.is_some() { return; } - let (Some((_, page, _)), Some(renderer)) = (self.key, self.renderer.as_ref()) else { return }; + let (Some((_, page, _)), Some(renderer), Some(item_renderer)) = (self.key, self.renderer.as_ref(), self.item_renderer.as_ref()) else { + return; + }; let Some(p) = doc.info.pages.get(page) else { return }; let max_side = ctx.input(|i| i.max_texture_side) as f32; let scale = scale.min(crate::canvas::BASE_SIDE.min(max_side) / p.width.max(p.height).max(1.0)); let tag = (scale * 1000.0) as u64; if self.tag != Some(tag) { - renderer.set_queue(vec![pdfcraft_render::RenderRequest { page, scale, tag, ..Default::default() }]); + for r in [renderer, item_renderer] { + r.set_queue(vec![pdfcraft_render::RenderRequest { page, scale, tag, ..Default::default() }]); + } self.tag = Some(tag); } - if let Some(out) = renderer.try_recv() { + if self.receive(ctx, tag) && self.pending() { + ctx.request_repaint_after(std::time::Duration::from_millis(16)); + } + } + + /// Takes in the rasters rendered at `tag`. `false` when either failed: the preview is then + /// dropped whole, including rasters in from an earlier scale. + fn receive(&mut self, ctx: &egui::Context, tag: u64) -> bool { + let (Some(renderer), Some(item_renderer)) = (self.renderer.as_ref(), self.item_renderer.as_ref()) else { + return false; + }; + let mut failed = false; + for (r, slot, name) in + [(renderer, &mut self.texture, "added-content-move-background"), (item_renderer, &mut self.item, "added-content-move-item")] + { + let Some(out) = r.try_recv() else { continue }; if out.error.is_some() { - // Without a background the item stays put while dragged, as before. - self.renderer = None; - return; - } - if out.request.tag == tag { - self.texture = Some(ctx.load_texture( - "added-content-move-background", + failed = true; + } else if out.request.tag == tag { + *slot = Some(ctx.load_texture( + name, crate::canvas::texture_image([out.width as usize, out.height as usize], out.rgba), egui::TextureOptions::LINEAR, )); } } - if self.pending() { - ctx.request_repaint_after(std::time::Duration::from_millis(16)); + if failed { + // Without both rasters the item stays put while dragged, as before. + self.renderer = None; + self.item_renderer = None; + self.texture = None; + self.item = None; } + !failed } /// The move was applied: keep the preview, now at the new generation, until the page is /// rendered again. Any other edit ends it. pub fn committed(&mut self, edit: &Edit, generation: u64) { match edit { - Edit::UpdateContent { page, index, content } if self.texture.is_some() && self.key.is_some_and(|(_, p, i)| (p, i) == (*page, *index)) => { + Edit::UpdateContent { page, index, content } if self.ready() && self.key.is_some_and(|(_, p, i)| (p, i) == (*page, *index)) => { self.key = Some((generation, *page, *index)); self.settling = Some(content.rect()); } @@ -198,21 +229,11 @@ impl MoveBackground { } } - /// The page without the moving item, then the item's pixels taken from `page_tex` (the page - /// as rendered, item included where it was) at the dragged (to the pages' `pointer`) or - /// applied position. - #[allow(clippy::too_many_arguments)] - pub fn paint( - &self, - painter: &egui::Painter, - xf: &PageXform, - page: usize, - info: &DocInfo, - cv: &ContentView, - page_tex: Option, - pointer: Option, - ) { - let (Some((_, kp, _)), Some(background), Some(page_tex), Some(from)) = (self.key, &self.texture, page_tex, self.from) else { return }; + /// The page without the moving item, then the item's own pixels (from its raster alone, so + /// nothing it covered comes along) at the dragged (to the pages' `pointer`) or applied + /// position. + pub fn paint(&self, painter: &egui::Painter, xf: &PageXform, page: usize, info: &DocInfo, cv: &ContentView, pointer: Option) { + let (Some((_, kp, _)), Some(background), Some(item), Some(from)) = (self.key, &self.texture, &self.item, self.from) else { return }; if kp != page { return; } @@ -231,7 +252,7 @@ impl MoveBackground { xf.paint_image(&painter, background.id(), 0.0, 0.0, 1.0, 1.0); // A pixel around the box, so its edges come along. let (r, to) = (r.expand(1.0), to.expand(1.0)); - let mut mesh = egui::Mesh::with_texture(page_tex); + let mut mesh = egui::Mesh::with_texture(item.id()); for (a, b) in [ (r.left_top(), to.left_top()), (r.right_top(), to.right_top()), @@ -1033,6 +1054,35 @@ mod tests { assert!(i.flip_v && !i.flip_h, "Flip vertical mirrors top and bottom"); } + /// #4464: both rasters were in at one scale and a render at a new scale failed. The rasters + /// from the old scale must go too, or the item keeps following the pointer although the + /// preview gave up. + #[test] + fn failed_rerender_drops_the_rasters_from_the_earlier_scale() { + let ctx = egui::Context::default(); + let texture = |name| Some(ctx.load_texture(name, egui::ColorImage::filled([1, 1], Color32::WHITE), egui::TextureOptions::LINEAR)); + // Bytes no parser accepts: every render answers with an error. + let pool = || { + let pool = pdfcraft_render::RenderPool::new_inline(std::sync::Arc::new(b"not a pdf".to_vec()), Default::default()); + pool.set_queue(vec![pdfcraft_render::RenderRequest { page: 0, scale: 2.0, tag: 2000, ..Default::default() }]); + pool + }; + let mut mb = MoveBackground { + key: Some((0, 0, 0)), + renderer: Some(pool()), + texture: texture("background"), + item_renderer: Some(pool()), + item: texture("item"), + tag: Some(2000), + from: Some([0.0, 0.0, 10.0, 10.0]), + ..Default::default() + }; + assert!(mb.ready(), "both rasters from the earlier scale are in"); + assert!(!mb.receive(&ctx, 2000), "the new scale's renders fail"); + assert!(mb.texture.is_none() && mb.item.is_none(), "no raster is left to draw the moving item from"); + assert!(!mb.ready() && !mb.pending()); + } + /// #4245: the page counts the spacing after a line's last character too, so a line that fits /// only without it wraps in the saved text and must wrap in the editor. #[test] diff --git a/crates/ui-egui/tests/it/content.rs b/crates/ui-egui/tests/it/content.rs index 0d7253707e..0353a7d2b3 100644 --- a/crates/ui-egui/tests/it/content.rs +++ b/crates/ui-egui/tests/it/content.rs @@ -609,6 +609,39 @@ fn dragged_text_follows_the_pointer_before_release() { assert_ne!(added(&h)[0].content.rect(), rect, "releasing moves it"); } +/// #4464: what a dragged text box covered in its first place stays there; only the text follows +/// the pointer. The page has a red square under the text's first place and nothing elsewhere. +#[test] +fn dragged_text_leaves_what_it_covered_behind() { + let _gpu = gpu(); + let content = "1 0 0 rg 20 60 240 70 re f"; + let pdf = format!( + "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >> endobj\n3 0 obj << /Type /Page /Parent 2 0 R /Contents 4 0 R >> endobj\n4 0 obj << /Length {} >> stream\n{content}\nendstream endobj\ntrailer << /Root 1 0 R >>\n%%EOF", + content.len() + ); + let mut h = harness_bytes(pdf.into_bytes()); + let (rect, from, to, end) = drag_added_text(&mut h); + assert!(rect[0] > 20.0 && rect[2] < 260.0 && rect[1] > 60.0 && rect[3] < 130.0, "the text starts on the red square: {rect:?}"); + if let Ok(dir) = std::env::var("PDFCRAFT_SHOTS") { + h.render().unwrap().save(format!("{dir}/added-text-drag-over-red.png")).unwrap(); + } + let pixels = h.render().unwrap(); + let red = |r: egui::Rect| { + let (x0, y0, x1, y1) = (r.left().max(0.0) as u32, r.top().max(0.0) as u32, r.right() as u32, r.bottom() as u32); + (y0..y1.min(pixels.height())) + .flat_map(|y| (x0..x1.min(pixels.width())).map(move |x| (x, y))) + .filter(|(x, y)| matches!(pixels.get_pixel(*x, *y).0, [r, g, b, _] if r > 200 && g < 80 && b < 80)) + .count() + }; + let (old, new) = ink_at(&mut h, from, to); + assert!(new > 20 && old == 0, "the text follows the pointer ({old} old, {new} new)"); + assert!(red(from) > 100, "the red square stays where the text was ({} red pixels)", red(from)); + assert_eq!(red(to), 0, "and doesn't come along with the text"); + h.drop_at(end); + h.run_steps(3); + assert_ne!(added(&h)[0].content.rect(), rect, "releasing moves it"); +} + /// After release the text stays in its new place while the page is re-rendered with the move, /// instead of jumping back to where it was for those frames. #[test]