diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 5231524b8..d2ac06a35 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -152,7 +152,7 @@ pub fn specs() -> Vec { "Move Object", [], None, - "{kind: chart|image|shape|slicer|control, id, at?: \"C3\", dx?, dy?, width?, height?}", + "{kind: chart|image|shape|slicer|control, id, at?: \"C3\", dx?, dy?, width?, height? (each at most 10,000 points, or the object's current size when a file made it larger)}", has_doc, move_object ), @@ -933,10 +933,10 @@ fn chart_set(s: &mut Session, p: &Json) -> Result { c.anchor.cell = at; } if let Some(w) = f64_param(p, "width") { - c.anchor.width = w.clamp(20.0, 10000.0) as f32; + c.anchor.width = w.clamp(20.0, size_cap(c.anchor.width)) as f32; } if let Some(h) = f64_param(p, "height") { - c.anchor.height = h.clamp(20.0, 10000.0) as f32; + c.anchor.height = h.clamp(20.0, size_cap(c.anchor.height)) as f32; } if let Some(v) = f64_param(p, "dx") { c.anchor.dx = v as f32; @@ -1250,6 +1250,18 @@ fn delete_object(s: &mut Session, p: &Json) -> Result { /// Largest offset (points) `object.move` accepts from the anchor cell. const MAX_OBJECT_OFFSET: f64 = 100_000.0; +/// Largest width or height (points) `object.move` and `chart.set` give an object that is not +/// already larger: see [`size_cap`]. +const MAX_OBJECT_SIZE: f64 = 10_000.0; + +/// The most a command may make a dimension that is `current` now: [`MAX_OBJECT_SIZE`], or the +/// current size when a file made the object larger (the XLSX reader accepts anchors far past the +/// limit). Such an object keeps its size when it is moved and shrinks by what a resize asks, but +/// it never grows further, so hostile input is still capped. Never NaN: `f64::max` drops one. +fn size_cap(current: f32) -> f64 { + MAX_OBJECT_SIZE.max(f64::from(current)) +} + fn move_object(s: &mut Session, p: &Json) -> Result { let (sheet, o) = object_param(s, p, "object.move")?; let at = cell_param(s, p, "at")?; @@ -1265,10 +1277,10 @@ fn move_object(s: &mut Session, p: &Json) -> Result { a.dy = v.clamp(-MAX_OBJECT_OFFSET, MAX_OBJECT_OFFSET) as f32; } if let Some(v) = f64_param(p, "width") { - a.width = v.clamp(1.0, 10000.0) as f32; + a.width = v.clamp(1.0, size_cap(a.width)) as f32; } if let Some(v) = f64_param(p, "height") { - a.height = v.clamp(0.0, 10000.0) as f32; + a.height = v.clamp(0.0, size_cap(a.height)) as f32; } Ok(Json::Null) }) diff --git a/crates/engine/src/tests/objects.rs b/crates/engine/src/tests/objects.rs index 77425e6c4..20a4ab20e 100644 --- a/crates/engine/src/tests/objects.rs +++ b/crates/engine/src/tests/objects.rs @@ -331,3 +331,61 @@ fn a_copied_sheets_form_controls_take_free_ids_too() { assert_eq!(ids.len(), 6, "every control and shape id is distinct across both sheets: {ids:?}"); assert_eq!(ids.iter().filter(|&&i| i == u32::MAX).count(), 1, "only the file's slider keeps the largest id"); } + +/// A file's anchors are not held to the 10,000 points `object.move` gives a new size. A picture +/// a file made 12,000 wide keeps that width when it is moved, shrinks by what a resize asks +/// (11,990 stays 11,990, no jump to 10,000) and never grows further; a dimension the file kept +/// within the limit is still capped there. +#[test] +fn an_oversized_imported_object_keeps_its_size_when_moved_and_shrinks_only_by_a_resize() { + let mut s = s(); + let image = super::add_image(&mut s, "B2", AnchorMode::MoveOnly); + // The file's picture is 12,000 by 300: set on the model, then written and read back as a file. + { + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + let a = wb.sheet_mut(0).unwrap().object_anchor_mut(ObjRef::new(ObjKind::Image, image)).unwrap(); + a.width = 12_000.0; + a.height = 300.0; + } + let saved = s.execute("file.saveBytes", json!({"format": "xlsx"})).unwrap(); + s.execute("file.open", json!({"name": "wide.xlsx", "base64": saved["base64"]})).unwrap(); + let o = ObjRef::new(ObjKind::Image, sheet(&s, 0).images[0].id); + let size = |s: &Session| { + let a = sheet(s, 0).object_anchor(o).unwrap(); + (a.width.round(), a.height.round()) + }; + assert_eq!(size(&s), (12_000.0, 300.0), "the reader keeps the file's size"); + s.execute("object.move", json!({"kind": "image", "id": o.id, "at": "D5", "dx": 3, "dy": 4})).unwrap(); + let a = sheet(&s, 0).object_anchor(o).unwrap(); + assert_eq!((a.cell, a.dx, a.dy), (at("D5"), 3.0, 4.0)); + assert_eq!(size(&s), (12_000.0, 300.0), "a move without a size keeps the file's size"); + s.execute("object.move", json!({"kind": "image", "id": o.id, "width": 12_010})).unwrap(); + assert_eq!(size(&s), (12_000.0, 300.0), "the object does not grow past what the file gave"); + s.execute("object.move", json!({"kind": "image", "id": o.id, "width": 11_990})).unwrap(); + assert_eq!(size(&s), (11_990.0, 300.0), "a resize shrinks by what it asks, with no jump to 10,000"); + s.execute("object.move", json!({"kind": "image", "id": o.id, "width": 50_000, "height": 50_000})).unwrap(); + assert_eq!(size(&s), (11_990.0, 10_000.0), "hostile sizes are capped: at the current width, and at 10,000 for a height the file kept within it"); +} + +/// `chart.set` follows the same rule: the Format Chart pane's Size fields send the unchanged +/// dimension back with the edited one, so a chart a file made 12,000 wide keeps that width when +/// its height is edited, shrinks by what is asked and never grows further. +#[test] +fn chart_set_keeps_a_chart_size_a_file_made_larger_than_the_limit() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [[1], [2], [3]]})).unwrap(); + let chart = id_of(&s.execute("insert.chart", json!({"range": "A1:A3", "type": "column", "at": "C2"})).unwrap(), "chart"); + // The file's chart is 12,000 wide (set on the model, as a loaded file would). + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + wb.sheet_mut(0).unwrap().charts.iter_mut().find(|c| c.id == chart).unwrap().anchor.width = 12_000.0; + let size = |s: &Session| { + let c = sheet(s, 0).charts.iter().find(|c| c.id == chart).unwrap(); + (c.anchor.width, c.anchor.height) + }; + s.execute("chart.set", json!({"chart": chart, "width": 12_000, "height": 250})).unwrap(); + assert_eq!(size(&s), (12_000.0, 250.0), "the unchanged width sent back with a new height stays"); + s.execute("chart.set", json!({"chart": chart, "width": 11_990})).unwrap(); + assert_eq!(size(&s), (11_990.0, 250.0), "a resize shrinks by what it asks"); + s.execute("chart.set", json!({"chart": chart, "width": 50_000, "height": 50_000})).unwrap(); + assert_eq!(size(&s), (11_990.0, 10_000.0), "hostile sizes are capped at the current width and at 10,000"); +} diff --git a/crates/model/src/sheet.rs b/crates/model/src/sheet.rs index 243e1e918..0ad6320d1 100644 --- a/crates/model/src/sheet.rs +++ b/crates/model/src/sheet.rs @@ -730,6 +730,7 @@ mod tests { series: vec![], legend: LegendPos::default(), data_labels: false, + data_table: None, gridlines: false, style: 0, x_title: None, @@ -737,7 +738,6 @@ mod tests { source: None, by_rows: false, pivot: None, - data_table: None, }), ObjKind::Image => s.images.push(Image { id, anchor, data: Arc::new(vec![]), mime: String::new(), alt: String::new() }), ObjKind::Shape => s.shapes.push(Shape { diff --git a/crates/ui-egui/src/grid/interact.rs b/crates/ui-egui/src/grid/interact.rs index 289f76206..65214813b 100644 --- a/crates/ui-egui/src/grid/interact.rs +++ b/crates/ui-egui/src/grid/interact.rs @@ -987,11 +987,19 @@ fn drag_object(app: &mut SheetApp, geo: &Geo, sh: &Sheet, p: Pos2) { }; // Re-anchor at the cell under the new top-left. let nc = CellRef::new(sh.row_at(y0 as f64), sh.col_at(x0 as f64)); - let params = json!({ + let mut params = json!({ "kind": obj.kind.name(), "id": obj.id, "at": nc.a1(), "dx": x0 - sh.col_left(nc.col) as f32, "dy": y0 - sh.row_top(nc.row) as f32, - "width": x1 - x0, "height": y1 - y0, }); + // Only a handle changes the size. A body move sends none, so the engine keeps whatever the + // object has: a file can make it larger than `object.move` accepts for a new size, and + // moving it must not shrink it to that limit. + if handle.is_some() + && let Some(p) = params.as_object_mut() + { + p.insert("width".into(), json!(x1 - x0)); + p.insert("height".into(), json!(y1 - y0)); + } let revision = app.session.active().map(|d| d.revision); let _ = app.session.run("object.move", params); if stepped { diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 69dbbe99f..9675d2a95 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -308,13 +308,29 @@ fn format_chart(app: &mut SheetApp, ui: &mut Ui) { ui.collapsing("Size", |ui| { // Keep the value being typed or dragged between frames; reloading it from the chart every frame threw the edit away. let draft = egui::Id::unique(("gridcraft.chart_size_draft", id)); - let (mut w, mut h) = ui.data_mut(|d| d.get_temp::<(f32, f32)>(draft)).unwrap_or((chart.anchor.width, chart.anchor.height)); - let a = ui.add(egui::DragValue::new(&mut w).prefix("Width ").range(40.0..=4000.0)); - let b = ui.add(egui::DragValue::new(&mut h).prefix("Height ").range(40.0..=4000.0)); + let current = (chart.anchor.width, chart.anchor.height); + let (mut w, mut h) = ui.data_mut(|d| d.get_temp::<(f32, f32)>(draft)).unwrap_or(current); + // A file can make a chart larger than the 4,000 points a field otherwise reaches. The + // range keeps such a size, so the field shows it instead of clamping it to 4,000. + let range = |v: f32| 40.0..=v.max(4000.0); + let a = ui.add(egui::DragValue::new(&mut w).prefix("Width ").range(range(current.0))); + let b = ui.add(egui::DragValue::new(&mut h).prefix("Height ").range(range(current.1))); if a.drag_stopped() || a.lost_focus() || b.drag_stopped() || b.lost_focus() { ui.data_mut(|d| d.remove::<(f32, f32)>(draft)); if !ui.input(|i| i.key_pressed(egui::Key::Escape)) { - app.run_or_alert("chart.set", json!({"chart": id, "width": w, "height": h})); + // Only an edited dimension is sent: the other keeps what the chart has, however + // large a file made it. + let mut params = serde_json::Map::new(); + params.insert("chart".into(), json!(id)); + if w != current.0 { + params.insert("width".into(), json!(w)); + } + if h != current.1 { + params.insert("height".into(), json!(h)); + } + if params.len() > 1 { + app.run_or_alert("chart.set", Json::Object(params)); + } } } else if a.dragged() || a.has_focus() || b.dragged() || b.has_focus() { ui.data_mut(|d| d.insert_temp(draft, (w, h))); diff --git a/crates/ui-egui/tests/it/chart_data_table.rs b/crates/ui-egui/tests/it/chart_data_table.rs index aceeb0ae9..a94e5a16b 100644 --- a/crates/ui-egui/tests/it/chart_data_table.rs +++ b/crates/ui-egui/tests/it/chart_data_table.rs @@ -1,6 +1,7 @@ use egui::accesskit::Role; use egui_kittest::kittest::Queryable; use gridcraft_engine::Session; +use gridcraft_engine::model::{ObjKind, ObjRef}; use serde_json::json; #[test] @@ -10,7 +11,7 @@ fn format_chart_pane_controls_the_engine_data_table() { session.execute("range.setValues", json!({"range": "A1", "values": [["Quarter", "North", "South"], ["Q1", 10, 30], ["Q2", 30, 10]]})).unwrap(); let id = session.execute("insert.chart", json!({"range": "A1:C3", "type": "column", "at": "E3"})).unwrap()["chart"].as_u64().unwrap() as u32; let mut h = crate::support::harness(session, 4); - h.state_mut().select_object_id(Some(id)); + h.state_mut().selected_object = Some(ObjRef::new(ObjKind::Chart, id)); h.state_mut().grid.pane = Some("formatChart".into()); h.run_steps(2); h.get_by_role_and_label(Role::DisclosureTriangle, "Data Table").click(); diff --git a/crates/ui-egui/tests/it/chart_resize.rs b/crates/ui-egui/tests/it/chart_resize.rs index 2ae8707df..36934fbcb 100644 --- a/crates/ui-egui/tests/it/chart_resize.rs +++ b/crates/ui-egui/tests/it/chart_resize.rs @@ -118,6 +118,51 @@ fn dragging_the_size_field_resizes_the_chart() { assert!(w1 > w0 + 20.0, "width went from {w0} to {w1}"); } +/// Makes the chart 12,000 wide and reopens the workbook from its XLSX bytes, the way a file can +/// carry a chart larger than the 4,000 points the Size fields otherwise reach; returns its id. +fn imported_oversized_chart(h: &mut Harness, id: u32) -> u32 { + { + let wb = std::sync::Arc::make_mut(&mut h.state_mut().session.doc_mut().unwrap().wb); + wb.sheet_mut(0).unwrap().charts.iter_mut().find(|c| c.id == id).unwrap().anchor.width = 12_000.0; + } + let saved = h.state_mut().run("file.saveBytes", json!({"format": "xlsx"})).unwrap(); + h.state_mut().run("file.open", json!({"name": "wide.xlsx", "base64": saved["base64"]})).unwrap(); + h.run_steps(4); + let id = h.state().session.doc().unwrap().wb.active().unwrap().charts[0].id; + let (w, hh) = size(h, id); + assert_eq!((w.round(), hh.round()), (12_000.0, 200.0), "the reader keeps the file's size"); + id +} + +/// A chart a file made 12,000 wide shows that width in the Size fields, and a new height sends +/// the height alone, so nothing shrinks the width to the fields' 4,000 reach on the way. +#[test] +fn the_size_fields_keep_an_oversized_imported_chart_width_when_the_height_changes() { + let (mut h, id) = fixture(); + let id = imported_oversized_chart(&mut h, id); + open_size_fields(&mut h, id); + let (width_value, height_field) = { + let mut fields = h.get_all_by_role(Role::SpinButton); + let width = fields.next().unwrap(); + let height = fields.next().unwrap(); + (width.value(), height.rect()) + }; + assert!(width_value.as_deref().is_some_and(|v| v.contains("12000")), "the field shows the file's width: {width_value:?}"); + h.input_mut().events.push(Event::PointerMoved(height_field.center())); + button(&mut h, height_field.center(), true); + button(&mut h, height_field.center(), false); + for digit in ["2", "0", "1"] { + h.input_mut().events.push(Event::Text(digit.into())); + h.run_steps(2); + } + h.key_press(Key::Enter); + h.run_steps(2); + let (w, hh) = size(&h, id); + assert_eq!((w.round(), hh), (12_000.0, 201.0), "the width stays and the height is the typed one"); + let (_, p) = h.state().session.journal.iter().rev().find(|(cmd, _)| cmd == "chart.set").unwrap(); + assert!(p.get("width").is_none(), "a height edit sends no width: {p}"); +} + fn right_click(h: &mut Harness, pos: egui::Pos2, wobble: egui::Vec2) { h.input_mut().events.push(Event::PointerMoved(pos)); h.run_steps(2); diff --git a/crates/ui-egui/tests/it/objects.rs b/crates/ui-egui/tests/it/objects.rs index e3382352d..e0c272817 100644 --- a/crates/ui-egui/tests/it/objects.rs +++ b/crates/ui-egui/tests/it/objects.rs @@ -362,3 +362,65 @@ fn dragging_an_unselected_object_by_its_edge_moves_it() { assert!((after.size() - before.size()).length() < 0.5, "not resized: {before:?} {after:?}"); assert!((after.min - before.min - vec2(30.0, 20.0)).length() < 1.0, "moved with the pointer: {before:?} {after:?}"); } + +fn anchor(h: &Harness, o: ObjRef) -> gridcraft_engine::model::Anchor { + h.state().session.doc().unwrap().wb.active().unwrap().object_anchor(o).unwrap() +} + +/// Makes the fixture's picture 12,000 by 60 and reopens the workbook from its XLSX bytes, the way +/// a file can carry an object wider than the 10,000 points `object.move` gives a new size. +/// Returns the picture as reopened. +fn imported_oversized_picture(h: &mut Harness, o: &Objects) -> ObjRef { + { + let wb = std::sync::Arc::make_mut(&mut h.state_mut().session.doc_mut().unwrap().wb); + let a = wb.sheet_mut(0).unwrap().object_anchor_mut(o.image).unwrap(); + a.width = 12_000.0; + a.height = 60.0; + } + let saved = h.state_mut().run("file.saveBytes", json!({"format": "xlsx"})).unwrap(); + h.state_mut().run("file.open", json!({"name": "wide.xlsx", "base64": saved["base64"]})).unwrap(); + h.run_steps(4); + let pic = ObjRef::new(ObjKind::Image, h.state().session.doc().unwrap().wb.active().unwrap().images[0].id); + let a = anchor(h, pic); + assert_eq!((a.width.round(), a.height.round()), (12_000.0, 60.0), "the reader keeps the file's size: {a:?}"); + pic +} + +/// Dragging the body of such a picture moves it and sends no size, so nothing shrinks it to the +/// limit on the way. +#[test] +fn dragging_the_body_of_an_oversized_imported_object_keeps_its_size() { + let (mut h, o) = fixture(); + let pic = imported_oversized_picture(&mut h, &o); + let before = rect(&h, pic); + // Well inside the body, on the part of the picture in view and clear of the handles' reach. + let grab = before.left_top() + vec2(30.0, 30.0); + click(&mut h, grab); + assert_eq!(h.state().selected_object, Some(pic)); + drag(&mut h, grab, grab + vec2(30.0, 25.0)); + let a = anchor(&h, pic); + assert_eq!((a.width.round(), a.height.round()), (12_000.0, 60.0), "the size is untouched: {a:?}"); + let after = rect(&h, pic); + assert!((after.min - before.min - vec2(30.0, 25.0)).length() < 1.0, "moved with the pointer: {before:?} {after:?}"); + // The drag sent the position only: the engine saw no size to cap. + let (_, p) = h.state().session.journal.iter().rev().find(|(id, _)| id == "object.move").unwrap(); + assert!(p.get("width").is_none() && p.get("height").is_none(), "a body move sends no size: {p}"); +} + +/// A handle drag on such a picture resizes it by the drag only: the left handle moved 10 px to +/// the right leaves it 11,990 wide, with no jump to 10,000. +#[test] +fn a_handle_drag_on_an_oversized_imported_object_resizes_by_the_drag_only() { + let (mut h, o) = fixture(); + let pic = imported_oversized_picture(&mut h, &o); + let before = rect(&h, pic); + click(&mut h, before.left_top() + vec2(30.0, 30.0)); + let grab = Handle::Left.pos(before); + drag(&mut h, grab, grab + vec2(10.0, 0.0)); + // In window points (the grid draws at its zoom): 10 narrower, the left edge 10 further right. + let after = rect(&h, pic); + assert!((before.width() - after.width() - 10.0).abs() < 1.0, "narrower by the drag only: {before:?} {after:?}"); + assert!((after.left() - before.left() - 10.0).abs() < 1.0 && (after.top() - before.top()).abs() < 0.5, "{before:?} {after:?}"); + let a = anchor(&h, pic); + assert!(a.width > 11_900.0 && a.width < 12_000.0 && a.height.round() == 60.0, "no jump to the 10,000 limit: {a:?}"); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 3d1ed8c41..2bc6080b5 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -96,6 +96,9 @@ where `kind` is `chart`, `image`, `shape`, `slicer` or `control` (a form button, button); the same names go in the `kind` parameter of `object.delete`, `object.move`, `object.setAnchorMode` and the `arrange.*` commands. Form controls are worked with `control.setValue` and `control.press`, and listed by `control.list`. +`object.move` and `chart.set` cap `width` and `height` at 10,000 points, or at the object's +current size when a file made it larger: such an object keeps its size when it is moved and +shrinks by what a resize asks, but does not grow further. `selectedChart` remains for existing clients: the id when the selection is a chart, otherwise `null`. With an object selected, `ui.key {key: "Delete"}` (or Backspace) deletes the object instead of clearing cells, and Escape clears the selection.