Fix DCEL selection bugs

This commit is contained in:
Skyler Lehmkuhl 2026-03-05 19:55:39 -05:00
parent 63a8080e60
commit f97e61751f
8 changed files with 567 additions and 10 deletions

View File

@ -132,6 +132,10 @@ pub struct ActionExecutor {
/// Maximum number of actions to keep in undo stack /// Maximum number of actions to keep in undo stack
max_undo_depth: usize, max_undo_depth: usize,
/// Monotonically increasing counter, bumped on every `execute` call.
/// Used to detect whether any actions were taken during a region selection.
epoch: u64,
} }
impl ActionExecutor { impl ActionExecutor {
@ -144,6 +148,7 @@ impl ActionExecutor {
undo_stack: Vec::new(), undo_stack: Vec::new(),
redo_stack: Vec::new(), redo_stack: Vec::new(),
max_undo_depth: 100, // Default: keep last 100 actions max_undo_depth: 100, // Default: keep last 100 actions
epoch: 0,
} }
} }
@ -188,6 +193,9 @@ impl ActionExecutor {
// Clear redo stack (new action invalidates redo history) // Clear redo stack (new action invalidates redo history)
self.redo_stack.clear(); self.redo_stack.clear();
// Bump epoch so region selections can detect that an action occurred
self.epoch = self.epoch.wrapping_add(1);
// Add to undo stack // Add to undo stack
self.undo_stack.push(action); self.undo_stack.push(action);
@ -289,6 +297,16 @@ impl ActionExecutor {
self.redo_stack.len() self.redo_stack.len()
} }
/// Return the current action epoch.
///
/// The epoch is a monotonically increasing counter that is bumped every
/// time `execute` is called. It is never decremented on undo/redo, so
/// callers can record it at a point in time and later compare to detect
/// whether any action was executed in the interim.
pub fn epoch(&self) -> u64 {
self.epoch
}
/// Clear all undo/redo history /// Clear all undo/redo history
pub fn clear_history(&mut self) { pub fn clear_history(&mut self) {
self.undo_stack.clear(); self.undo_stack.clear();
@ -335,6 +353,7 @@ impl ActionExecutor {
// 3. Push to undo stack (both succeeded) // 3. Push to undo stack (both succeeded)
self.redo_stack.clear(); self.redo_stack.clear();
self.epoch = self.epoch.wrapping_add(1);
self.undo_stack.push(action); self.undo_stack.push(action);
// Limit undo stack size // Limit undo stack size

View File

@ -9,7 +9,7 @@
//! Faces are classified by which vertices they touch — no sampling needed. //! Faces are classified by which vertices they touch — no sampling needed.
use super::{Dcel, EdgeId, FaceId, VertexId}; use super::{Dcel, EdgeId, FaceId, VertexId};
use kurbo::{BezPath, Point, Shape}; use kurbo::{BezPath, CubicBez, Point, Shape};
/// Vertex classification relative to the region boundary. /// Vertex classification relative to the region boundary.
#[derive(Clone, Copy, PartialEq, Eq, Debug)] #[derive(Clone, Copy, PartialEq, Eq, Debug)]
@ -171,6 +171,88 @@ impl Dcel {
} }
} }
} }
/// Merge a modified `selected` DCEL back into `self` (the snapshot DCEL).
///
/// Call this when deselecting after uncommitted modifications (e.g. a move).
/// `self` is the snapshot; inside-vertex positions and edge curves are
/// updated from `selected`, then the invisible region boundary edges are
/// removed to restore a unified DCEL.
pub fn merge_back_from_selected(
&mut self,
selected: &Dcel,
inside_vertices: &[VertexId],
_boundary_vertices: &[VertexId],
region_edge_ids: &[EdgeId],
) {
// Step 1: Copy inside vertex positions from selected_dcel.
for &vid in inside_vertices {
if vid.idx() < selected.vertices.len() && !selected.vertices[vid.idx()].deleted {
if vid.idx() < self.vertices.len() && !self.vertices[vid.idx()].deleted {
self.vertices[vid.idx()].position = selected.vertices[vid.idx()].position;
}
}
}
// Step 2: Update curves for edges whose endpoints changed.
let inside_set: std::collections::HashSet<VertexId> =
inside_vertices.iter().copied().collect();
for i in 0..self.edges.len() {
let e = &self.edges[i];
if e.deleted { continue; }
let [fwd, bwd] = e.half_edges;
let v1 = self.half_edges[fwd.idx()].origin;
let v2 = self.half_edges[bwd.idx()].origin;
let v1_inside = inside_set.contains(&v1);
let v2_inside = inside_set.contains(&v2);
if v1_inside && v2_inside {
// Fully inside — copy transformed curve from selected.
if i < selected.edges.len() && !selected.edges[i].deleted {
self.edges[i].curve = selected.edges[i].curve;
}
} else if v1_inside || v2_inside {
// Mixed edge: one endpoint moved, one is boundary (fixed).
// Approximate with a straight line to preserve connectivity.
let p0 = self.vertices[v1.idx()].position;
let p3 = self.vertices[v2.idx()].position;
self.edges[i].curve = line_to_cubic_pts(p0, p3);
}
}
// Step 3: Remove invisible region boundary edges to heal face splits.
for &eid in region_edge_ids {
if eid.idx() < self.edges.len() && !self.edges[eid.idx()].deleted {
self.remove_edge(eid);
}
}
// Step 4: Clean up stale inner half-edge references left by remove_edge.
self.repair_stale_inner_half_edges();
}
/// Remove stale `inner_half_edges` entries that point to deleted half-edges.
///
/// After `remove_edge` calls, some faces' hole-boundary references can point
/// to half-edges that no longer exist. This cleans them up.
pub fn repair_stale_inner_half_edges(&mut self) {
for face in self.faces.iter_mut() {
face.inner_half_edges.retain(|&he| {
he.idx() < self.half_edges.len() && !self.half_edges[he.idx()].deleted
});
}
}
}
/// Convert two points into a degenerate cubic (straight-line cubic bezier).
fn line_to_cubic_pts(p0: Point, p3: Point) -> CubicBez {
CubicBez::new(
p0,
Point::new(p0.x + (p3.x - p0.x) / 3.0, p0.y + (p3.y - p0.y) / 3.0),
Point::new(p0.x + 2.0 * (p3.x - p0.x) / 3.0, p0.y + 2.0 * (p3.y - p0.y) / 3.0),
p3,
)
} }
#[cfg(test)] #[cfg(test)]
@ -178,6 +260,46 @@ mod tests {
use super::*; use super::*;
use kurbo::{CubicBez, Point}; use kurbo::{CubicBez, Point};
/// After `extract_region`, verify no face in `dcel` has half-edges from
/// both Inside-classified and Outside-classified vertices.
fn assert_no_face_straddles(dcel: &Dcel, region: &BezPath, boundary_verts: &[VertexId]) {
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
enum VClass { Inside, Outside, Boundary }
let classes: Vec<VClass> = dcel.vertices.iter().enumerate().map(|(i, v)| {
if v.deleted { return VClass::Outside; }
let vid = VertexId(i as u32);
if boundary_verts.contains(&vid) {
VClass::Boundary
} else if region.winding(v.position) != 0 {
VClass::Inside
} else {
VClass::Outside
}
}).collect();
for i in 1..dcel.faces.len() {
let face = &dcel.faces[i];
if face.deleted || face.outer_half_edge.is_none() { continue; }
let boundary = dcel.face_boundary(FaceId(i as u32));
let mut has_inside = false;
let mut has_outside = false;
for he_id in &boundary {
let vid = dcel.half_edges[he_id.idx()].origin;
match classes[vid.idx()] {
VClass::Inside => has_inside = true,
VClass::Outside => has_outside = true,
VClass::Boundary => {}
}
}
assert!(
!(has_inside && has_outside),
"face {} straddles boundary (has both Inside and Outside vertices)",
i
);
}
}
fn line_cubic(a: Point, b: Point) -> CubicBez { fn line_cubic(a: Point, b: Point) -> CubicBez {
CubicBez::new( CubicBez::new(
a, a,
@ -190,6 +312,41 @@ mod tests {
) )
} }
#[test]
fn extract_region_no_face_straddles() {
let mut dcel = Dcel::new();
// Two horizontal lines spanning the region boundary.
// After insert_stroke splits them, boundary vertices appear at x=50.
let a0 = Point::new(0.0, 30.0);
let a1 = Point::new(100.0, 30.0);
let b0 = Point::new(0.0, 70.0);
let b1 = Point::new(100.0, 70.0);
let va0 = dcel.alloc_vertex(a0);
let va1 = dcel.alloc_vertex(a1);
let vb0 = dcel.alloc_vertex(b0);
let vb1 = dcel.alloc_vertex(b1);
dcel.insert_edge(va0, va1, FaceId(0), line_cubic(a0, a1));
dcel.insert_edge(vb0, vb1, FaceId(0), line_cubic(b0, b1));
let mut region = BezPath::new();
region.move_to(Point::new(-10.0, -10.0));
region.line_to(Point::new(50.0, -10.0));
region.line_to(Point::new(50.0, 110.0));
region.line_to(Point::new(-10.0, 110.0));
region.close_path();
// Simulate boundary splitting: mid-points at x=50
let v_amid = dcel.alloc_vertex(Point::new(50.0, 30.0));
let v_bmid = dcel.alloc_vertex(Point::new(50.0, 70.0));
let boundary_verts = vec![v_amid, v_bmid];
let extracted = dcel.extract_region(&region, &boundary_verts);
assert_no_face_straddles(&dcel, &region, &boundary_verts);
assert_no_face_straddles(&extracted, &region, &boundary_verts);
}
#[test] #[test]
fn extract_region_basic() { fn extract_region_basic() {
let mut dcel = Dcel::new(); let mut dcel = Dcel::new();
@ -232,6 +389,97 @@ mod tests {
assert_eq!(ext_edges, 0, "edges span boundary → removed from extracted"); assert_eq!(ext_edges, 0, "edges span boundary → removed from extracted");
} }
/// Replicate: draw filled rect (0,0)-(100,100), region-select top-left corner
/// with selection (-50,-50)-(50,50). Goal: validate passes on both DCELs.
#[test]
fn rect_corner_region_select() {
let mut dcel = Dcel::new();
// Draw rectangle: W=(0,0) X=(100,0) Y=(100,100) Z=(0,100)
let w = Point::new(0.0, 0.0);
let x = Point::new(100.0, 0.0);
let y = Point::new(100.0, 100.0);
let z = Point::new(0.0, 100.0);
dcel.insert_stroke(&[
line_cubic(w, x),
line_cubic(x, y),
line_cubic(y, z),
line_cubic(z, w),
], None, None, 1.0);
// Simulate paint-bucket: find the inner face cycle and create F1
let face_query = dcel.find_face_at_point(Point::new(50.0, 50.0));
eprintln!("paint bucket face query: {:?}, he: {:?}", face_query.face, face_query.cycle_he);
if !face_query.cycle_he.is_none() && face_query.face.0 == 0 {
let f1 = dcel.create_face_at_cycle(face_query.cycle_he);
eprintln!("created F{} for interior", f1.0);
}
dcel.validate();
eprintln!("DCEL valid after rect+paint");
// Selection boundary: A=(-50,-50) B=(50,-50) C=(50,50) D=(-50,50)
let a = Point::new(-50.0, -50.0);
let b = Point::new(50.0, -50.0);
let c = Point::new(50.0, 50.0);
let d = Point::new(-50.0, 50.0);
let mut region_path = BezPath::new();
region_path.move_to(a);
region_path.line_to(b);
region_path.line_to(c);
region_path.line_to(d);
region_path.close_path();
let stroke_result = dcel.insert_stroke(&[
line_cubic(a, b),
line_cubic(b, c),
line_cubic(c, d),
line_cubic(d, a),
], None, None, 1.0);
eprintln!("after insert_stroke: {} new edges, {} new faces",
stroke_result.new_edges.len(), stroke_result.new_faces.len());
dcel.validate();
eprintln!("DCEL valid after selection boundary insert");
// Dump face structure after insert_stroke
for (i, face) in dcel.faces.iter().enumerate() {
if face.deleted { continue; }
let ohe = face.outer_half_edge;
if ohe.is_none() {
eprintln!(" F{i}: outer_he=NONE (unbounded)");
} else {
let cycle = dcel.walk_cycle(ohe);
let origins: Vec<_> = cycle.iter().map(|&he| {
let vid = dcel.half_edges[he.idx()].origin;
dcel.vertices[vid.idx()].position
}).collect();
eprintln!(" F{i}: {} HEs, vertices: {:?}", cycle.len(), origins);
}
}
let boundary_verts: Vec<VertexId> = stroke_result.new_vertices.clone();
let mut extracted = dcel.extract_region(&region_path, &boundary_verts);
eprintln!("after extract_region");
// Check face_boundary on all non-F0 faces
for (i, face) in dcel.faces.iter().enumerate() {
if face.deleted || i == 0 { continue; }
let b = dcel.face_boundary(FaceId(i as u32));
eprintln!(" self F{i}: {} boundary HEs", b.len());
}
for (i, face) in extracted.faces.iter().enumerate() {
if face.deleted || i == 0 { continue; }
let b = extracted.face_boundary(FaceId(i as u32));
eprintln!(" extracted F{i}: {} boundary HEs", b.len());
}
dcel.validate();
extracted.validate();
eprintln!("both DCELs valid after extract_region");
}
#[test] #[test]
fn extract_region_with_boundary_vertices() { fn extract_region_with_boundary_vertices() {
let mut dcel = Dcel::new(); let mut dcel = Dcel::new();
@ -269,4 +517,154 @@ mod tests {
assert_eq!(ext_edges, 1, "extracted should have left edge"); assert_eq!(ext_edges, 1, "extracted should have left edge");
assert_eq!(self_edges, 1, "self should have right edge"); assert_eq!(self_edges, 1, "self should have right edge");
} }
/// Replicate: two rectangles, region-select an area that doesn't intersect either.
/// The selected area is to the left of both rectangles, selecting empty space.
/// Goal: validate passes on both DCELs and walk_cycle doesn't infinite-loop.
#[test]
fn rect_empty_region_select() {
let mut dcel = Dcel::new();
// Rect 1: (220,353)-(290,379)
let r1a = Point::new(220.0, 353.0);
let r1b = Point::new(290.0, 353.0);
let r1c = Point::new(290.0, 379.0);
let r1d = Point::new(220.0, 379.0);
dcel.insert_stroke(&[
line_cubic(r1a, r1b), line_cubic(r1b, r1c),
line_cubic(r1c, r1d), line_cubic(r1d, r1a),
], None, None, 1.0);
// Rect 2: (304,383)-(551,483)
let r2a = Point::new(304.0, 383.0);
let r2b = Point::new(551.0, 383.0);
let r2c = Point::new(551.0, 483.0);
let r2d = Point::new(304.0, 483.0);
dcel.insert_stroke(&[
line_cubic(r2a, r2b), line_cubic(r2b, r2c),
line_cubic(r2c, r2d), line_cubic(r2d, r2a),
], None, None, 1.0);
dcel.validate();
// Selection: (136,332)-(208,347) — entirely to the left of both rectangles
let sel_min = Point::new(136.0, 332.0);
let sel_max = Point::new(208.0, 347.0);
let a = sel_min;
let b = Point::new(sel_max.x, sel_min.y);
let c = sel_max;
let d = Point::new(sel_min.x, sel_max.y);
let mut region_path = BezPath::new();
region_path.move_to(a);
region_path.line_to(b);
region_path.line_to(c);
region_path.line_to(d);
region_path.close_path();
let stroke_result = dcel.insert_stroke(&[
line_cubic(a, b), line_cubic(b, c),
line_cubic(c, d), line_cubic(d, a),
], None, None, 1.0);
dcel.validate();
let boundary_verts = stroke_result.new_vertices.clone();
let extracted = dcel.extract_region(&region_path, &boundary_verts);
// Walk all face boundaries in both DCELs — must not infinite-loop
for (i, face) in dcel.faces.iter().enumerate() {
if face.deleted { continue; }
let _b = dcel.face_boundary(FaceId(i as u32));
for &ihe in &face.inner_half_edges {
if !ihe.is_none() && !dcel.half_edges[ihe.idx()].deleted {
let _c = dcel.walk_cycle(ihe);
}
}
}
for (i, face) in extracted.faces.iter().enumerate() {
if face.deleted { continue; }
let _b = extracted.face_boundary(FaceId(i as u32));
for &ihe in &face.inner_half_edges {
if !ihe.is_none() && !extracted.half_edges[ihe.idx()].deleted {
let _c = extracted.walk_cycle(ihe);
}
}
}
dcel.validate();
extracted.validate();
}
/// Replicate: multiple consecutive region-selects on two rectangles,
/// some of which select empty space. Verifies no crash accumulates.
#[test]
fn multiple_region_selects() {
fn make_rect_dcel() -> Dcel {
let mut dcel = Dcel::new();
let r1a = Point::new(220.0, 353.0);
let r1b = Point::new(290.0, 353.0);
let r1c = Point::new(290.0, 379.0);
let r1d = Point::new(220.0, 379.0);
dcel.insert_stroke(&[
line_cubic(r1a, r1b), line_cubic(r1b, r1c),
line_cubic(r1c, r1d), line_cubic(r1d, r1a),
], None, None, 1.0);
let r2a = Point::new(304.0, 383.0);
let r2b = Point::new(551.0, 383.0);
let r2c = Point::new(551.0, 483.0);
let r2d = Point::new(304.0, 483.0);
dcel.insert_stroke(&[
line_cubic(r2a, r2b), line_cubic(r2b, r2c),
line_cubic(r2c, r2d), line_cubic(r2d, r2a),
], None, None, 1.0);
dcel
}
let selects: &[(f64, f64, f64, f64)] = &[
(438.0, 326.0, 614.0, 445.0), // intersects rect 2
(181.0, 312.0, 407.0, 448.0), // intersects both
(460.0, 293.0, 516.0, 507.0), // intersects rect 2
(167.0, 311.0, 380.0, 437.0), // intersects both
(690.0, 360.0, 703.0, 448.0), // empty (right)
(136.0, 332.0, 208.0, 347.0), // empty (left) — crashes in original bug
];
for (min_x, min_y, max_x, max_y) in selects {
let mut dcel = make_rect_dcel();
let a = Point::new(*min_x, *min_y);
let b = Point::new(*max_x, *min_y);
let c = Point::new(*max_x, *max_y);
let d = Point::new(*min_x, *max_y);
let mut region_path = BezPath::new();
region_path.move_to(a);
region_path.line_to(b);
region_path.line_to(c);
region_path.line_to(d);
region_path.close_path();
let stroke_result = dcel.insert_stroke(&[
line_cubic(a, b), line_cubic(b, c),
line_cubic(c, d), line_cubic(d, a),
], None, None, 1.0);
dcel.validate();
let boundary_verts = stroke_result.new_vertices.clone();
let extracted = dcel.extract_region(&region_path, &boundary_verts);
// Walk all face boundaries and inner cycles — must not infinite-loop
for (i, face) in extracted.faces.iter().enumerate() {
if face.deleted { continue; }
let _b = extracted.face_boundary(FaceId(i as u32));
for &ihe in &face.inner_half_edges {
if !ihe.is_none() && !extracted.half_edges[ihe.idx()].deleted {
let _ = extracted.walk_cycle(ihe);
}
}
}
dcel.validate();
extracted.validate();
}
}
} }

View File

@ -122,7 +122,7 @@ impl Dcel {
self.insert_edge_both_connected(he_fwd, he_bwd, v1, v2, edge_id, &curve, face) self.insert_edge_both_connected(he_fwd, he_bwd, v1, v2, edge_id, &curve, face)
} }
_ => { _ => {
self.insert_edge_one_isolated(he_fwd, he_bwd, v1, v2, edge_id, &curve, v1_isolated) self.insert_edge_one_isolated(he_fwd, he_bwd, v1, v2, edge_id, &curve, v1_isolated, face)
} }
} }
} }
@ -169,6 +169,7 @@ impl Dcel {
edge_id: EdgeId, edge_id: EdgeId,
curve: &CubicBez, curve: &CubicBez,
v1_is_isolated: bool, v1_is_isolated: bool,
face_hint: FaceId,
) -> (EdgeId, FaceId) { ) -> (EdgeId, FaceId) {
let (connected, isolated) = if v1_is_isolated { (v2, v1) } else { (v1, v2) }; let (connected, isolated) = if v1_is_isolated { (v2, v1) } else { (v1, v2) };
@ -187,7 +188,21 @@ impl Dcel {
}; };
let ccw_succ = self.find_ccw_successor(connected, out_angle); let ccw_succ = self.find_ccw_successor(connected, out_angle);
let he_into = self.half_edges[ccw_succ.idx()].prev; let he_into = self.half_edges[ccw_succ.idx()].prev;
let actual_face = self.half_edges[he_into.idx()].face; let angular_face = self.half_edges[he_into.idx()].face;
// If angular ordering disagrees with face_hint, try to find the correct
// sector using find_predecessor_on_face — same logic as insert_edge_both_connected.
let (he_into, ccw_succ, actual_face) = if angular_face != face_hint {
if let Some((alt_into, alt_ccw)) =
self.find_predecessor_on_face(connected, out_angle, face_hint)
{
(alt_into, alt_ccw, face_hint)
} else {
(he_into, ccw_succ, angular_face)
}
} else {
(he_into, ccw_succ, angular_face)
};
// Splice: ... → he_into → [he_out → he_back] → ccw_succ → ... // Splice: ... → he_into → [he_out → he_back] → ccw_succ → ...
self.half_edges[he_into.idx()].next = he_out; self.half_edges[he_into.idx()].next = he_out;
@ -247,6 +262,25 @@ impl Dcel {
debug_assert_eq!(face_v1, face_v2); debug_assert_eq!(face_v1, face_v2);
(into_v1, into_v2, ccw_v1, ccw_v2, face_v1) (into_v1, into_v2, ccw_v1, ccw_v2, face_v1)
} }
} else if face_v1 != face_v2 {
// Angular ordering disagrees between the two endpoints.
// Trust face_hint (midpoint probe) as the authoritative face —
// it correctly determines which face the edge's interior lies in,
// regardless of which angular sector each vertex landed in.
let target = face_hint;
let fix_v1 = if face_v1 == target {
(into_v1, ccw_v1)
} else {
self.find_predecessor_on_face(v1, fwd_angle, target)
.unwrap_or((into_v1, ccw_v1))
};
let fix_v2 = if face_v2 == target {
(into_v2, ccw_v2)
} else {
self.find_predecessor_on_face(v2, bwd_angle, target)
.unwrap_or((into_v2, ccw_v2))
};
(fix_v1.0, fix_v2.0, fix_v1.1, fix_v2.1, target)
} else { } else {
debug_assert_eq!( debug_assert_eq!(
face_v1, face_v2, face_v1, face_v2,

View File

@ -429,6 +429,17 @@ pub struct RegionSelection {
pub transform: Affine, pub transform: Affine,
/// Whether the selection has been committed (via an operation on the selection) /// Whether the selection has been committed (via an operation on the selection)
pub committed: bool, pub committed: bool,
/// Non-boundary vertices that are strictly inside the region (for merge-back).
pub inside_vertices: Vec<VertexId>,
/// Region boundary intersection vertices (for merge-back and fill propagation).
pub boundary_vertices: Vec<VertexId>,
/// IDs of the invisible edges inserted for the region boundary stroke.
/// Removing these during merge-back heals the face splits they created.
pub region_edge_ids: Vec<EdgeId>,
/// Action epoch recorded when this selection was created.
/// Compared against `ActionExecutor::epoch()` on deselect to decide
/// whether merge-back is needed or a clean snapshot restore suffices.
pub action_epoch_at_selection: u64,
} }
#[cfg(test)] #[cfg(test)]

View File

@ -88,6 +88,10 @@ pub struct TestCase {
pub ended_with_panic: bool, pub ended_with_panic: bool,
pub panic_message: Option<String>, pub panic_message: Option<String>,
pub panic_backtrace: Option<String>, pub panic_backtrace: Option<String>,
/// Serialized geometry context at the time of the crash (e.g. DCEL + region path).
/// Populated by set_pending_geometry before risky operations.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub geometry_context: Option<serde_json::Value>,
} }
impl TestCase { impl TestCase {
@ -102,6 +106,7 @@ impl TestCase {
ended_with_panic: false, ended_with_panic: false,
panic_message: None, panic_message: None,
panic_backtrace: None, panic_backtrace: None,
geometry_context: None,
} }
} }

View File

@ -194,17 +194,23 @@ fn main() -> eframe::Result {
let test_mode_is_replaying: std::sync::Arc<std::sync::atomic::AtomicBool> = let test_mode_is_replaying: std::sync::Arc<std::sync::atomic::AtomicBool> =
std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
let test_mode_pending_geometry: std::sync::Arc<std::sync::Mutex<Option<serde_json::Value>>> =
std::sync::Arc::new(std::sync::Mutex::new(None));
#[cfg(debug_assertions)]
let test_mode_panic_snapshot_for_app = test_mode_panic_snapshot.clone(); let test_mode_panic_snapshot_for_app = test_mode_panic_snapshot.clone();
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
let test_mode_pending_event_for_app = test_mode_pending_event.clone(); let test_mode_pending_event_for_app = test_mode_pending_event.clone();
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
let test_mode_is_replaying_for_app = test_mode_is_replaying.clone(); let test_mode_is_replaying_for_app = test_mode_is_replaying.clone();
#[cfg(debug_assertions)]
let test_mode_pending_geometry_for_app = test_mode_pending_geometry.clone();
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
{ {
let panic_snapshot = test_mode_panic_snapshot.clone(); let panic_snapshot = test_mode_panic_snapshot.clone();
let pending_event = test_mode_pending_event.clone(); let pending_event = test_mode_pending_event.clone();
let is_replaying = test_mode_is_replaying.clone(); let is_replaying = test_mode_is_replaying.clone();
let pending_geometry = test_mode_pending_geometry.clone();
let test_dir = directories::ProjectDirs::from("", "", "lightningbeam") let test_dir = directories::ProjectDirs::from("", "", "lightningbeam")
.map(|dirs| dirs.data_dir().join("test_cases")) .map(|dirs| dirs.data_dir().join("test_cases"))
.unwrap_or_else(|| std::path::PathBuf::from("test_cases")); .unwrap_or_else(|| std::path::PathBuf::from("test_cases"));
@ -219,7 +225,7 @@ fn main() -> eframe::Result {
format!("{}", info) format!("{}", info)
}; };
let backtrace = format!("{}", std::backtrace::Backtrace::force_capture()); let backtrace = format!("{}", std::backtrace::Backtrace::force_capture());
test_mode::TestModeState::record_panic(&panic_snapshot, &pending_event, &is_replaying, msg, backtrace, &test_dir); test_mode::TestModeState::record_panic(&panic_snapshot, &pending_event, &is_replaying, &pending_geometry, msg, backtrace, &test_dir);
default_hook(info); default_hook(info);
})); }));
} }
@ -229,7 +235,7 @@ fn main() -> eframe::Result {
options, options,
Box::new(move |cc| { Box::new(move |cc| {
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
let app = EditorApp::new(cc, layouts, theme, test_mode_panic_snapshot_for_app, test_mode_pending_event_for_app, test_mode_is_replaying_for_app); let app = EditorApp::new(cc, layouts, theme, test_mode_panic_snapshot_for_app, test_mode_pending_event_for_app, test_mode_is_replaying_for_app, test_mode_pending_geometry_for_app);
#[cfg(not(debug_assertions))] #[cfg(not(debug_assertions))]
let app = EditorApp::new(cc, layouts, theme); let app = EditorApp::new(cc, layouts, theme);
Ok(Box::new(app)) Ok(Box::new(app))
@ -921,6 +927,7 @@ impl EditorApp {
#[cfg(debug_assertions)] panic_snapshot: std::sync::Arc<std::sync::Mutex<Option<lightningbeam_core::test_mode::TestCase>>>, #[cfg(debug_assertions)] panic_snapshot: std::sync::Arc<std::sync::Mutex<Option<lightningbeam_core::test_mode::TestCase>>>,
#[cfg(debug_assertions)] pending_event: std::sync::Arc<std::sync::Mutex<Option<lightningbeam_core::test_mode::TestEvent>>>, #[cfg(debug_assertions)] pending_event: std::sync::Arc<std::sync::Mutex<Option<lightningbeam_core::test_mode::TestEvent>>>,
#[cfg(debug_assertions)] is_replaying: std::sync::Arc<std::sync::atomic::AtomicBool>, #[cfg(debug_assertions)] is_replaying: std::sync::Arc<std::sync::atomic::AtomicBool>,
#[cfg(debug_assertions)] pending_geometry: std::sync::Arc<std::sync::Mutex<Option<serde_json::Value>>>,
) -> Self { ) -> Self {
let current_layout = layouts[0].layout.clone(); let current_layout = layouts[0].layout.clone();
@ -1114,7 +1121,7 @@ impl EditorApp {
// Debug test mode (F5) // Debug test mode (F5)
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
test_mode: test_mode::TestModeState::new(panic_snapshot, pending_event, is_replaying), test_mode: test_mode::TestModeState::new(panic_snapshot, pending_event, is_replaying, pending_geometry),
// Debug overlay (F3) // Debug overlay (F3)
cursor_cache: custom_cursor::CursorCache::new(), cursor_cache: custom_cursor::CursorCache::new(),

View File

@ -4203,9 +4203,28 @@ impl StagePane {
return; return;
} }
// Capture DCEL snapshot + region path for crash diagnosis (debug builds only)
#[cfg(debug_assertions)]
{
use vello::kurbo::PathEl;
let path_elems: Vec<serde_json::Value> = region_path.elements().iter().map(|el| match el {
PathEl::MoveTo(p) => serde_json::json!({"type": "M", "x": p.x, "y": p.y}),
PathEl::LineTo(p) => serde_json::json!({"type": "L", "x": p.x, "y": p.y}),
PathEl::QuadTo(p1, p2) => serde_json::json!({"type": "Q", "x1": p1.x, "y1": p1.y, "x2": p2.x, "y2": p2.y}),
PathEl::CurveTo(p1, p2, p3) => serde_json::json!({"type": "C", "x1": p1.x, "y1": p1.y, "x2": p2.x, "y2": p2.y, "x3": p3.x, "y3": p3.y}),
PathEl::ClosePath => serde_json::json!({"type": "Z"}),
}).collect();
let geom = serde_json::json!({
"region_path": path_elems,
"dcel_snapshot": serde_json::to_value(&snapshot).unwrap_or(serde_json::Value::Null),
});
shared.test_mode.set_pending_geometry(geom);
}
// Insert region boundary as invisible edges (no stroke style/color) // Insert region boundary as invisible edges (no stroke style/color)
let stroke_result = dcel.insert_stroke(&segments, None, None, 1.0); let stroke_result = dcel.insert_stroke(&segments, None, None, 1.0);
let boundary_verts = stroke_result.new_vertices; let boundary_verts: Vec<_> = stroke_result.new_vertices.clone();
let region_edge_ids: Vec<_> = stroke_result.new_edges.clone();
// Extract the inside portion; self (dcel) keeps the outside + boundary. // Extract the inside portion; self (dcel) keeps the outside + boundary.
let mut selected_dcel = dcel.extract_region(&region_path, &boundary_verts); let mut selected_dcel = dcel.extract_region(&region_path, &boundary_verts);
@ -4224,9 +4243,25 @@ impl StagePane {
if !has_visible { if !has_visible {
// Nothing visible inside — restore snapshot and bail // Nothing visible inside — restore snapshot and bail
*dcel = snapshot; *dcel = snapshot;
#[cfg(debug_assertions)]
shared.test_mode.clear_pending_geometry();
return; return;
} }
// Compute inside_vertices: non-deleted verts in selected_dcel that aren't boundary.
let inside_vertices: Vec<_> = selected_dcel
.vertices
.iter()
.enumerate()
.filter_map(|(i, v)| {
if v.deleted { return None; }
let vid = lightningbeam_core::dcel::VertexId(i as u32);
if !boundary_verts.contains(&vid) { Some(vid) } else { None }
})
.collect();
let action_epoch = shared.action_executor.epoch();
shared.selection.clear(); shared.selection.clear();
// Store region selection state with extracted DCEL // Store region selection state with extracted DCEL
@ -4238,7 +4273,14 @@ impl StagePane {
selected_dcel, selected_dcel,
transform: vello::kurbo::Affine::IDENTITY, transform: vello::kurbo::Affine::IDENTITY,
committed: false, committed: false,
inside_vertices,
boundary_vertices: boundary_verts,
region_edge_ids,
action_epoch_at_selection: action_epoch,
}); });
#[cfg(debug_assertions)]
shared.test_mode.clear_pending_geometry();
} }
/// Revert an uncommitted region selection, restoring the DCEL from snapshot /// Revert an uncommitted region selection, restoring the DCEL from snapshot
@ -4255,11 +4297,26 @@ impl StagePane {
return; return;
} }
// Restore the DCEL from the snapshot taken before boundary insertion let no_actions_taken =
shared.action_executor.epoch() == region_sel.action_epoch_at_selection;
let doc = shared.action_executor.document_mut(); let doc = shared.action_executor.document_mut();
if let Some(AnyLayer::Vector(vl)) = doc.get_layer_mut(&region_sel.layer_id) { if let Some(AnyLayer::Vector(vl)) = doc.get_layer_mut(&region_sel.layer_id) {
if let Some(dcel) = vl.dcel_at_time_mut(region_sel.time) { if let Some(dcel) = vl.dcel_at_time_mut(region_sel.time) {
*dcel = region_sel.dcel_snapshot; if no_actions_taken {
// Nothing changed: restore snapshot cleanly (undo boundary insertion)
*dcel = region_sel.dcel_snapshot;
} else {
// Actions were applied to the selection: merge selected_dcel back
let mut merged = region_sel.dcel_snapshot;
merged.merge_back_from_selected(
&region_sel.selected_dcel,
&region_sel.inside_vertices,
&region_sel.boundary_vertices,
&region_sel.region_edge_ids,
);
*dcel = merged;
}
} }
} }

View File

@ -192,6 +192,9 @@ pub struct TestModeState {
/// Current in-flight event, set before processing. If a panic occurs during /// Current in-flight event, set before processing. If a panic occurs during
/// processing, the panic hook appends this to the saved test case. /// processing, the panic hook appends this to the saved test case.
pub pending_event: Arc<Mutex<Option<TestEvent>>>, pub pending_event: Arc<Mutex<Option<TestEvent>>>,
/// Geometry context set before risky operations (e.g. region select).
/// Shared with the panic hook so crashes include the relevant geometry.
pub pending_geometry: Arc<Mutex<Option<serde_json::Value>>>,
/// Always-on ring buffer of last N events (for crash capture outside test mode) /// Always-on ring buffer of last N events (for crash capture outside test mode)
pub event_ring: VecDeque<TestEvent>, pub event_ring: VecDeque<TestEvent>,
pub ring_start_time: Instant, pub ring_start_time: Instant,
@ -205,7 +208,7 @@ pub struct TestModeState {
} }
impl TestModeState { impl TestModeState {
pub fn new(panic_snapshot: Arc<Mutex<Option<TestCase>>>, pending_event: Arc<Mutex<Option<TestEvent>>>, is_replaying: Arc<AtomicBool>) -> Self { pub fn new(panic_snapshot: Arc<Mutex<Option<TestCase>>>, pending_event: Arc<Mutex<Option<TestEvent>>>, is_replaying: Arc<AtomicBool>, pending_geometry: Arc<Mutex<Option<serde_json::Value>>>) -> Self {
let test_dir = directories::ProjectDirs::from("", "", "lightningbeam") let test_dir = directories::ProjectDirs::from("", "", "lightningbeam")
.map(|dirs| dirs.data_dir().join("test_cases")) .map(|dirs| dirs.data_dir().join("test_cases"))
.unwrap_or_else(|| PathBuf::from("test_cases")); .unwrap_or_else(|| PathBuf::from("test_cases"));
@ -219,6 +222,7 @@ impl TestModeState {
status_message: None, status_message: None,
panic_snapshot, panic_snapshot,
pending_event, pending_event,
pending_geometry,
event_ring: VecDeque::with_capacity(RING_BUFFER_SIZE), event_ring: VecDeque::with_capacity(RING_BUFFER_SIZE),
ring_start_time: Instant::now(), ring_start_time: Instant::now(),
ring_event_count: 0, ring_event_count: 0,
@ -228,6 +232,22 @@ impl TestModeState {
} }
} }
/// Store geometry context for panic capture.
/// Called before risky operations (e.g. region select) so the panic hook
/// can include it in the crash file for easier reproduction.
pub fn set_pending_geometry(&self, context: serde_json::Value) {
if let Ok(mut guard) = self.pending_geometry.try_lock() {
*guard = Some(context);
}
}
/// Clear the pending geometry context (call after the operation succeeds).
pub fn clear_pending_geometry(&self) {
if let Ok(mut guard) = self.pending_geometry.try_lock() {
*guard = None;
}
}
/// Store the current in-flight event for panic capture. /// Store the current in-flight event for panic capture.
/// Called before processing so the panic hook can grab it if processing panics. /// Called before processing so the panic hook can grab it if processing panics.
pub fn set_pending_event(&self, kind: TestEventKind) { pub fn set_pending_event(&self, kind: TestEventKind) {
@ -339,6 +359,7 @@ impl TestModeState {
panic_snapshot: &Arc<Mutex<Option<TestCase>>>, panic_snapshot: &Arc<Mutex<Option<TestCase>>>,
pending_event: &Arc<Mutex<Option<TestEvent>>>, pending_event: &Arc<Mutex<Option<TestEvent>>>,
is_replaying: &Arc<AtomicBool>, is_replaying: &Arc<AtomicBool>,
pending_geometry: &Arc<Mutex<Option<serde_json::Value>>>,
msg: String, msg: String,
backtrace: String, backtrace: String,
test_dir: &PathBuf, test_dir: &PathBuf,
@ -372,6 +393,11 @@ impl TestModeState {
} }
} }
// Attach geometry context if one was set before the crash
if let Ok(mut geom_guard) = pending_geometry.try_lock() {
test_case.geometry_context = geom_guard.take();
}
test_case.ended_with_panic = true; test_case.ended_with_panic = true;
test_case.panic_message = Some(msg); test_case.panic_message = Some(msg);
test_case.panic_backtrace = Some(backtrace); test_case.panic_backtrace = Some(backtrace);