fix(view): exclude XLine/Ray display segments from ZOOM Extents (#284)
XLine/Ray tessellate as +/-1e6 display segments (entities/ray.rs), and both fit_all outlier defenses miss them: the IQR reject passes because a construction line through the drawing has its centroid at its base point, inside the consensus cluster; and the per-point lim filter passes because local_extent_max is computed once at document load and stays at the 1e9 default for drawings created fresh in-app. The far endpoints then poison the bounds and the view fits +/-1e6, shrinking real geometry to a dot. Exclude XLine/Ray wires from the extents up front (AutoCAD likewise ignores infinite lines in ZOOM Extents), falling back to fitting their base points when the drawing holds nothing else. Regression tests verified to fail without the fix (camera distance 3,000,000) and pass with it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
23d9a261b1
commit
a3107d95e4
2 changed files with 105 additions and 2 deletions
|
|
@ -636,6 +636,30 @@ impl Scene {
|
|||
if self.current_layout != "Model" {
|
||||
wires.extend(self.viewport_content_wires(layout_block, None, None));
|
||||
}
|
||||
// Ray / XLine tessellate as ±DISPLAY_EXTENT display segments
|
||||
// (entities/ray.rs) — their endpoints are rendering artifacts, not
|
||||
// drawing extent. A construction line through the drawing defeats
|
||||
// both rejects below: its centroid sits at the base point (inside
|
||||
// the consensus cluster), and in a fresh drawing `local_extent_max`
|
||||
// is still the 1e9 default, so the far points pass the `lim` filter
|
||||
// too (issue #284). Exclude them up front — matching AutoCAD, where
|
||||
// infinite lines never contribute to ZOOM Extents — and fall back
|
||||
// to their base points if the drawing holds nothing else.
|
||||
let mut infinite_base_pts: Vec<glam::Vec3> = Vec::new();
|
||||
wires.retain(|w| {
|
||||
let is_infinite = Self::handle_from_wire_name(&w.name)
|
||||
.and_then(|h| self.document.get_entity(h))
|
||||
.map(|e| matches!(e, EntityType::XLine(_) | EntityType::Ray(_)))
|
||||
.unwrap_or(false);
|
||||
if is_infinite {
|
||||
infinite_base_pts.extend(
|
||||
w.key_vertices
|
||||
.iter()
|
||||
.map(|v| glam::Vec3::new(v[0] as f32, v[1] as f32, v[2] as f32)),
|
||||
);
|
||||
}
|
||||
!is_infinite
|
||||
});
|
||||
// 3D solids render as meshes, not wires, so collect their (offset-rel)
|
||||
// XY AABBs separately — a drawing of only solids has no wires to fit.
|
||||
let mesh_aabbs: Vec<[f32; 4]> = self
|
||||
|
|
@ -650,7 +674,7 @@ impl Scene {
|
|||
.map(|(_, set)| set.world_aabb)
|
||||
.filter(|a| a[0].is_finite() && a[2].is_finite())
|
||||
.collect();
|
||||
if wires.is_empty() && mesh_aabbs.is_empty() {
|
||||
if wires.is_empty() && mesh_aabbs.is_empty() && infinite_base_pts.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -687,7 +711,7 @@ impl Scene {
|
|||
});
|
||||
}
|
||||
}
|
||||
if cents.is_empty() && mesh_aabbs.is_empty() {
|
||||
if cents.is_empty() && mesh_aabbs.is_empty() && infinite_base_pts.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -757,6 +781,14 @@ impl Scene {
|
|||
min = min.min(glam::Vec3::new(*ax, *ay, 0.0));
|
||||
max = max.max(glam::Vec3::new(*bx, *by, 0.0));
|
||||
}
|
||||
// Drawing holds only infinite construction geometry: fit the view to
|
||||
// its base points rather than leaving the camera unchanged.
|
||||
if min.x > max.x {
|
||||
for p in &infinite_base_pts {
|
||||
min = min.min(*p);
|
||||
max = max.max(*p);
|
||||
}
|
||||
}
|
||||
// If no usable points found, leave the camera unchanged.
|
||||
if min.x > max.x {
|
||||
return;
|
||||
|
|
|
|||
71
tests/zoom_extents_xline.rs
Normal file
71
tests/zoom_extents_xline.rs
Normal file
|
|
@ -0,0 +1,71 @@
|
|||
// Regression for #284: ZOOM Extents must ignore the ±1e6 display segments
|
||||
// that XLine/Ray tessellate into, instead of fitting the camera to them.
|
||||
use acadrust::entities::{Line, XLine};
|
||||
use acadrust::types::Vector3;
|
||||
use acadrust::EntityType;
|
||||
use OpenCADStudio::scene::Scene;
|
||||
|
||||
fn add_rect(scene: &mut Scene) {
|
||||
let corners = [
|
||||
(0.0, 0.0, 10.0, 0.0),
|
||||
(10.0, 0.0, 10.0, 10.0),
|
||||
(10.0, 10.0, 0.0, 10.0),
|
||||
(0.0, 10.0, 0.0, 0.0),
|
||||
];
|
||||
for (x1, y1, x2, y2) in corners {
|
||||
let mut line = Line::new();
|
||||
line.start = Vector3::new(x1, y1, 0.0);
|
||||
line.end = Vector3::new(x2, y2, 0.0);
|
||||
scene.add_entity(EntityType::Line(line));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fit_all_ignores_xline_display_extent() {
|
||||
let mut scene = Scene::new();
|
||||
add_rect(&mut scene);
|
||||
|
||||
// Construction line through the rectangle centre at 45° — its centroid
|
||||
// sits inside the drawing cluster, and in a fresh document
|
||||
// `local_extent_max` is still the 1e9 default, so before the fix both
|
||||
// outlier rejects passed its ±1e6 display endpoints into the bounds.
|
||||
let xl = XLine::new(Vector3::new(5.0, 5.0, 0.0), Vector3::new(1.0, 1.0, 0.0));
|
||||
scene.add_entity(EntityType::XLine(xl));
|
||||
|
||||
scene.fit_all();
|
||||
|
||||
let cam = scene.camera.borrow();
|
||||
assert!(
|
||||
cam.distance.is_finite() && cam.distance < 1000.0,
|
||||
"camera must fit the 10x10 rectangle, not the xline's 1e6 display \
|
||||
segment (distance = {})",
|
||||
cam.distance
|
||||
);
|
||||
assert!(
|
||||
(cam.target.x - 5.0).abs() < 5.0 && (cam.target.y - 5.0).abs() < 5.0,
|
||||
"camera target must stay on the rectangle (target = {:?})",
|
||||
cam.target
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fit_all_with_only_xline_fits_base_point() {
|
||||
let mut scene = Scene::new();
|
||||
let xl = XLine::new(Vector3::new(100.0, 200.0, 0.0), Vector3::new(0.0, 1.0, 0.0));
|
||||
scene.add_entity(EntityType::XLine(xl));
|
||||
|
||||
scene.fit_all();
|
||||
|
||||
let cam = scene.camera.borrow();
|
||||
assert!(
|
||||
cam.distance.is_finite() && cam.distance < 1000.0,
|
||||
"xline-only drawing must fit near the base point, not ±1e6 \
|
||||
(distance = {})",
|
||||
cam.distance
|
||||
);
|
||||
assert!(
|
||||
(cam.target.x - 100.0).abs() < 5.0 && (cam.target.y - 200.0).abs() < 5.0,
|
||||
"camera target must be the xline base point (target = {:?})",
|
||||
cam.target
|
||||
);
|
||||
}
|
||||
Loading…
Reference in a new issue