More region select fixes

This commit is contained in:
Skyler Lehmkuhl 2026-03-06 05:52:20 -05:00
parent f97e61751f
commit bc7d997cff
4 changed files with 145 additions and 14 deletions

View File

@ -596,6 +596,80 @@ mod tests {
extracted.validate();
}
/// Selection entirely inside a filled face: the original face must survive.
///
/// Before this fix, `insert_edge_both_connected` would overwrite F1's
/// `outer_half_edge` with the spur back-chain, causing F1 to lose its
/// rectangle boundary and visually disappear.
#[test]
fn rect_inner_region_select_preserves_face() {
use crate::shape::ShapeColor;
let red = ShapeColor::rgb(255, 0, 0);
let mut dcel = Dcel::new();
// Build a rectangle: (100,100)-(300,200)
let tl = Point::new(100.0, 100.0);
let tr = Point::new(300.0, 100.0);
let br = Point::new(300.0, 200.0);
let bl = Point::new(100.0, 200.0);
dcel.insert_stroke(&[
line_cubic(tl, tr), line_cubic(tr, br),
line_cubic(br, bl), line_cubic(bl, tl),
], None, None, 1.0);
// Simulate paint-bucket: create face for rectangle interior and assign fill.
let fq = dcel.find_face_at_point(Point::new(200.0, 150.0));
let f1 = if !fq.cycle_he.is_none() && fq.face.0 == 0 {
dcel.create_face_at_cycle(fq.cycle_he)
} else {
fq.face
};
dcel.faces[f1.idx()].fill_color = Some(red);
dcel.validate();
// Confirm F1 has fill
assert!(dcel.faces.iter().skip(1).any(|f| !f.deleted && f.fill_color.is_some()),
"rect face should have fill before selection");
// Selection: (150,130)-(250,170) — entirely inside the rectangle
let sa = Point::new(150.0, 130.0);
let sb = Point::new(250.0, 130.0);
let sc = Point::new(250.0, 170.0);
let sd = Point::new(150.0, 170.0);
let mut region_path = BezPath::new();
region_path.move_to(sa);
region_path.line_to(sb);
region_path.line_to(sc);
region_path.line_to(sd);
region_path.close_path();
let sel_result = dcel.insert_stroke(&[
line_cubic(sa, sb), line_cubic(sb, sc),
line_cubic(sc, sd), line_cubic(sd, sa),
], None, None, 1.0);
dcel.validate();
let boundary_verts = sel_result.new_vertices.clone();
let extracted = dcel.extract_region(&region_path, &boundary_verts);
dcel.validate();
extracted.validate();
// The live DCEL must still have a filled face (the original rectangle).
let live_filled = dcel.faces.iter().enumerate()
.filter(|(i, f)| *i > 0 && !f.deleted && f.fill_color.is_some())
.count();
assert!(live_filled >= 1,
"live DCEL should still have at least one filled face after selection; got {live_filled}");
// The extracted DCEL must also have a filled face (selection interior, inherits fill).
let extracted_filled = extracted.faces.iter().enumerate()
.filter(|(i, f)| *i > 0 && !f.deleted && f.fill_color.is_some())
.count();
assert!(extracted_filled >= 1,
"extracted DCEL should have at least one filled face; got {extracted_filled}");
}
/// Replicate: multiple consecutive region-selects on two rectangles,
/// some of which select empty space. Verifies no crash accumulates.
#[test]

View File

@ -331,6 +331,25 @@ impl Dcel {
// the old face's outer_half_edge.
let old_ohe = self.faces[actual_face.idx()].outer_half_edge;
let fwd_has_old = !old_ohe.is_none() && self.cycle_contains(he_fwd, old_ohe);
let bwd_has_old = !fwd_has_old && !old_ohe.is_none() && self.cycle_contains(he_bwd, old_ohe);
if !fwd_has_old && !bwd_has_old {
// Neither new cycle contains the face's existing outer boundary.
// This happens when the edge closes a loop entirely within the face
// (all selection vertices are isolated/floating, never connected to the
// face's real boundary). Don't overwrite outer_half_edge — it still
// correctly points to the face's real boundary (e.g. the rectangle).
// he_fwd = the enclosed interior (new face F2); he_bwd = reverse
// traversal of the selection boundary (stays in actual_face).
self.assign_cycle_face(he_bwd, actual_face);
let new_face = self.alloc_face();
self.faces[new_face.idx()].fill_color = self.faces[actual_face.idx()].fill_color;
self.faces[new_face.idx()].image_fill = self.faces[actual_face.idx()].image_fill;
self.faces[new_face.idx()].fill_rule = self.faces[actual_face.idx()].fill_rule;
self.faces[new_face.idx()].outer_half_edge = he_fwd;
self.assign_cycle_face(he_fwd, new_face);
return (edge_id, new_face);
}
let (he_old_cycle, he_new_cycle) = if fwd_has_old {
(he_fwd, he_bwd)

View File

@ -201,6 +201,16 @@ impl Selection {
}
}
/// Select a face by ID only, without adding boundary edges or vertices.
///
/// Use this when the geometry lives in a separate DCEL (e.g. region selection's
/// `selected_dcel`) so we don't add stale edge/vertex IDs to the selection.
pub fn select_face_id_only(&mut self, face_id: FaceId) {
if !face_id.is_none() && face_id.0 != 0 {
self.selected_faces.insert(face_id);
}
}
/// Select a face and all its boundary edges + vertices.
pub fn select_face(&mut self, face_id: FaceId, dcel: &Dcel) {
if face_id.is_none() || face_id.0 == 0 || dcel.face(face_id).deleted {

View File

@ -1090,7 +1090,7 @@ impl egui_wgpu::CallbackTrait for VelloCallback {
// Render selected DCEL from active region selection (with transform)
if let Some(ref region_sel) = self.ctx.region_selection {
let sel_transform = camera_transform * region_sel.transform;
let sel_transform = overlay_transform * region_sel.transform;
lightningbeam_core::renderer::render_dcel(
&region_sel.selected_dcel,
&mut scene,
@ -1105,6 +1105,27 @@ impl egui_wgpu::CallbackTrait for VelloCallback {
scene
};
// Render region selection fill into the overlay scene.
// In HDR mode the main scene-building block returns an empty scene (only layer content
// goes through the HDR pipeline), so we must add the selected-DCEL fill here so it
// appears underneath the stipple overlay. In legacy mode the render_dcel call inside
// the block already handled this, but running it again is harmless since `scene` would
// be a fresh empty scene only in HDR mode.
if USE_HDR_COMPOSITING {
if let Some(ref region_sel) = self.ctx.region_selection {
let sel_transform = overlay_transform * region_sel.transform;
let mut image_cache = shared.image_cache.lock().unwrap();
lightningbeam_core::renderer::render_dcel(
&region_sel.selected_dcel,
&mut scene,
sel_transform,
1.0,
&self.ctx.document,
&mut image_cache,
);
}
}
// Render drag preview objects with transparency
if let (Some(delta), Some(active_layer_id)) = (self.ctx.drag_delta, self.ctx.active_layer_id) {
if let Some(layer) = self.ctx.document.get_layer(&active_layer_id) {
@ -1478,18 +1499,6 @@ impl egui_wgpu::CallbackTrait for VelloCallback {
_ => {}
}
// 2c. Draw active region selection boundary
if let Some(ref region_sel) = self.ctx.region_selection {
// Draw the region boundary as a dashed outline
let boundary_color = Color::from_rgba8(255, 150, 0, 150);
scene.stroke(
&Stroke::new(1.0).with_dashes(0.0, &[6.0, 4.0]),
overlay_transform,
boundary_color,
None,
&region_sel.region_path,
);
}
// 3. Draw rectangle creation preview
if let lightningbeam_core::tool::ToolState::CreatingRectangle { ref start_point, ref current_point, centered, constrain_square, .. } = self.ctx.tool_state {
@ -2744,6 +2753,12 @@ impl StagePane {
None => return, // No active layer
};
// Revert any active region selection on mouse press before borrowing the document
// immutably, so the two selection modes don't coexist.
if self.rsp_primary_pressed(ui) {
Self::revert_region_selection_static(shared);
}
let active_layer = match shared.action_executor.document().get_layer(&active_layer_id) {
Some(layer) => layer,
None => return,
@ -4069,8 +4084,10 @@ impl StagePane {
// Mouse down: start region selection
if self.rsp_drag_started(response) {
// Revert any existing uncommitted region selection
// Revert any existing uncommitted region selection, and clear the
// regular selection so both selection modes don't coexist.
Self::revert_region_selection_static(shared);
shared.selection.clear();
match *shared.region_select_mode {
RegionSelectMode::Rectangle => {
@ -4264,6 +4281,17 @@ impl StagePane {
shared.selection.clear();
// Populate global selection with the faces from the extracted DCEL so
// property panels and other tools can see what is selected. We add face
// IDs only (no boundary edges/vertices) because the boundary geometry
// lives in selected_dcel, not in the live DCEL.
for (i, face) in selected_dcel.faces.iter().enumerate() {
if face.deleted || i == 0 { continue; }
if face.fill_color.is_some() || face.image_fill.is_some() {
shared.selection.select_face_id_only(lightningbeam_core::dcel::FaceId(i as u32));
}
}
// Store region selection state with extracted DCEL
*shared.region_selection = Some(lightningbeam_core::selection::RegionSelection {
region_path,