forked from 1b-agent/opencadstudio-upstream
fix(hatch): deselect refresh, boundary offset, fill-click
Three hatch fixes: - Selected hatches are tinted, but the hatch model cache and GPU upload were keyed only on geometry_epoch, so Esc cleared the selection set without repainting — the hatch stayed blue (#71). Key both on selection_generation too. - add_hatch stored the boundary entity in local render space, so the boundary wire re-projected `world_offset` away from the fill on offset (large-coordinate) drawings. Build it in WCS. - Clicking a hatch only selected it on its boundary wire, not its fill. Two causes: visible_hatches_for_click used a narrower block-visibility test than the render path (fill drawn but not clickable), and point_in_polygon closed every NaN sub-path — degrading the explicit 2-point edge encoding of a boundary into zero-area loops. Mirror the render's visibility test and only auto-close sub-paths of >=3 verts. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
ebf12780c5
commit
6b4272521c
4 files changed
+82
-37
No files matched your search
+27
-24
@@ -344,45 +344,48 @@ fn world_to_screen(world: Vec3, view_proj: Mat4, bounds: Rectangle) -> Point {
|
||||
/// end of one sub-path and the start of the next. Each sub-path with at
|
||||
/// least 2 finite vertices contributes its segments to the parity flip.
|
||||
fn point_in_polygon(p: Point, poly: &[Point]) -> bool {
|
||||
// Ray-cast crossing test for a single edge a→b.
|
||||
fn cross(p: Point, a: Point, b: Point, inside: &mut bool) {
|
||||
if (a.y > p.y) != (b.y > p.y)
|
||||
&& p.x < (b.x - a.x) * (p.y - a.y) / (b.y - a.y) + a.x
|
||||
{
|
||||
*inside = !*inside;
|
||||
}
|
||||
}
|
||||
|
||||
let mut inside = false;
|
||||
let mut prev: Option<Point> = None;
|
||||
let mut path_start: Option<Point> = None;
|
||||
// Vertices in the current sub-path. A boundary can be encoded either as a
|
||||
// ring (`[v0,v1,v2,v3]`, needs an implicit closing edge) or as an explicit
|
||||
// edge list (`[v0,v1, NaN, v1,v2, NaN, …]`, already closed). Only close a
|
||||
// sub-path that is a real ring (≥3 verts); closing a 2-point explicit edge
|
||||
// would add a degenerate back-edge that cancels its own crossing.
|
||||
let mut count = 0usize;
|
||||
let close = |prev: Option<Point>, path_start: Option<Point>, count: usize, inside: &mut bool| {
|
||||
if count >= 3 {
|
||||
if let (Some(pv), Some(sv)) = (prev, path_start) {
|
||||
cross(p, pv, sv, inside);
|
||||
}
|
||||
}
|
||||
};
|
||||
for &pt in poly {
|
||||
if !pt.x.is_finite() || !pt.y.is_finite() {
|
||||
// Close the just-finished sub-path before starting the next.
|
||||
if let (Some(prev_v), Some(start_v)) = (prev, path_start) {
|
||||
if ((prev_v.y > p.y) != (start_v.y > p.y))
|
||||
&& (p.x
|
||||
< (start_v.x - prev_v.x) * (p.y - prev_v.y) / (start_v.y - prev_v.y)
|
||||
+ prev_v.x)
|
||||
{
|
||||
inside = !inside;
|
||||
}
|
||||
}
|
||||
close(prev, path_start, count, &mut inside);
|
||||
prev = None;
|
||||
path_start = None;
|
||||
count = 0;
|
||||
continue;
|
||||
}
|
||||
if let Some(prev_v) = prev {
|
||||
if ((prev_v.y > p.y) != (pt.y > p.y))
|
||||
&& (p.x < (pt.x - prev_v.x) * (p.y - prev_v.y) / (pt.y - prev_v.y) + prev_v.x)
|
||||
{
|
||||
inside = !inside;
|
||||
}
|
||||
cross(p, prev_v, pt, &mut inside);
|
||||
} else {
|
||||
path_start = Some(pt);
|
||||
}
|
||||
prev = Some(pt);
|
||||
count += 1;
|
||||
}
|
||||
// Close the final sub-path.
|
||||
if let (Some(prev_v), Some(start_v)) = (prev, path_start) {
|
||||
if ((prev_v.y > p.y) != (start_v.y > p.y))
|
||||
&& (p.x
|
||||
< (start_v.x - prev_v.x) * (p.y - prev_v.y) / (start_v.y - prev_v.y) + prev_v.x)
|
||||
{
|
||||
inside = !inside;
|
||||
}
|
||||
}
|
||||
close(prev, path_start, count, &mut inside);
|
||||
inside
|
||||
}
|
||||
|
||||
|
||||
+41
-7
@@ -662,7 +662,10 @@ pub struct Scene {
|
||||
/// is handled at draw time via `hatch_skip_flags` in the pipeline,
|
||||
/// not at build time — that lets the GPU buffer stay stable across
|
||||
/// pan/zoom while still skipping out-of-view hatches.
|
||||
hatch_cache: RefCell<Option<(u64, Arc<Vec<HatchModel>>)>>,
|
||||
/// Keyed by `(geometry_epoch, selection_generation)` — selected hatches
|
||||
/// are tinted, so a select/deselect must rebuild even when the geometry
|
||||
/// is unchanged (issue #71).
|
||||
hatch_cache: RefCell<Option<(u64, u64, Arc<Vec<HatchModel>>)>>,
|
||||
/// Cached wipeout fill models, keyed by geometry_epoch. Same
|
||||
/// reasoning as `hatch_cache`.
|
||||
wipeout_cache: RefCell<Option<(u64, Arc<Vec<HatchModel>>)>>,
|
||||
@@ -1874,14 +1877,17 @@ impl Scene {
|
||||
pub(super) fn hatch_models_arc(&self) -> Arc<Vec<HatchModel>> {
|
||||
{
|
||||
let cache = self.hatch_cache.borrow();
|
||||
if let Some((cached_epoch, ref arc)) = *cache {
|
||||
if cached_epoch == self.geometry_epoch {
|
||||
if let Some((cached_epoch, cached_sel, ref arc)) = *cache {
|
||||
if cached_epoch == self.geometry_epoch
|
||||
&& cached_sel == self.selection_generation
|
||||
{
|
||||
return Arc::clone(arc);
|
||||
}
|
||||
}
|
||||
}
|
||||
let arc = Arc::new(self.synced_hatch_models());
|
||||
*self.hatch_cache.borrow_mut() = Some((self.geometry_epoch, Arc::clone(&arc)));
|
||||
*self.hatch_cache.borrow_mut() =
|
||||
Some((self.geometry_epoch, self.selection_generation, Arc::clone(&arc)));
|
||||
arc
|
||||
}
|
||||
|
||||
@@ -1969,11 +1975,29 @@ impl Scene {
|
||||
/// and was causing the wrong hatch to be selected on click).
|
||||
pub fn visible_hatches_for_click(&self) -> HashMap<Handle, HatchModel> {
|
||||
let layout_block = self.current_layout_block_handle();
|
||||
let model_block = self.model_space_block_handle();
|
||||
let layer_hidden = |layer: &str| {
|
||||
self.document
|
||||
.layers
|
||||
.get(layer)
|
||||
.map(|l| l.flags.off || l.flags.frozen)
|
||||
.unwrap_or(false)
|
||||
};
|
||||
self.hatches
|
||||
.iter()
|
||||
.filter_map(|(&h, m)| {
|
||||
let owner = self.document.get_entity(h)?.common().owner_handle;
|
||||
if self.belongs_to_visible_block(h, owner, layout_block) {
|
||||
let c = self.document.get_entity(h)?.common();
|
||||
if c.invisible || layer_hidden(&c.layer) {
|
||||
return None;
|
||||
}
|
||||
// Mirror `synced_hatch_models`' visibility test (which drives
|
||||
// the fill render) so anything drawn is also clickable on its
|
||||
// fill, not just its boundary wire. The model-space fallback
|
||||
// matters when the layout block handle differs from the
|
||||
// entity's owner (issue: hatch fill not selectable).
|
||||
if self.belongs_to_visible_block(h, c.owner_handle, layout_block)
|
||||
|| self.belongs_to_visible_block(h, c.owner_handle, model_block)
|
||||
{
|
||||
Some((h, m.clone()))
|
||||
} else {
|
||||
None
|
||||
@@ -4326,7 +4350,17 @@ impl Scene {
|
||||
model.pattern,
|
||||
crate::scene::hatch_model::HatchPattern::Solid
|
||||
);
|
||||
let [wx, wy] = model.world_origin;
|
||||
// The boundary points arrive in local render space (world_offset
|
||||
// already subtracted). The stored DXF entity must hold WCS, so add the
|
||||
// offset back — otherwise the boundary wire, re-projected through the
|
||||
// normal entity path, lands `world_offset` away from the fill.
|
||||
let off = if self.current_layout == "Model" {
|
||||
self.world_offset
|
||||
} else {
|
||||
[0.0; 3]
|
||||
};
|
||||
let wx = model.world_origin[0] + off[0];
|
||||
let wy = model.world_origin[1] + off[1];
|
||||
let verts: Vec<Vector2> = model
|
||||
.boundary
|
||||
.iter()
|
||||
|
||||
@@ -119,7 +119,10 @@ pub struct Pipeline {
|
||||
/// Last `(geometry_epoch, camera_generation)` value for which GPU buffers
|
||||
/// were uploaded. We re-upload when either side changes — pan/zoom bumps
|
||||
/// camera_generation, which triggers re-culling and a fresh upload.
|
||||
pub cached_epoch: (u64, u64),
|
||||
/// `(geometry_epoch, camera_generation, selection_generation)` of the
|
||||
/// last static-buffer upload. Selection is tracked so the selected-hatch
|
||||
/// tint refreshes on select / deselect (issue #71).
|
||||
pub cached_epoch: (u64, u64, u64),
|
||||
/// Content id of the wire buffer currently resident on the GPU (Phase
|
||||
/// 3.2). When the incoming `ViewportData.wire_content_id` matches, the
|
||||
/// world-space wire vertices are unchanged (e.g. a pure pan reused the
|
||||
@@ -899,7 +902,7 @@ impl Pipeline {
|
||||
gpu_face3d_fill: None,
|
||||
gpu_face3d_edges: vec![],
|
||||
viewcube,
|
||||
cached_epoch: (u64::MAX, u64::MAX),
|
||||
cached_epoch: (u64::MAX, u64::MAX, u64::MAX),
|
||||
cached_wire_id: u64::MAX,
|
||||
cached_selection: (u64::MAX, u64::MAX),
|
||||
}
|
||||
|
||||
+9
-4
@@ -221,7 +221,7 @@ impl shader::Primitive for Primitive {
|
||||
let (uo_y, us_y) = uv_crop_axis(sr.y, sr.height);
|
||||
inner.upload_blit_uv(queue, [uo_x, uo_y], [us_x, us_y]);
|
||||
inner.upload_uniforms(queue, &vp.uniforms);
|
||||
let cur_key = (vp.geometry_epoch, vp.camera_generation);
|
||||
let cur_key = (vp.geometry_epoch, vp.camera_generation, vp.selection_generation);
|
||||
let fill_mode = vp.fill_mode;
|
||||
// 3D face fill requires *both* the doc-level FILLMODE *and* the
|
||||
// per-view Solid toggle. Hatches / wipeouts deliberately ignore
|
||||
@@ -229,9 +229,12 @@ impl shader::Primitive for Primitive {
|
||||
// the Wireframe overlay style.
|
||||
let face3d_fill_active = fill_mode && !vp.view_wireframe;
|
||||
if cur_key != inner.cached_epoch {
|
||||
// Static buffers (hatches/images/meshes) only need refresh on
|
||||
// a real geometry change, not on every camera tick.
|
||||
if vp.geometry_epoch != inner.cached_epoch.0 {
|
||||
// Hatches carry a selected-tint, so re-upload on a geometry OR
|
||||
// a selection change (issue #71); images / meshes only need a
|
||||
// geometry change.
|
||||
let geo_changed = vp.geometry_epoch != inner.cached_epoch.0;
|
||||
let sel_changed = vp.selection_generation != inner.cached_epoch.2;
|
||||
if geo_changed || sel_changed {
|
||||
if fill_mode {
|
||||
inner.upload_hatches(device, &vp.hatches[..]);
|
||||
inner.upload_wipeouts(device, &vp.wipeout_hatches[..]);
|
||||
@@ -239,6 +242,8 @@ impl shader::Primitive for Primitive {
|
||||
inner.upload_hatches(device, &[]);
|
||||
inner.upload_wipeouts(device, &[]);
|
||||
}
|
||||
}
|
||||
if geo_changed {
|
||||
inner.upload_images(device, queue, &vp.images[..]);
|
||||
inner.upload_meshes(device, &vp.meshes[..]);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user