mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-11-01 05:37:23 -06:00
Merge pull request #4912 from dotCooCoo/fix/projective-then-after-finite
Perspective: a homography product that overflows is refused instead of stored or drawn
This commit is contained in:
@@ -52,8 +52,8 @@ pub(super) fn grid_set(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
old.adopt_stored_attachments(d);
|
||||
for (id, plane, depth) in persp::attached_roots(d) {
|
||||
let h = old.homography_at(plane, depth).and_then(|h| h.inverse()).zip(g.homography_at(plane, depth));
|
||||
let (hi, h) = h.ok_or_else(|| EngineError::Other("an object in perspective would leave the edited grid".into()))?;
|
||||
let m = h.then_after(&hi);
|
||||
let m = h.and_then(|(hi, h)| h.checked_then_after(&hi));
|
||||
let m = m.ok_or_else(|| EngineError::Other("an object in perspective would leave the edited grid".into()))?;
|
||||
warp_checked(d, id, &m)?;
|
||||
}
|
||||
}
|
||||
@@ -98,17 +98,15 @@ pub(super) fn roots(s: &Session, p: &Value) -> Result<Vec<NodeId>> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Map `n` with `f`, failing if any point would leave the plane's visible side.
|
||||
/// Map `n` with `h` (type and symbols in perspective keep their art and are drawn through it),
|
||||
/// failing if any point would leave the plane's visible side.
|
||||
/// failing if any point would leave the plane's visible side or a projection would not be finite.
|
||||
fn warp_checked(d: &mut Document, id: NodeId, h: &Homography) -> Result<()> {
|
||||
let n = d.node_mut(id).ok_or(EngineError::NoNode(id))?;
|
||||
let mut pts = vec![];
|
||||
collect_points(n, &mut pts);
|
||||
if pts.iter().any(|q| h.apply(*q).is_none()) {
|
||||
if pts.iter().any(|q| h.apply(*q).is_none()) || !project_node(n, h) {
|
||||
return Err(EngineError::Other("the object would cross the horizon of the perspective plane".into()));
|
||||
}
|
||||
project_node(n, h);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -599,11 +597,14 @@ pub(super) fn edit_text(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let NodeKind::Text(t) = &mut n.kind else { return Err(EngineError::Other("select type in perspective".into())) };
|
||||
// Shown flat where it is drawn: the type moves there and the projection moves the other
|
||||
// way, so the picture stays.
|
||||
let (Some(flat), Some(drawn)) = (t.bounds(), t.bounds().and_then(|b| h.map_rect_bbox(b))) else { return Ok(()) };
|
||||
let Some(flat) = t.bounds() else { return Ok(()) };
|
||||
let cross = || EngineError::Other("the object would cross the horizon of the perspective plane".into());
|
||||
let drawn = h.map_rect_bbox(flat).ok_or_else(cross)?;
|
||||
let shift = vectorcraft_geom::Affine::translate(drawn.center() - flat.center());
|
||||
let moved = h.checked_then_after(&Homography::from_affine(shift.inverse())).ok_or_else(cross)?;
|
||||
t.transform(shift);
|
||||
if let Some(rec) = n.extras_if_any().and_then(|e| e.perspective.as_deref_mut()) {
|
||||
rec.projection = Some(h.then_after(&Homography::from_affine(shift.inverse())).to_array());
|
||||
rec.projection = Some(moved.to_array());
|
||||
rec.editing = true;
|
||||
}
|
||||
Ok(())
|
||||
|
||||
@@ -113,6 +113,39 @@ fn moving_type_in_perspective_moves_its_projection() {
|
||||
let _ = rect;
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn moving_type_whose_projection_would_overflow_is_refused() {
|
||||
let (mut s, _) = persp_session();
|
||||
let id = persp_text(&mut s);
|
||||
let NodeKind::Text(t) = &node(&s, id).kind else { panic!("type") };
|
||||
let flat = t.bounds().unwrap();
|
||||
// A stored projection that draws the type flat where it is, with every entry scaled by `k`: a
|
||||
// document can hold it (its entries are finite) and the type draws, but its product with a
|
||||
// scale by 2 in perspective overflows.
|
||||
let step = grid(&s).transform_map(Plane::Right, 0.0, vectorcraft_geom::Affine::scale(2.0), 0.0).unwrap();
|
||||
let scaled = |e: i32| {
|
||||
let k = 2f64.powf(f64::from(e) / 16.0);
|
||||
vectorcraft_geom::Homography([[k, 0.0, 0.0], [0.0, k, 0.0], [0.0, 0.0, k]])
|
||||
};
|
||||
let finite = |m: &vectorcraft_geom::Homography| m.to_array().iter().all(|v| v.is_finite());
|
||||
let draws = |m: &vectorcraft_geom::Homography| {
|
||||
let drawn = m.map_rect_bbox(flat);
|
||||
drawn.is_some_and(|b| step.map_rect_bbox(b).is_some()) && vectorcraft_geom::Homography::from_array(m.to_array()) == Some(*m)
|
||||
};
|
||||
let e = (0..16 * 1024).find(|e| draws(&scaled(*e)) && !finite(&step.then_after(&scaled(*e)))).unwrap_or_else(|| panic!("{step:?} {flat:?}"));
|
||||
let big = scaled(e);
|
||||
let mut doc = s.doc().unwrap().doc.clone();
|
||||
let mut extras = std::sync::Arc::make_mut(&mut doc).node_mut(id).unwrap().extras_mut();
|
||||
extras.perspective.as_deref_mut().unwrap().projection = Some(big.to_array());
|
||||
drop(extras);
|
||||
s.doc_mut().unwrap().doc = doc;
|
||||
let before = (node(&s, id), undo_len(&s));
|
||||
assert!(s.execute("perspective.transform", &json!({"ids": [id.0], "matrix": [2, 0, 0, 2, 0, 0]})).is_err());
|
||||
assert_eq!((node(&s, id), undo_len(&s)), before, "the document is unchanged");
|
||||
let stored = node(&s, id).extras().perspective.as_deref().and_then(|p| p.projection).unwrap();
|
||||
assert!(stored.iter().all(|v| v.is_finite()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn edit_text_shows_it_flat_in_isolation_then_projects_it_again() {
|
||||
let (mut s, _) = persp_session();
|
||||
@@ -141,6 +174,37 @@ fn edit_text_shows_it_flat_in_isolation_then_projects_it_again() {
|
||||
assert!(s2.execute("perspective.editText", &json!({"id": rect.0})).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn edit_text_whose_flat_view_would_overflow_is_refused() {
|
||||
let (mut s, _) = persp_session();
|
||||
let id = persp_text(&mut s);
|
||||
let NodeKind::Text(t) = &node(&s, id).kind else { panic!("type") };
|
||||
let flat = t.bounds().unwrap();
|
||||
// A stored projection that draws the type 4 times its size, with every entry scaled by `k`: a
|
||||
// document can hold it and the type draws, but the projection that shows it flat where it is
|
||||
// drawn overflows.
|
||||
let scaled = |e: i32| {
|
||||
let k = 2f64.powf(f64::from(e) / 16.0);
|
||||
vectorcraft_geom::Homography([[4.0 * k, 0.0, 0.0], [0.0, 4.0 * k, 0.0], [0.0, 0.0, k]])
|
||||
};
|
||||
let overflows = |m: &vectorcraft_geom::Homography| {
|
||||
let Some(drawn) = m.map_rect_bbox(flat) else { return false };
|
||||
let shift = vectorcraft_geom::Affine::translate(drawn.center() - flat.center());
|
||||
let moved = m.then_after(&vectorcraft_geom::Homography::from_affine(shift.inverse()));
|
||||
vectorcraft_geom::Homography::from_array(m.to_array()) == Some(*m) && moved.to_array().iter().any(|v| !v.is_finite())
|
||||
};
|
||||
let e = (0..16 * 1024).find(|e| overflows(&scaled(*e))).unwrap_or_else(|| panic!("{flat:?}"));
|
||||
let mut doc = s.doc().unwrap().doc.clone();
|
||||
let mut extras = std::sync::Arc::make_mut(&mut doc).node_mut(id).unwrap().extras_mut();
|
||||
extras.perspective.as_deref_mut().unwrap().projection = Some(scaled(e).to_array());
|
||||
drop(extras);
|
||||
s.doc_mut().unwrap().doc = doc;
|
||||
let before = (node(&s, id), undo_len(&s));
|
||||
assert!(s.execute("perspective.editText", &json!({"id": id.0})).is_err());
|
||||
assert_eq!((node(&s, id), undo_len(&s)), before, "the document is unchanged");
|
||||
assert_eq!(s.doc().unwrap().isolation, None, "not in isolation mode");
|
||||
}
|
||||
|
||||
/// Edit Text, which shows the type in isolation mode, is off while an opacity mask is edited.
|
||||
#[test]
|
||||
fn edit_text_is_off_while_an_opacity_mask_is_edited() {
|
||||
|
||||
@@ -99,6 +99,13 @@ impl Homography {
|
||||
Self(out)
|
||||
}
|
||||
|
||||
/// [`Self::then_after`]; `None` when an entry of the product is not finite (an entry
|
||||
/// overflows, or a factor has one that is not finite).
|
||||
pub fn checked_then_after(&self, other: &Self) -> Option<Self> {
|
||||
let c = self.then_after(other);
|
||||
c.is_finite().then_some(c)
|
||||
}
|
||||
|
||||
/// `self` seen through `a`: the map `a ∘ self ∘ a⁻¹` (what `self` becomes when everything it
|
||||
/// maps from and to is moved by `a`). When the product overflows, every entry is formed with
|
||||
/// its powers of two kept apart, and the product is returned divided by the largest magnitude
|
||||
@@ -267,6 +274,19 @@ mod tests {
|
||||
assert!(h().inverse().is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_product_that_overflows_is_none() {
|
||||
// Each factor is finite and invertible, but the product's first entry is 1e400.
|
||||
let big = Homography([[1e200, 0.0, 0.0], [0.0, 1e200, 0.0], [0.0, 0.0, 1e200]]);
|
||||
assert!(Homography::from_array(big.to_array()).is_some());
|
||||
assert_eq!(big.checked_then_after(&big), None);
|
||||
let nan = Homography([[1.0, 0.0, 0.0], [0.0, 1.0, 0.0], [0.0, 0.0, f64::NAN]]);
|
||||
assert_eq!(h().checked_then_after(&nan), None);
|
||||
assert_eq!(h().checked_then_after(&Homography::IDENTITY), Some(h()));
|
||||
let inv = h().inverse().unwrap();
|
||||
assert_eq!(h().checked_then_after(&inv), Some(h().then_after(&inv)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_derivative_that_overflows_is_none() {
|
||||
// The point maps to 1e300, but the derivative there is 1e302 / 1e-8.
|
||||
|
||||
@@ -50,9 +50,31 @@ pub fn warp_node_with(n: &mut Node, f: &dyn Fn(Point) -> Point) {
|
||||
}
|
||||
|
||||
/// [`warp_node_with`] for the projective map `h` (perspective): type and symbol instances keep
|
||||
/// their flat art and are drawn through `h` (composed with the projection they had).
|
||||
pub fn project_node(n: &mut Node, h: &Homography) {
|
||||
/// their flat art and are drawn through `h` (composed with the projection they had). Returns
|
||||
/// `false`, and leaves `n` as it is, when the projection of a type or symbol instance in `n` would
|
||||
/// have an entry that is not finite.
|
||||
pub fn project_node(n: &mut Node, h: &Homography) -> bool {
|
||||
if !projects_finitely(n, h) {
|
||||
return false;
|
||||
}
|
||||
warp_node(n, &|p| h.apply(p).unwrap_or(p), Some(h));
|
||||
true
|
||||
}
|
||||
|
||||
/// Whether every type and symbol instance that [`project_node`] reaches in `n` keeps a projection
|
||||
/// whose entries are all finite.
|
||||
fn projects_finitely(n: &Node, h: &Homography) -> bool {
|
||||
match &n.kind {
|
||||
NodeKind::Text(_) | NodeKind::SymbolInstance { .. } => {
|
||||
let had = n.extras().perspective.as_deref().and_then(|p| p.homography()).unwrap_or(Homography::IDENTITY);
|
||||
h.checked_then_after(&had).is_some()
|
||||
}
|
||||
NodeKind::Layer { children, .. }
|
||||
| NodeKind::Group { children, .. }
|
||||
| NodeKind::Compound { children, .. }
|
||||
| NodeKind::Blend { children, .. } => children.iter().all(|c| projects_finitely(c, h)),
|
||||
_ => true,
|
||||
}
|
||||
}
|
||||
|
||||
fn warp_node(n: &mut Node, f: &dyn Fn(Point) -> Point, h: Option<&Homography>) {
|
||||
@@ -62,7 +84,9 @@ fn warp_node(n: &mut Node, f: &dyn Fn(Point) -> Point, h: Option<&Homography>) {
|
||||
let mut extras = n.extras_mut();
|
||||
let rec = extras.perspective.get_or_insert_with(Default::default);
|
||||
let had = rec.homography().unwrap_or(Homography::IDENTITY);
|
||||
rec.projection = Some(h.then_after(&had).to_array());
|
||||
if let Some(c) = h.checked_then_after(&had) {
|
||||
rec.projection = Some(c.to_array());
|
||||
}
|
||||
return;
|
||||
}
|
||||
let bounds = n.geometric_bounds();
|
||||
|
||||
@@ -449,7 +449,7 @@ impl PerspectiveGrid {
|
||||
pub fn transform_map(&self, plane: Plane, depth: f64, m: Affine, dz: f64) -> Option<Homography> {
|
||||
let h = self.homography_at(plane, depth)?;
|
||||
let to = self.homography_at(plane, depth + dz)?;
|
||||
Some(to.then_after(&Homography::from_affine(m)).then_after(&h.inverse()?))
|
||||
to.checked_then_after(&Homography::from_affine(m))?.checked_then_after(&h.inverse()?)
|
||||
}
|
||||
|
||||
/// Where object `id` is attached (plane, depth): its own record, else the one a grid stored
|
||||
@@ -529,7 +529,7 @@ impl PerspectiveGrid {
|
||||
let sy = if b.height().abs() > 1e-9 { (c.y - a.y) / b.height() } else { sx.abs() * if c.y < a.y { -1.0 } else { 1.0 } };
|
||||
let sx = if b.width().abs() > 1e-9 { sx } else { sy.abs() };
|
||||
let m = Affine::new([sx, 0.0, 0.0, sy, a.x - b.x0 * sx, a.y - b.y0 * sy]);
|
||||
Some(h.then_after(&Homography::from_affine(m)))
|
||||
h.checked_then_after(&Homography::from_affine(m))
|
||||
}
|
||||
|
||||
/// The map that puts flat art drawn at `at` onto `plane` keeping its sizes in plane units (a
|
||||
@@ -544,7 +544,7 @@ impl PerspectiveGrid {
|
||||
let sx = if h.apply(q + Vec2::new(1.0, 0.0))?.x < at.x { -1.0 } else { 1.0 };
|
||||
let sy = if h.apply(q + Vec2::new(0.0, 1.0))?.y < at.y { -1.0 } else { 1.0 };
|
||||
let m = Affine::new([sx, 0.0, 0.0, sy, q.x - sx * at.x, q.y - sy * at.y]);
|
||||
Some(h.then_after(&Homography::from_affine(m)))
|
||||
h.checked_then_after(&Homography::from_affine(m))
|
||||
}
|
||||
|
||||
/// The map that slides art lying on `plane` by the plane-space offset between `from` and `to`.
|
||||
|
||||
@@ -991,6 +991,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur
|
||||
Lock Grid, Lock Station Point, Snap to Grid and Show Rulers (View › Perspective Grid) and Import Perspective Grid Presets (its choice to replace saved presets of the same names) refuse a value other than true, false or null, such as "true", and leave the grid and the saved presets as they were.
|
||||
All Artwork Above and Other Layers (Object › Lock and Object › Hide), Transform Each, Rearrange All Artboards, the width point and width profile commands, Liquify, Puppet Warp, Define Perspective Grid, Attach to Active Plane, Release with Perspective, moves and transforms in perspective, Move Plane and Move Plane to Match Object record no undo step when they change nothing.
|
||||
Move in Perspective (its copy, perpendicular and Snap to Grid choices), Transform in Perspective (its copy), Nudge in Perspective (its ×10 and copy), Draw in Perspective (Snap to Grid), Define Perspective Grid (its reproject choice), Show Perspective Grid, Width Point (Adjust Adjoining Width Points), Liquify (its pen pressure, Simplify and Affect choices) and Puppet Warp (its pins' rest shape) refuse a value other than true, false or null, such as "true", and leave the document and its undo history as they were.
|
||||
Moves and transforms in perspective and Define Perspective Grid with reproject return an error, and leave the document and its undo history as they were, when the projection they would give type or a symbol instance in perspective has a number too large to store. Object › Perspective › Edit Text returns an error, and does not enter isolation mode, when the type cannot be shown flat where it is drawn: when part of it crosses the horizon, or when that view's projection has a number too large to store. Regressions: `a_product_that_overflows_is_none` in `crates/geom/src/projective.rs`, and `moving_type_whose_projection_would_overflow_is_refused` and `edit_text_whose_flat_view_would_overflow_is_refused` in `crates/engine/src/tests/persp_text.rs`.
|
||||
Create Object Mosaic (its columns and rows), Transform Each (its reference point and Random seed) and Rearrange All Artboards (its columns) refuse a fraction, a negative number or a string, such as 2.5, -1 or "4"; 4.0 counts as 4.
|
||||
Move, Rotate, Scale, Reflect, Shear, Transform Again, Nudge and the Transform panel's values (their copy, Scale Strokes & Effects, Scale Corners, Transform Patterns, Transform Objects and area type options), Align, Distribute and Distribute Spacing (Transform Patterns), Object Properties (visible, locked, Isolate Blending and Opacity & Mask Define Knockout Shape) and Live Shape Properties (Invert Pie, Make Sides Equal and relative rounding) refuse a value other than true, false or null, such as "false", and leave the document as it was.
|
||||
Object Properties (its opacity), Live Shape Properties (corner radius and polygon sides), the Transform panel's reference point and Align, Distribute and Distribute Spacing to an artboard (the artboard's number) refuse a value other than a number or null, such as "50", and leave the document as it was.
|
||||
|
||||
Reference in New Issue
Block a user