1b-agent/OneCAD-Draft
1
0 Fork 0
forked from 1b-agent/opencadstudio-upstream
Code Pull Requests Activity

refactor(display): trim isolines follow-up comments

Browse Source
This commit is contained in:
Hakan Seven committed 2026-09-10 18:05:42 +03:00
1 parent a202a61ffe
commit 5ffbf598a9
6 files changed
+8 -91
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

src/app/commands/display.rs
+3 -43
View File
@@ -502,23 +502,8 @@ impl OpenCADStudio {
self.command_line.push_output(crate::t!("REDRAWALL: viewports refreshed.").as_ref());
return Some(Task::none());
}
// REGEN — full model regeneration (bump_geometry: geometry_epoch AND
// block_epoch; C4). No undo, no DB mutation, so do NOT touch
// self.tabs[i].dirty — a newly opened drawing must not become
// "modified" merely because tessellation caches were invalidated (C7).
// REGENALL is functionally identical (C5).
// Rebuild tessellation caches without modifying the document.
"REGEN" | "REGENALL" => {
// 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());
@@ -1903,32 +1888,13 @@ mod tests {
let _ = app.run_command_line("REGENALL");
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;
@@ -1937,17 +1903,14 @@ mod regen_tests {
assert!(
!app.tabs[i].scene.meshes.contains_key(&stale),
"REGEN must rebuild the mesh map, not leave what was there",
"REGEN left a stale mesh",
);
assert_ne!(
app.tabs[i].scene.geometry_epoch, epoch_before,
"and it still has to bump the geometry epoch",
"REGEN did not 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;
@@ -1978,8 +1941,6 @@ mod regen_tests {
"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);
@@ -2010,4 +1971,3 @@ mod regen_tests {
}
}
}
src/app/mod.rs
+1 -3
View File
@@ -517,9 +517,7 @@ 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.
/// Defer the ISOLINES mesh rebuild until the slider is released.
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.
src/app/settings.rs
+1 -12
View File
@@ -203,12 +203,7 @@ pub struct UserSettings {
/// GRIPOBJLIMIT: past this many selected objects, no grips are drawn at
/// 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.
/// Last Options page; unknown saved names fall back without rejecting the config.
#[serde(default, deserialize_with = "deserialize_options_tab")]
pub options_tab: crate::ui::window::options::OptionsTab,
/// Show the navigation cube (NAVVCUBE).
@@ -455,11 +450,6 @@ mod tests {
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#"{
@@ -479,4 +469,3 @@ mod tests {
assert_eq!(cfg.settings.savetime_min, 42);
}
}
src/app/update/file.rs
+1 -12
View File
@@ -356,13 +356,6 @@ impl OpenCADStudio {
}
/// Write a header variable on the active drawing and mark it modified.
///
/// 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),
@@ -374,11 +367,7 @@ impl OpenCADStudio {
}
}
/// 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.
/// Re-tessellate active-drawing solids after an ISOLINES change.
pub(in crate::app) fn regenerate_meshes(&mut self) {
let i = self.active_tab;
if let Some(tab) = self.tabs.get_mut(i) {
src/app/update/mod.rs
+1 -15
View File
@@ -6305,15 +6305,7 @@ impl OpenCADStudio {
Task::none()
}
// 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.
// ISOLINES is baked into solid meshes, so rebuild once on release.
Message::IsolinesChanged(value) => {
let value = value.max(0);
let changed = self
@@ -6334,14 +6326,11 @@ impl OpenCADStudio {
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()
@@ -6357,14 +6346,11 @@ impl OpenCADStudio {
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)
src/ui/window/options.rs
+1 -6
View File
@@ -1076,9 +1076,6 @@ pub fn view_window<'a>(
.spacing(0)
.width(sizing.width);
// ANNOAUTOSCALE's magnitude decides which objects a newly added scale
// reaches — 1 skips layers that are off, frozen, locked or frozen in the
// viewport, 2 keeps locked ones, 3 skips only locked, 4 takes everything.
@@ -1391,9 +1388,7 @@ 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.
// Retessellate once when dragging ends.
slider(
0..=64,
drawing_prefs.isolines.clamp(0, 64),