mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-01 14:08:23 -06:00
* M7: a moved text box's drag preview shows the box alone (#4464) While an added text box or picture was dragged, the pixels that followed the pointer were cut from the page raster inside the item's box, so whatever the box covered at its first place (a filled shape, a picture, other text) was carried along with it. The drag now renders the item alone too: a copy of the document whose page keeps only the item's own content stream and no annotations, drawn on a transparent page (`pdfcraft_edit::isolate_content`, `Document::added_content_alone`). Those pixels follow the pointer over the page rendered without the item, so what it covered stays put. The preview shows once both rasters are in; until then the item stays where it is, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0175FWj4xFhMhSH3UpMU8DSC * M7: drop the move preview's earlier rasters when a re-render fails (#4464) When the scale changed after both the page-without-item and item-alone rasters were in, and a render at the new scale failed, only the renderers were cleared. The old textures stayed, so the item kept following the pointer from them although the preview had given up. A failure now clears both texture slots too, so the item stays where it is while dragged. The receive step moves into its own method so a test can drive it with a pool whose renders always fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0175FWj4xFhMhSH3UpMU8DSC --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
e391053658
commit
2be6eb1577
@@ -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<Vec<u8>> {
|
||||
|
||||
@@ -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)]
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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<TextEditPreview, String> {
|
||||
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() } })
|
||||
})?
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -118,6 +118,10 @@ pub(crate) struct MoveBackground {
|
||||
key: Option<(u64, usize, usize)>,
|
||||
renderer: Option<pdfcraft_render::RenderPool>,
|
||||
texture: Option<egui::TextureHandle>,
|
||||
/// 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<pdfcraft_render::RenderPool>,
|
||||
item: Option<egui::TextureHandle>,
|
||||
tag: Option<u64>,
|
||||
/// 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<egui::TextureId>,
|
||||
pointer: Option<Pos2>,
|
||||
) {
|
||||
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<Pos2>) {
|
||||
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]
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user