review(#1660, #1663): associative sweep scan, arc angles, viewport off bit

- refresh_associative_sweeps runs on every edit and scanned the xdata of
  every entity; it now returns at once when no associative swept surface
  exists (its application is not registered) and looks at surfaces only.
- An arc's end angle was raised by TAU in a loop, which never ends on a
  huge angle from a damaged file; the sweep is taken with rem_euclid.
- The sweep DELOBJ test expected surface sweeps to delete their sources,
  which #1663 changed on purpose: an associative surface keeps them.
- codec c9ffcac (opencadcodec #103) adds an explicit viewport-off bit: the
  viewport on state is read with is_on() and set with turn_on()/turn_off()
  everywhere, so a viewport the file marks off shows off and can be turned
  back on from Properties.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Hakan SevenandClaude Opus 5.5 committed 2026-10-08 14:56:30 +03:00
1 parent 9519614e91
commit ff6e2a6613
13 files changed
+42 -29

No files matched your search

+1 -1
View File
@@ -1801,7 +1801,7 @@ mod tests {
viewport.id = 2; viewport.id = 2;
viewport.width = 100.0; viewport.width = 100.0;
viewport.height = 50.0; viewport.height = 50.0;
viewport.status.is_on = true; viewport.turn_on();
scene.add_entity(codec::EntityType::Viewport(viewport)); scene.add_entity(codec::EntityType::Viewport(viewport));
for entity in scene.document.entities_mut() { for entity in scene.document.entities_mut() {
if let codec::EntityType::Viewport(viewport) = entity { if let codec::EntityType::Viewport(viewport) = entity {
+11 -8
View File
@@ -935,14 +935,17 @@ mod delobj_tests {
expected, expected,
); );
} }
assert_eq!( // A swept surface is associative by default (SURFACEASSOCIATIVITY 1)
sweep_source_presence(2, crate::command::ExtrudeMode::Surface), // and keeps the profile and path it follows, whatever DELOBJ says;
(false, false), // DELOBJ reaches surfaces only with associativity off, a profile-wide
); // switch this parallel test leaves alone.
assert_eq!( assert!(crate::scene::model::sweep_model::surface_associativity());
sweep_source_presence(3, crate::command::ExtrudeMode::Surface), for value in [2, 3] {
(false, true), assert_eq!(
); sweep_source_presence(value, crate::command::ExtrudeMode::Surface),
(true, true),
);
}
} }
} }
+1 -1
View File
@@ -719,7 +719,7 @@ impl OpenCADStudio {
vp.view_height, vp.view_height,
vp.height, vp.height,
), ),
vp.status.is_on, vp.is_on(),
vp.status.locked, vp.status.locked,
)) ))
} else { } else {
+2 -2
View File
@@ -27,7 +27,7 @@ fn fixture() -> (OpenCADStudio, Handle, ViewportFrame) {
viewport.view_height = 1000.0; viewport.view_height = 1000.0;
viewport.custom_scale = 0.1; viewport.custom_scale = 0.1;
viewport.view_direction = Vector3::UNIT_Z; viewport.view_direction = Vector3::UNIT_Z;
viewport.status.is_on = true; viewport.turn_on();
let viewport = scene.add_entity(EntityType::Viewport(viewport)); let viewport = scene.add_entity(EntityType::Viewport(viewport));
let frame = scene.viewport_frame(viewport).unwrap(); let frame = scene.viewport_frame(viewport).unwrap();
scene.document.header.dimension_associativity = 2; scene.document.header.dimension_associativity = 2;
@@ -484,7 +484,7 @@ fn viewport_dimension_eligibility_is_shared_and_uses_clipping() {
first.viewport first.viewport
); );
if let Some(EntityType::Viewport(vp)) = scene.document.get_entity_mut(first.viewport) { if let Some(EntityType::Viewport(vp)) = scene.document.get_entity_mut(first.viewport) {
vp.status.is_on = false; vp.turn_off();
} }
assert!(scene.viewport_frames_at_paper_point(paper).is_empty()); assert!(scene.viewport_frames_at_paper_point(paper).is_empty());
assert!(scene.dimension_pick_through_viewport(paper, 0.2).is_none()); assert!(scene.dimension_pick_through_viewport(paper, 0.2).is_none());
+8 -5
View File
@@ -113,7 +113,7 @@ fn properties(vp: &Viewport) -> Vec<PropSection> {
field: "vp_on", field: "vp_on",
value: PropValue::BoolToggle { value: PropValue::BoolToggle {
field: "vp_on", field: "vp_on",
value: vp.status.is_on, value: vp.is_on(),
}, },
}, },
ro( ro(
@@ -193,11 +193,14 @@ fn apply_geom_prop(vp: &mut Viewport, field: &str, value: &str) {
return; return;
} }
"vp_on" => { "vp_on" => {
vp.status.is_on = if value == "toggle" { // turn_on/turn_off keep the explicit-off bit in step, so a
!vp.status.is_on // viewport the file marked off can be turned back on.
let on = if value == "toggle" { !vp.is_on() } else { value == "true" };
if on {
vp.turn_on();
} else { } else {
value == "true" vp.turn_off();
}; }
return; return;
} }
"vp_perspective" => { "vp_perspective" => {
+1 -1
View File
@@ -2539,7 +2539,7 @@ fn fix_viewport_status_flags(doc: &mut CadDocument) {
// If bit 0 is not set but bit 15 is, this is an external-format viewport: // If bit 0 is not set but bit 15 is, this is an external-format viewport:
// treat bit 15 as "on" and bit 14 as "locked". // treat bit 15 as "on" and bit 14 as "locked".
if (bits & 0x0001) == 0 && (bits & 0x8000) != 0 { if (bits & 0x0001) == 0 && (bits & 0x8000) != 0 {
vp.status.is_on = true; vp.turn_on();
vp.status.locked = (bits & 0x4000) != 0; vp.status.locked = (bits & 0x4000) != 0;
} }
} }
+1 -1
View File
@@ -7504,7 +7504,7 @@ impl Scene {
let Some(EntityType::Viewport(viewport)) = self.document.get_entity(handle) else { let Some(EntityType::Viewport(viewport)) = self.document.get_entity(handle) else {
continue; continue;
}; };
if viewport.common.owner_handle != paper_block || !viewport.status.is_on { if viewport.common.owner_handle != paper_block || !viewport.is_on() {
continue; continue;
} }
let mode = render_mode_override.unwrap_or(viewport.render_mode); let mode = render_mode_override.unwrap_or(viewport.render_mode);
+4 -2
View File
@@ -475,8 +475,10 @@ pub fn assoc_edge_curve(entity: &EntityType) -> Option<(i32, codec::objects::Ass
EntityType::Arc(value) => { EntityType::Arc(value) => {
let normal = glam::DVec3::new(value.normal.x, value.normal.y, value.normal.z).try_normalize()?; let normal = glam::DVec3::new(value.normal.x, value.normal.y, value.normal.z).try_normalize()?;
let axis = arbitrary_x(normal); let axis = arbitrary_x(normal);
let mut end = value.end_angle; // The end past the start by the arc's sweep; an equal pair is a
while end <= value.start_angle { end += std::f64::consts::TAU; } // full turn. (A loop adding TAU never ends on a huge angle.)
let sweep = (value.end_angle - value.start_angle).rem_euclid(std::f64::consts::TAU);
let end = value.start_angle + if sweep > 0.0 { sweep } else { std::f64::consts::TAU };
Some((11, K::Arc, arc(glam::DVec3::new(value.center.x, value.center.y, value.center.z), normal, axis, value.radius, value.start_angle, end))) Some((11, K::Arc, arc(glam::DVec3::new(value.center.x, value.center.y, value.center.z), normal, axis, value.radius, value.start_angle, end)))
} }
EntityType::LwPolyline(polyline) => { EntityType::LwPolyline(polyline) => {
+1 -1
View File
@@ -13,7 +13,7 @@ use codec::EntityType;
use crate::entities::curve::entity_curve; use crate::entities::curve::entity_curve;
pub use super::sweep_command_model::{ pub use super::sweep_command_model::{
is_sweep_path, is_sweep_profile, scaled_sweep_path_refusal, spatial_profile, spatial_sweep_record, surface_sweep_record, sweep_expressions, link_sweep_expressions, link_sweep_sources, sweep_sources, resweep_from_sources, surface_associativity, set_surface_associativity, assoc_edge_curve, sweep_record, sweep_refusal_code, sweep_selection_options, swept_surface_entity, swept_with_options, is_sweep_path, is_sweep_profile, scaled_sweep_path_refusal, spatial_profile, spatial_sweep_record, surface_sweep_record, sweep_expressions, link_sweep_expressions, link_sweep_sources, sweep_sources, SWEEP_SOURCES_APP, resweep_from_sources, surface_associativity, set_surface_associativity, assoc_edge_curve, sweep_record, sweep_refusal_code, sweep_selection_options, swept_surface_entity, swept_with_options,
}; };
/// A drawn profile, as the kernel wants it: the plane it lies in and the /// A drawn profile, as the kernel wants it: the plane it lies in and the
+6 -1
View File
@@ -1140,8 +1140,13 @@ impl Scene {
.filter(|(_, kind)| !matches!(kind, crate::scene::ChangeKind::Removed)) .filter(|(_, kind)| !matches!(kind, crate::scene::ChangeKind::Removed))
.map(|(handle, _)| *handle) .map(|(handle, _)| *handle)
.collect::<rustc_hash::FxHashSet<_>>(); .collect::<rustc_hash::FxHashSet<_>>();
if changed.is_empty() { return Vec::new(); } // Every edit passes through here: a drawing with no associative
// swept surface (no application registered for them) skips the scan.
if changed.is_empty() || !self.document.app_ids.contains(sweep_model::SWEEP_SOURCES_APP) {
return Vec::new();
}
let linked = self.document.entities() let linked = self.document.entities()
.filter(|entity| matches!(entity, EntityType::Surface(_)))
.filter_map(|entity| { .filter_map(|entity| {
let handle = entity.common().handle; let handle = entity.common().handle;
let (profile, path) = sweep_model::sweep_sources(entity)?; let (profile, path) = sweep_model::sweep_sources(entity)?;
+3 -3
View File
@@ -801,7 +801,7 @@ impl Scene {
let Some(EntityType::Viewport(vp)) = self.document.get_entity(*handle) else { let Some(EntityType::Viewport(vp)) = self.document.get_entity(*handle) else {
return None; return None;
}; };
if !vp.status.is_on { if !vp.is_on() {
return None; return None;
} }
let rect = self.viewport_screen_rect(vp.common.handle, canvas)?; let rect = self.viewport_screen_rect(vp.common.handle, canvas)?;
@@ -838,7 +838,7 @@ impl Scene {
pub fn viewport_displays_content(&self, vp_handle: Handle) -> bool { pub fn viewport_displays_content(&self, vp_handle: Handle) -> bool {
matches!( matches!(
self.document.get_entity(vp_handle), self.document.get_entity(vp_handle),
Some(EntityType::Viewport(vp)) if vp.status.is_on Some(EntityType::Viewport(vp)) if vp.is_on()
) )
} }
@@ -850,7 +850,7 @@ impl Scene {
let Some(EntityType::Viewport(vp)) = self.document.get_entity(*handle) else { let Some(EntityType::Viewport(vp)) = self.document.get_entity(*handle) else {
return None; return None;
}; };
if vp.status.is_on { if vp.is_on() {
Some(vp.common.handle) Some(vp.common.handle)
} else { } else {
None None
+1 -1
View File
@@ -194,7 +194,7 @@ impl Scene {
let Some(EntityType::Viewport(vp)) = self.document.get_entity(handle) else { let Some(EntityType::Viewport(vp)) = self.document.get_entity(handle) else {
continue; continue;
}; };
if !vp.status.is_on if !vp.is_on()
|| self.entity_temporarily_hidden(handle) || self.entity_temporarily_hidden(handle)
{ {
continue; continue;
+2 -2
View File
@@ -21,7 +21,7 @@ impl Scene {
}) })
.filter(|vp| { .filter(|vp| {
vp.common.owner_handle == paper_block vp.common.owner_handle == paper_block
&& vp.status.is_on && vp.is_on()
&& only_vp.map_or(true, |h| vp.common.handle == h) && only_vp.map_or(true, |h| vp.common.handle == h)
&& exclude_vp.map_or(true, |h| vp.common.handle != h) && exclude_vp.map_or(true, |h| vp.common.handle != h)
}) })
@@ -439,7 +439,7 @@ impl Scene {
.iter() .iter()
.filter_map(|handle| match self.document.get_entity(*handle) { .filter_map(|handle| match self.document.get_entity(*handle) {
Some(EntityType::Viewport(viewport)) Some(EntityType::Viewport(viewport))
if viewport.common.owner_handle == paper_block && viewport.status.is_on => if viewport.common.owner_handle == paper_block && viewport.is_on() =>
{ {
Some(viewport) Some(viewport)
} }