fix(display): regenerate existing solid isolines

Rebuild cached solid meshes for REGEN and apply Options ISOLINES changes once when the slider is released.
This commit is contained in:
Panos authored and GitHub committed 2026-09-10 18:00:53 +03:00
1 parent 4de2792798
commit a202a61ffe
6 files changed
+281 -73

No files matched your search

+119 -1
View File
@@ -508,7 +508,18 @@ impl OpenCADStudio {
// "modified" merely because tessellation caches were invalidated (C7).
// REGENALL is functionally identical (C5).
"REGEN" | "REGENALL" => {
self.tabs[i].scene.bump_geometry();
// Solid meshes carry the isoline count they were tessellated
// with: `add_entity_internal` and `update_entity` pass
// `header.isolines` into `tessellate_volume` and store the
// result. Bumping the geometry epoch clears the wire memo but
// never re-runs that, so changing ISOLINES left every existing
// solid showing the density it was born with, and REGEN — the
// command whose whole job is this — did not help.
//
// `populate_meshes_from_document` re-tessellates from the
// document and bumps the epoch itself, so it replaces the bump
// rather than adding to it.
self.tabs[i].scene.populate_meshes_from_document();
self.command_line.push_output(crate::t!("REGEN: regenerated model.").as_ref());
return Some(Task::none());
}
@@ -1893,3 +1904,110 @@ mod tests {
assert!(!app.tabs[i].dirty, "REGENALL must not dirty the document either");
}
}
#[cfg(test)]
mod regen_tests {
use crate::app::OpenCADStudio;
/// A solid's mesh carries the isoline count it was tessellated with:
/// `add_entity_internal` and `update_entity` pass `header.isolines` into
/// `tessellate_volume` and store the result. REGEN used to bump the
/// geometry epoch and stop, which clears the wire memo but leaves
/// `scene.meshes` exactly as it was — reported on #1123 as "REGEN should
/// fix this, but the isolines keep visible".
///
/// Building ACIS geometry here would be out of proportion, so the mesh map
/// is seeded with a stale entry instead: REGEN must rebuild the map from
/// the document, and an entry belonging to no entity cannot survive that.
/// Bumping alone would leave it in place.
#[test]
fn regen_rebuilds_the_mesh_map_rather_than_only_bumping_the_epoch() {
let mut app = OpenCADStudio::new_for_test();
app.automation_op(r#"{"op":"new"}"#);
let i = app.active_tab;
// An entry belonging to no entity. Rebuilding the map from the document
// drops it; bumping the geometry epoch leaves it exactly where it is,
// which is the whole difference this fix turns on.
let stale = acadrust::Handle::new(0xDEAD);
app.tabs[i].scene.meshes.insert(stale, stale_mesh());
let epoch_before = app.tabs[i].scene.geometry_epoch;
let _ = app.run_command_line("REGEN");
assert!(
!app.tabs[i].scene.meshes.contains_key(&stale),
"REGEN must rebuild the mesh map, not leave what was there",
);
assert_ne!(
app.tabs[i].scene.geometry_epoch, epoch_before,
"and it still has to bump the geometry epoch",
);
}
/// Dragging the isolines slider must not rebuild every solid's mesh on the
/// way — a drag emits a message per pixel, and the rebuild is the same work
/// REGEN does. It waits for the release, and only runs if the value moved.
#[test]
fn the_isolines_slider_rebuilds_once_on_release_and_not_while_dragging() {
use crate::app::Message;
let mut app = OpenCADStudio::new_for_test();
app.automation_op(r#"{"op":"new"}"#);
let i = app.active_tab;
let seed = |app: &mut OpenCADStudio, i: usize| {
let stale = acadrust::Handle::new(0xBEEF);
app.tabs[i].scene.meshes.insert(stale, stale_mesh());
stale
};
let stale = seed(&mut app, i);
for value in [4i16, 3, 2, 1, 0] {
let _ = app.update(Message::IsolinesChanged(value));
}
assert!(
app.tabs[i].scene.meshes.contains_key(&stale),
"the drag itself must not rebuild the meshes",
);
assert_eq!(app.tabs[i].scene.document.header.isolines, 0);
let _ = app.update(Message::IsolinesReleased);
assert!(
!app.tabs[i].scene.meshes.contains_key(&stale),
"releasing after a change rebuilds them",
);
// A release with nothing changed since the last one is a click that
// moved nothing, and must not pay for a rebuild.
let stale = seed(&mut app, i);
let _ = app.update(Message::IsolinesChanged(0));
let _ = app.update(Message::IsolinesReleased);
assert!(
app.tabs[i].scene.meshes.contains_key(&stale),
"releasing without a change must rebuild nothing",
);
}
fn stale_mesh() -> crate::scene::model::mesh_model::MeshLodSet {
crate::scene::model::mesh_model::MeshLodSet {
lods: Vec::new(),
material: None,
face_materials: Default::default(),
visual_style: None,
complete: true,
edge_verts: Vec::new(),
edge_verts_low: Vec::new(),
curved_gens: Vec::new(),
metrics: Default::default(),
world_aabb: [0.0; 4],
z_aabb: [0.0; 2],
instance_source: None,
instance_transform: None,
instance_handle: None,
instance_color: None,
instance_aabb: None,
}
}
}
+7
View File
@@ -517,6 +517,10 @@ pub(super) struct OpenCADStudio {
crosshair_color: Option<[u8; 3]>,
/// Editable Options buffer for the crosshair colour.
crosshair_color_input: String,
/// Set while the isolines slider is being dragged with a value that
/// differs from the drawing's, so the costly mesh rebuild runs once on
/// release rather than on every pixel of the drag.
isolines_awaiting_regen: bool,
/// Edit buffer for the SNAPANG field on the Options Drafting page. Kept
/// separate from `snap_angle_deg` so a half-typed angle is not parsed.
snap_angle_input: String,
@@ -2048,6 +2052,8 @@ pub enum Message {
OpenFolder(String),
/// Change isolines per surface in the current drawing (ISOLINES).
IsolinesChanged(i16),
/// The isolines slider was released; rebuild the meshes if it moved.
IsolinesReleased,
/// Toggle silhouette edges in the current drawing (DISPSILH).
DispSilhChanged(bool),
/// Change surface density U in the current drawing (SURFU).
@@ -3460,6 +3466,7 @@ impl OpenCADStudio {
cursor_type: settings::CursorType::Crosshair,
crosshair_color: None,
crosshair_color_input: String::new(),
isolines_awaiting_regen: false,
snap_angle_input: "0".to_string(),
lineweight_display_scale: 100,
isometric_drafting: false,
+52
View File
@@ -145,6 +145,27 @@ pub(crate) fn snaps_from_osmode(osmode: i32) -> (Vec<SnapType>, bool) {
(modes, osmode & OSMODE_SUPPRESS == 0)
}
fn deserialize_options_tab<'de, D>(
deserializer: D,
) -> Result<crate::ui::window::options::OptionsTab, D::Error>
where
D: serde::Deserializer<'de>,
{
use crate::ui::window::options::OptionsTab;
use serde::Deserialize;
let name = String::deserialize(deserializer).unwrap_or_default();
Ok(match name.as_str() {
"files" => OptionsTab::Files,
"open-and-save" => OptionsTab::OpenAndSave,
"display" => OptionsTab::Display,
"drafting" => OptionsTab::Drafting,
"modeling" => OptionsTab::Modeling,
"selection" => OptionsTab::Selection,
"user-preferences" => OptionsTab::UserPreferences,
_ => OptionsTab::General,
})
}
/// Render a drafting angle without a trailing `.0`, so `22.5` but `30`.
///
/// The polar pop-up formats its presets the same way; both are showing the
@@ -183,6 +204,12 @@ pub struct UserSettings {
/// all. 0 means no limit. The drawing header carries no slot for it.
pub grip_object_limit: i32,
/// Which Options page was showing when the dialog was last closed.
///
/// Read leniently: `AppConfig::load` discards the *whole* file when any
/// field fails to parse, so a page name that no longer exists — a tab
/// renamed or merged away between versions — would silently reset every
/// setting the user has. An unknown name falls back to the first page.
#[serde(default, deserialize_with = "deserialize_options_tab")]
pub options_tab: crate::ui::window::options::OptionsTab,
/// Show the navigation cube (NAVVCUBE).
pub show_viewcube: bool,
@@ -427,4 +454,29 @@ mod tests {
let b: std::collections::HashSet<_> = back.into_iter().collect();
assert_eq!(a, b);
}
/// `AppConfig::load` throws the whole file away when any field fails to
/// parse, so a page name that no longer exists would quietly reset every
/// preference the user has. This branch removed the Drawing page, and
/// anyone who had it open when they last closed the dialog has that name
/// on disk.
#[test]
fn a_page_name_that_no_longer_exists_does_not_cost_the_other_settings() {
let json = r#"{
"settings": {
"options_tab": "drawing",
"pick_add": false,
"savetime_min": 42
}
}"#;
let cfg: crate::app::config::AppConfig =
serde_json::from_str(json).expect("an unknown page name must not fail the parse");
assert_eq!(
cfg.settings.options_tab,
crate::ui::window::options::OptionsTab::General,
);
assert!(!cfg.settings.pick_add, "the rest of the file must survive");
assert_eq!(cfg.settings.savetime_min, 42);
}
}
+20 -7
View File
@@ -355,24 +355,37 @@ impl OpenCADStudio {
}
}
/// Write a tessellation-affecting header variable on the active drawing.
/// Write a header variable on the active drawing and mark it modified.
///
/// These decide how solids and surfaces are tessellated, so the geometry
/// has to be rebuilt for the change to appear — the same reason `LTSCALE`
/// bumps it. Marking the tab modified is what makes the new value reach
/// the file.
pub(in crate::app) fn set_drawing_tessellation_var(
/// Deliberately does no geometry work. The variables the Options pages
/// write differ in what they cost to apply, and treating them alike is
/// wrong in both directions: `DISPSILH` is read at render time and needs
/// nothing, `SURFU`/`SURFV`/`SURFTYPE` are inputs to PEDIT's smoothing and
/// need nothing until it runs, while `ISOLINES` is baked into solid meshes
/// at tessellation and needs more than a bump — see `regenerate_meshes`.
pub(in crate::app) fn set_drawing_var(
&mut self,
write: impl FnOnce(&mut acadrust::document::HeaderVariables),
) {
let i = self.active_tab;
if let Some(tab) = self.tabs.get_mut(i) {
write(&mut tab.scene.document.header);
tab.scene.bump_geometry();
tab.dirty = true;
}
}
/// Re-tessellate the active drawing's solids from the document.
///
/// What REGEN does, and the only thing that makes a changed `ISOLINES`
/// reach solids that already exist. Costly on a solid-heavy drawing, so
/// callers driven by a slider run it when the drag ends, not per tick.
pub(in crate::app) fn regenerate_meshes(&mut self) {
let i = self.active_tab;
if let Some(tab) = self.tabs.get_mut(i) {
tab.scene.populate_meshes_from_document();
}
}
/// Rebuild text geometry in every open drawing.
///
/// `TEXTFILL` is a process-global, so a change to it invalidates the text
+58 -39
View File
@@ -6305,54 +6305,73 @@ impl OpenCADStudio {
Task::none()
}
// The five below are drawing variables: they go into the header of
// the active drawing and mark it modified, exactly as SETVAR does.
// The four that feed solid tessellation also bump the geometry, or
// the change is invisible until an unrelated edit rebuilds it.
// Drawing variables: they go into the header of the active
// drawing and mark it modified, as SETVAR does. What each one
// costs to apply differs, so they are not treated alike.
//
// ISOLINES is baked into a solid's mesh when it is tessellated, so
// an existing solid keeps the density it was born with until the
// meshes are rebuilt. That is expensive, and a slider emits on
// every pixel of a drag, so the rebuild waits for the release —
// and only happens if the value actually moved.
Message::IsolinesChanged(value) => {
self.set_drawing_tessellation_var(|header| header.isolines = value.max(0));
Task::none()
}
Message::DispSilhChanged(on) => {
self.set_drawing_tessellation_var(|header| header.display_silhouette = on);
Task::none()
}
Message::SurfaceUChanged(value) => {
self.set_drawing_tessellation_var(|header| {
header.surface_u_density = value.clamp(0, 200)
});
Task::none()
}
Message::SurfaceVChanged(value) => {
self.set_drawing_tessellation_var(|header| {
header.surface_v_density = value.clamp(0, 200)
});
Task::none()
}
Message::SurfaceTypeChanged(value) => {
self.set_drawing_tessellation_var(|header| header.surface_type = value);
Task::none()
}
Message::SolidHistChanged(record) => {
let i = self.active_tab;
if let Some(tab) = self.tabs.get_mut(i) {
tab.scene.document.header.record_solid_history = record;
tab.dirty = true;
let value = value.max(0);
let changed = self
.tabs
.get(self.active_tab)
.is_some_and(|tab| tab.scene.document.header.isolines != value);
if changed {
self.set_drawing_var(|header| header.isolines = value);
self.isolines_awaiting_regen = true;
}
Task::none()
}
Message::IsolinesReleased => {
if std::mem::take(&mut self.isolines_awaiting_regen) {
self.regenerate_meshes();
}
Task::none()
}
// Read at render time, so nothing has to be rebuilt for it.
Message::DispSilhChanged(on) => {
self.set_drawing_var(|header| header.display_silhouette = on);
Task::none()
}
// Inputs to PEDIT's mesh smoothing; they change nothing until it
// runs, so writing the header is the whole job.
Message::SurfaceUChanged(value) => {
self.set_drawing_var(|header| header.surface_u_density = value.clamp(0, 200));
Task::none()
}
Message::SurfaceVChanged(value) => {
self.set_drawing_var(|header| header.surface_v_density = value.clamp(0, 200));
Task::none()
}
Message::SurfaceTypeChanged(value) => {
self.set_drawing_var(|header| header.surface_type = value);
Task::none()
}
// Whether history is recorded from now on; nothing to redraw.
Message::SolidHistChanged(record) => {
self.set_drawing_var(|header| header.record_solid_history = record);
Task::none()
}
// Read while building geometry, so this one does bump — the same
// thing the SHOWHIST command does.
Message::ShowHistChanged(mode) => {
self.set_drawing_var(|header| {
header.show_solid_history = mode.clamp(0, 2)
});
let i = self.active_tab;
if let Some(tab) = self.tabs.get_mut(i) {
tab.scene.document.header.show_solid_history = mode.clamp(0, 2);
tab.scene.bump_geometry();
tab.dirty = true;
}
Task::none()
}
+25 -26
View File
@@ -32,7 +32,6 @@ pub enum OptionsTab {
Modeling,
Selection,
UserPreferences,
Drawing,
}
/// Application preferences the dialog reads that are plain scalars on the app.
@@ -1077,29 +1076,7 @@ pub fn view_window<'a>(
.spacing(0)
.width(sizing.width);
let drawing = column![
text(crate::t!("Block Edit")).size(15),
Space::new().height(10),
row![
iced::widget::checkbox(double_click_block_refedit)
.on_toggle(Message::DoubleClickBlockRefeditChanged)
.size(15),
text(crate::t!("Double-click to edit block in-place (REFEDIT)")).size(12),
]
.spacing(8)
.align_y(iced::Center),
Space::new().height(18),
row![
iced::widget::checkbox(double_click_block_attedit)
.on_toggle(Message::DoubleClickBlockAtteditChanged)
.size(15),
text(crate::t!("Double-click attributed blocks to edit attributes (ATTEDIT)")).size(12),
]
.spacing(8)
.align_y(iced::Center),
]
.spacing(0)
.width(sizing.width);
// ANNOAUTOSCALE's magnitude decides which objects a newly added scale
@@ -1225,6 +1202,26 @@ pub fn view_window<'a>(
.size(11)
.width(sizing.width),
Space::new().height(24),
text(crate::t!("Block Edit")).size(15),
Space::new().height(10),
row![
iced::widget::checkbox(double_click_block_refedit)
.on_toggle(Message::DoubleClickBlockRefeditChanged)
.size(15),
text(crate::t!("Double-click to edit block in-place (REFEDIT)")).size(12),
]
.spacing(8)
.align_y(iced::Center),
Space::new().height(18),
row![
iced::widget::checkbox(double_click_block_attedit)
.on_toggle(Message::DoubleClickBlockAtteditChanged)
.size(15),
text(crate::t!("Double-click attributed blocks to edit attributes (ATTEDIT)")).size(12),
]
.spacing(8)
.align_y(iced::Center),
Space::new().height(24),
text(crate::t!("Drawing Units")).size(15),
Space::new().height(10),
row![
@@ -1394,12 +1391,16 @@ pub fn view_window<'a>(
.push(
row![
text(crate::t!("Isolines per surface")).size(12).width(150),
// Rebuilding every solid's mesh is what makes a new
// isoline count reach solids that already exist, and it is
// far too costly to do on each pixel of a drag.
slider(
0..=64,
drawing_prefs.isolines.clamp(0, 64),
Message::IsolinesChanged,
)
.step(1i16)
.on_release(Message::IsolinesReleased)
.width(Fill),
text(drawing_prefs.isolines.clamp(0, 64).to_string())
.size(11)
@@ -1565,7 +1566,6 @@ pub fn view_window<'a>(
OptionsTab::Drafting => drafting.into(),
OptionsTab::Modeling => modeling.into(),
OptionsTab::UserPreferences => user_prefs.into(),
OptionsTab::Drawing => drawing.into(),
};
// A vertical rail rather than a horizontal strip: the tab names are
@@ -1588,7 +1588,6 @@ pub fn view_window<'a>(
tab_button(crate::t!("3D Modeling"), OptionsTab::Modeling),
tab_button(crate::t!("Selection"), OptionsTab::Selection),
tab_button(crate::t!("User Preferences"), OptionsTab::UserPreferences),
tab_button(crate::t!("Drawing"), OptionsTab::Drawing),
]
.spacing(2)
.width(iced::Length::Fixed(TAB_RAIL_WIDTH));