fix: do not leave a dragged object on its page (#2720)

While a floating picture, shape or text box is dragged, the page raster is
not repainted (the texture cache key does not include the drag state) and a
translucent preview is pasted at the pointer. The user therefore sees the
original copy at full opacity plus the preview, i.e. two copies of the same
object.

Exclude only the dragged object from that page's display pass, and repaint
that page once when the drag starts and once when it ends; the preview keeps
using the previous texture. The rest of the page is left untouched (no
full-page clear, no per-frame repaint).

Adds three tests: a dragged inline object and a dragged floating shape are
left out of their page, and the view key changes when a drag takes an object
off the page.

Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
GavinQiErandBrandon Thomas authored and GitHub committed 2026-10-18 11:49:30 -04:00
1 parent 596176c794
commit fe4faca3f0
5 files changed
+181 -11

No files matched your search

+46 -1
View File
@@ -169,6 +169,9 @@ pub struct DisplayOptions {
pub hide_ink: bool,
/// How tracked changes and comments are marked when `markup` is on (Track Changes Options).
pub revisions: MarkupOptions,
/// Leave this object out of the page: while it is dragged, the preview under the pointer
/// stands in for it, so the page must not also show it where it was.
pub hide_object: Option<wordcraft_doc::Pos>,
}
impl Default for DisplayOptions {
@@ -182,6 +185,39 @@ impl Default for DisplayOptions {
placeholders: false,
hide_ink: false,
revisions: MarkupOptions::default(),
hide_object: None,
}
}
}
/// An object left out of a page's drawing ([`DisplayOptions::hide_object`]).
struct Hidden {
object: wordcraft_doc::Pos,
/// The text box stories it shows (its own and its group members', resolved to the story each
/// one shows): a box's text is drawn as lines of its story, so it goes with the box.
boxes: Vec<u32>,
}
impl Hidden {
/// The object at `object` and the text box stories whose lines belong to it.
fn new(doc: &Document, object: &wordcraft_doc::Pos) -> Hidden {
let boxes = doc
.para(object.story, &object.path)
.and_then(|p| p.object_at(object.off))
.map(|o| o.text_boxes().into_iter().map(|id| doc.text_box_chain(id).and_then(|c| c.first().copied()).unwrap_or(id)).collect())
.unwrap_or_default();
Hidden { object: object.clone(), boxes }
}
fn hides(&self, it: &Placed) -> bool {
let me = &self.object;
match it {
Placed::Object { story, path, off, .. } | Placed::Image { story, path, off, .. } | Placed::Graphic { story, path, off, .. } => {
*story == me.story && *path == me.path && *off == me.off
}
Placed::Shape { object: Some(pos), .. } => pos == me,
Placed::Lines { story: StoryRef::Part(id), .. } => self.boxes.contains(id),
_ => false,
}
}
}
@@ -373,6 +409,7 @@ fn stroke_of(u: Underline) -> Stroke {
/// Draw items for a page.
pub fn page_display(doc: &Document, page: &Page, opts: &DisplayOptions) -> Vec<Draw> {
let mut out = Vec::new();
let hidden = opts.hide_object.as_ref().map(|p| Hidden::new(doc, p));
let ha = if opts.dim_header { 0.45 } else { 1.0 };
let ba = if opts.dim_body { 0.45 } else { 1.0 };
for (items, a) in [(&page.header, ha), (&page.footer, ha), (&page.items, ba)] {
@@ -380,6 +417,10 @@ pub fn page_display(doc: &Document, page: &Page, opts: &DisplayOptions) -> Vec<D
let clips = clipped_cells(items);
let mut done: Vec<u32> = Vec::new();
for it in items.iter() {
// The object being dragged is drawn by its preview instead: leave it out here.
if hidden.as_ref().is_some_and(|h| h.hides(it)) {
continue;
}
if let Placed::Lines { story, path, .. } = it
&& let Some(rect) = cell_clip(&clips, *story, &path.0)
{
@@ -611,7 +652,7 @@ fn item_in(doc: &Document, it: &Placed, opts: &DisplayOptions, alpha: f32, outer
Placed::Image { rect, media, crop, spin, .. } => {
spun(*spin, *rect, vec![Draw::Image { rect: *rect, media: media.clone(), crop: *crop, alpha, alt: String::new() }], out)
}
Placed::Shape { rect, kind, fill, stroke, stroke_width, effects, freeform, spin, extra } => {
Placed::Shape { rect, kind, fill, stroke, stroke_width, effects, freeform, spin, extra, .. } => {
let mut items = Vec::new();
let fx = effects_in(*effects, spin.within(outer));
shape_draws(*rect, *kind, *fill, *stroke, *stroke_width, fx, freeform.as_deref(), extra, opts, &mut items);
@@ -1079,6 +1120,10 @@ fn lines(
if c.obj_h <= 0.0 {
continue;
}
// A dragged inline object is left out too: its preview stands in for it.
if opts.hide_object.as_ref().is_some_and(|p| p.story == story && p.path == *path && p.off == c.start) {
continue;
}
let cx = x + line.cl_left(k).unwrap_or(0.0);
let obj = para.and_then(|p| p.objects.get(oi));
let rect = inline_rect(obj, cx, base, c.adv, c.obj_h);
+6
View File
@@ -276,6 +276,9 @@ pub enum Placed {
spin: Spin,
/// Arrowheads and text warp (see [`wordcraft_doc::connector::ShapeExtra`]).
extra: wordcraft_doc::connector::ShapeExtra,
/// The floating object it draws (a Drawing Canvas's members: the canvas's), for leaving
/// exactly one object out of a page (see [`crate::display::DisplayOptions::hide_object`]).
object: Option<wordcraft_doc::Pos>,
},
/// A floating chart or diagram, drawn from its items inside `rect`. The object is the U+FFFC at
/// byte `off` of paragraph `path` (its alt text).
@@ -2966,6 +2969,7 @@ fn float_items(o: &InlineObject, rect: Rect, outer: (Spin, Point), story: StoryR
vec![Placed::Image { rect, media: media.clone(), crop: *crop, story, path: Path(path.to_vec()), off, spin }]
}
InlineObject::Shape { kind, fill, stroke, stroke_width, effects, freeform, extra, .. } => {
let object = wordcraft_doc::Pos { story, path: Path(path.to_vec()), off };
vec![Placed::Shape {
rect,
kind: *kind,
@@ -2976,6 +2980,7 @@ fn float_items(o: &InlineObject, rect: Rect, outer: (Spin, Point), story: StoryR
freeform: freeform.clone(),
spin,
extra: extra.clone(),
object: Some(object),
}]
}
InlineObject::Graphic { graphic, .. } => {
@@ -3003,6 +3008,7 @@ fn float_items(o: &InlineObject, rect: Rect, outer: (Spin, Point), story: StoryR
freeform: None,
spin,
extra: Default::default(),
object: Some(wordcraft_doc::Pos { story, path: Path(path.to_vec()), off }),
},
Placed::Clip { rect, spin, items: members },
]
+2
View File
@@ -175,6 +175,8 @@ pub(crate) fn symbol_items(pl: &Arc<ParaLayout>, x: f32, y: f32, level: u8, chil
effects: Default::default(),
spin: Default::default(),
extra: Default::default(),
// A heading's symbol, not an object of its own: never left out of a page.
object: None,
};
let white = Border { style: BorderStyle::Single, width: 1.1, color: Some(Rgb(0xFF, 0xFF, 0xFF)), space: 0.0 };
if level >= BODY {
+49 -8
View File
@@ -189,6 +189,18 @@ impl CanvasState {
self.unit_drag = None;
}
/// The object a drag has taken off its page: which page, and which object. `None` unless an
/// object is being **moved** (resizing and turning show the object itself where it is).
///
/// A dragged object's page is not laid out again while it is dragged (that would relayout the
/// whole document every frame), so the page's raster still shows it where it was — the user
/// sees it twice: once in place, once under the pointer. Rendering the page without that one
/// object fixes it (see [`wordcraft_layout::display::DisplayOptions::hide_object`]); erasing
/// the page would make the other objects blink.
fn dragged_off_page(&self) -> Option<(usize, Pos)> {
crate::objects::dragged_off_page(self.obj_drag.as_ref())
}
/// How many pages have a cached rendering.
#[cfg(test)]
pub(crate) fn texture_count(&self) -> usize {
@@ -707,9 +719,10 @@ pub fn sync_fit_zoom(app: &mut WordApp, scale: f32) {
}
/// Fingerprint of everything outside the layout that changes how pages render: the view's
/// marks and markup, dark mode, the page colour and watermark, header/body dimming, the media.
/// marks and markup, dark mode, the page colour and watermark, header/body dimming, the media,
/// and (while a drag lasts) the object left out of the page.
/// Taken once per frame.
fn view_key(app: &WordApp, dim_body: bool) -> u64 {
fn view_key(app: &WordApp, dim_body: bool, hidden: Option<&Pos>) -> u64 {
let mut h = std::collections::hash_map::DefaultHasher::new();
let v = &app.session.view;
(v.marks, app.session.prefs.marks, v.show_markup, v.dark_mode, wordcraft_render::DARK_PAPER, v.hide_ink, v.draw.replay).hash(&mut h);
@@ -718,6 +731,9 @@ fn view_key(app: &WordApp, dim_body: bool) -> u64 {
format!("{:?}{:?}{:?}", app.session.doc.settings.page_color, app.session.doc.settings.page_picture, app.session.doc.settings.watermark)
.hash(&mut h);
app.session.doc.media.len().hash(&mut h);
// A page drawn without a dragged object must not pass for the page with it: the drag starts
// and ends change the key once each, so the page is rendered again then and only then.
hidden.hash(&mut h);
h.finish()
}
@@ -736,7 +752,7 @@ fn page_content_key(page: &Page) -> u64 {
Placed::Fill { rect, color } => format!("{rect:?}{color:?}").hash(&mut h),
Placed::Rule { x0, y0, x1, y1, border } => format!("{x0}{y0}{x1}{y1}{border:?}").hash(&mut h),
Placed::Image { rect, media, spin, .. } => format!("{rect:?}{media}{spin:?}").hash(&mut h),
Placed::Shape { rect, kind, fill, stroke, stroke_width, effects, freeform, spin, extra } => {
Placed::Shape { rect, kind, fill, stroke, stroke_width, effects, freeform, spin, extra, .. } => {
format!("{rect:?}{kind:?}{fill:?}{stroke:?}{stroke_width}{effects:?}{spin:?}{extra:?}").hash(&mut h);
freeform.as_ref().map(|f| std::sync::Arc::as_ptr(f) as usize).hash(&mut h);
}
@@ -1175,7 +1191,10 @@ pub fn show(app: &mut WordApp, ui: &mut Ui) {
let t0 = crate::now_ms();
let mut rects = Vec::with_capacity(geo.rects.len());
let content_origin = ui.min_rect().min;
let view = view_key(app, dim_body);
// The object a move has taken off its page: the page is drawn again without it (or it
// would be on the page where it was and under the pointer at once).
let hidden = app.canvas.dragged_off_page();
let view = view_key(app, dim_body, hidden.as_ref().map(|(_, p)| p));
for (i, pr) in geo.rects.iter().enumerate() {
let sr = pr.translate(content_origin.to_vec2());
let Some(page) = layout.pages.get(i) else { continue };
@@ -1194,10 +1213,15 @@ pub fn show(app: &mut WordApp, ui: &mut Ui) {
let key =
if visible || cached { page_key(app, &layout, i, view, scale_px) ^ if tiled { (tile_height_px as u64) << 32 } else { 0 } } else { 0 };
let mut fresh = cached && app.canvas.textures.get(&i).is_some_and(|(k, _)| *k == key);
// The page a drag took an object off: render it this frame whatever the budget, or the
// raster with the object still on it shows for a frame.
let forced = hidden.as_ref().is_some_and(|(page, _)| *page == i);
// Render before placing the page: a page re-rendered in this frame (a zoom step, an
// edit) then sits on the pixel grid in this frame too, rather than a frame later.
if visible && !tiled && !fresh && (rendered < 2 || !app.canvas.textures.contains_key(&i) && rendered < 4) {
let opts = page_render_options(app, dim_body);
if visible && !tiled && !fresh && (forced || rendered < 2 || !app.canvas.textures.contains_key(&i) && rendered < 4) {
let mut opts = page_render_options(app, dim_body);
// Only on the page it is on: the `Pos` can't match an object of another page.
opts.display.hide_object = hidden.as_ref().filter(|(page, _)| *page == i).map(|(_, p)| p.clone());
let img = wordcraft_render::render_page(&app.session.doc, page, scale_px, &opts);
let ci = page_image(&img);
match app.canvas.textures.get_mut(&i) {
@@ -1251,6 +1275,9 @@ pub fn show(app: &mut WordApp, ui: &mut Ui) {
visible_tile_keys.insert(id);
let fresh = app.canvas.tiles.get(&id).is_some_and(|(k, _)| *k == key);
if !fresh && rendered < 4 {
let mut opts = page_render_options(app, dim_body);
// Only on the page it is on: the `Pos` can't match an object of another page.
opts.display.hide_object = hidden.as_ref().filter(|(page, _)| *page == i).map(|(_, p)| p.clone());
// The tiny subtraction avoids a 1px overshoot from round-tripping f32
// pixels through page points and render_area's ceil() conversion.
let img = wordcraft_render::render_area(
@@ -1261,7 +1288,7 @@ pub fn show(app: &mut WordApp, ui: &mut Ui) {
page.w,
(height_px as f32 - 0.01).max(0.01) / scale_px,
scale_px,
&page_render_options(app, dim_body),
&opts,
);
let ci = page_image(&img);
match app.canvas.tiles.get_mut(&id) {
@@ -3557,7 +3584,7 @@ mod tests {
let at = |a: &mut WordApp, off: usize| {
let layout = a.session.layout();
let c = layout.caret_on(&Pos::body(0, off), 0).unwrap();
let view = view_key(a, false);
let view = view_key(a, false, None);
caret_color_at(a, &layout, c.page, c.x, c.top + c.height / 2.0, view, false)
};
assert_eq!(at(&mut a, 2), Color32::BLACK, "white paper");
@@ -5742,4 +5769,18 @@ mod tests {
fn page_at_index(rects: &[Rect], p: Pos2) -> Option<usize> {
nearest_page(rects, p)
}
/// A page drawn without a dragged object must not pass for the page with it: the drag's start
/// and end change the page's texture key once each, so the page is rendered again then and
/// only then. Without this, the old raster — the object still on it where it was — stays while
/// the preview follows the pointer, and the object shows twice.
#[test]
fn the_view_key_changes_when_a_drag_takes_an_object_off_the_page() {
let doc = wordcraft_doc::Document::from_text("Hello WordCraft");
let app = WordApp::new(wordcraft_engine::Session::new(doc), Default::default());
let object = Pos::body(0, 5);
let hidden = view_key(&app, false, Some(&object));
assert_ne!(hidden, view_key(&app, false, None), "the page without the dragged object is rendered again");
assert_eq!(hidden, view_key(&app, false, Some(&object)), "the same drag keeps the key: no re-render every frame");
}
}
+78 -2
View File
@@ -5,8 +5,9 @@
//! everything else.
//!
//! While dragging, nothing is laid out: the preview is the object drawn alone, once, when the
//! drag starts (#1010: a cut of the page would carry along whatever is behind it). The drop is
//! one command, one relayout.
//! drag starts (#1010: a cut of the page would carry along whatever is behind it). The page it
//! was on is drawn again without it (see [`dragged_off_page`]), so it is not left behind where it
//! was. The drop is one command, one relayout.
//!
//! Moving one snaps it to alignment guides ([`crate::guides`]): the page's edges and centre, the
//! margins and the other floating objects on the page, shown as thin lines while dragging. Alt
@@ -299,6 +300,18 @@ pub fn pointer(app: &mut WordApp, ui: &Ui, resp: &egui::Response, pages: &[Rect]
true
}
/// The object a move has taken off its page (which page, and where it is in the document): the
/// page is rendered again without it while the preview follows the pointer, so the object is not
/// left behind where it was. Only for a move — resizing and turning show the object itself, and
/// dragging a connector's end onto a shape still shows that connector.
pub(crate) fn dragged_off_page(drag: Option<&ObjectDrag>) -> Option<(usize, Pos)> {
let d = drag?;
if !d.drag.moved || d.end.is_some() || d.drag.grab != Grab::Move {
return None;
}
Some((d.object.page, d.object.pos()))
}
/// Snap a moved object's bounds (rotated ones by their turned box) to the guides of the page
/// it's over: the page's edges and centre, its margins, the other floating objects there.
fn snap_move(layout: &DocLayout, d: &mut ObjectDrag, scale: f32) {
@@ -966,4 +979,67 @@ mod tests {
button(&h, end, false);
steps(&mut h, 3);
}
/// A 100×60 pt rectangle filled E2EFDA: [`fill_pixels`] counts the page's pixels of that colour.
fn filled_shape() -> InlineObject {
InlineObject::Shape {
kind: ShapeKind::Rectangle,
w: 100.0,
h: 60.0,
fill: Some(wordcraft_doc::props::Rgb(0xE2, 0xEF, 0xDA)),
stroke: None,
stroke_width: 0.0,
float: Default::default(),
story: None,
freeform: None,
effects: Default::default(),
extra: Default::default(),
}
}
/// How many pixels of the first page have the fill colour: how many copies of the shape the
/// page shows.
fn fill_pixels(app: &mut WordApp, opts: &wordcraft_render::RenderOptions) -> usize {
let layout = app.session.layout();
let img = wordcraft_render::render_page(&app.session.doc, &layout.pages[0], 1.0, opts);
img.to_straight().as_chunks::<4>().0.iter().filter(|p| (p[0], p[1], p[2]) == (0xE2, 0xEF, 0xDA)).count()
}
/// A shape dragged on the page is left out of it: the preview under the pointer stands in for
/// it, so the page must not draw it where it was as well (`DisplayOptions::hide_object`).
///
/// Judged by pixels: the page raster has none of its fill colour once it is left out, and a
/// thousand or so while it isn't. Without this, "it stays where it was while it is dragged"
/// would come back with nothing to catch it.
#[test]
fn a_dragged_inline_object_is_left_out_of_its_page() {
let (mut app, hit) = app_with(filled_shape());
let pos = hit.pos();
let on = wordcraft_render::RenderOptions::default();
let before = fill_pixels(&mut app, &on);
assert!(before > 1000, "the shape is on its page to begin with: {before} px");
let mut hidden = on.clone();
hidden.display.hide_object = Some(pos);
assert_eq!(fill_pixels(&mut app, &hidden), 0, "a dragged object is left out of its page");
}
/// The same for an object that is already floating: it is drawn as a `Placed::Shape` of its
/// own rather than inside a line, so leaving it out goes by the object the shape carries.
#[test]
fn a_dragged_floating_shape_is_left_out_of_its_page() {
let (mut app, hit) = app_with(filled_shape());
let pos = hit.pos();
let end = Pos { off: pos.off + wordcraft_doc::para::OBJ.len_utf8(), ..pos.clone() };
let _ = app.run("select.range", serde_json::json!({"anchor": pos, "focus": end}));
app.run("arrange.bounds", serde_json::json!({"page": 0, "x": 100.0, "y": 150.0})).expect("floats it");
let hit = app.session.layout().object(&pos, 0).expect("laid out");
assert!(hit.floating(), "arrange.bounds makes it float");
let on = wordcraft_render::RenderOptions::default();
let before = fill_pixels(&mut app, &on);
assert!(before > 1000, "the floating shape is on its page to begin with: {before} px");
let mut hidden = on.clone();
hidden.display.hide_object = Some(pos);
assert_eq!(fill_pixels(&mut app, &hidden), 0, "a dragged floating shape is left out of its page");
}
}