Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor b9d4c37021 address review: correct tessellate_within doc 2026-11-03 21:55:41 +00:00
Dylan M. Taylor e91d6d7625 fix(geom): bound the points one polyline tessellation samples
Polyline::tessellate sampled every arc segment (up to 4096 points each) with no total limit, and hatch boundary preparation did this for every loop, so compact curved vertices expanded into gigabytes. tessellate_within takes a point budget; once spent, remaining arcs are drawn as chords. Hatch loops share one budget.
2026-11-03 16:05:31 +00:00
8 changed files with 55 additions and 69 deletions
+4 -5
View File
@@ -234,10 +234,9 @@ fn tables(d: &mut Drawing, out: &mut Vec<Issue>) {
missing("table style", "Standard", out);
d.table_styles.insert(0, TableStyle::default());
}
// Name sets built once: both tables come from the drawing, so a scan per reference is quadratic.
let linetypes: HashSet<String> = d.linetypes.iter().map(|l| l.name.to_ascii_lowercase()).collect();
let linetypes: Vec<String> = d.linetypes.iter().map(|l| l.name.clone()).collect();
for l in &mut d.layers {
if !linetypes.contains(&l.linetype.to_ascii_lowercase()) {
if !linetypes.iter().any(|n| n.eq_ignore_ascii_case(&l.linetype)) {
out.push(issue(
"missingLinetype",
"tables",
@@ -248,8 +247,8 @@ fn tables(d: &mut Drawing, out: &mut Vec<Issue>) {
l.linetype = "Continuous".into();
}
}
let styles: HashSet<String> = d.text_styles.iter().map(|s| s.name.to_ascii_lowercase()).collect();
let known = |n: &str| styles.contains(&n.to_ascii_lowercase());
let styles: Vec<String> = d.text_styles.iter().map(|s| s.name.clone()).collect();
let known = |n: &str| styles.iter().any(|s| s.eq_ignore_ascii_case(n));
for s in &mut d.dim_styles {
if !known(&s.text_style) {
out.push(issue(
+4 -32
View File
@@ -408,24 +408,12 @@ fn run_state_restore(s: &mut Session, p: &Value) -> Result<Value> {
let name = str_param(p, "name").ok_or_else(|| bad("layerstate.restore", "`name` is required"))?;
let d = s.doc_mut()?;
let st = d.layer_states.iter().find(|st| st.name == name).cloned().ok_or_else(|| bad("layerstate.restore", "no such layer state"))?;
restore_layers(d, st.layers);
ok()
}
/// Put the saved layers back over the drawing's layers of the same name (without regard to case);
/// saved layers the drawing no longer has are skipped. Both lists can come from a file, so the
/// drawing's layers are looked up in an index rather than scanned per saved layer.
pub(crate) fn restore_layers(d: &mut cadcraft_doc::Drawing, saved: Vec<Layer>) {
let mut index: std::collections::HashMap<String, usize> = std::collections::HashMap::with_capacity(d.layers.len());
for (i, l) in d.layers.iter().enumerate() {
// The first of names that differ only in case, as `Drawing::layer_mut` finds it.
index.entry(l.name.to_ascii_lowercase()).or_insert(i);
}
for layer in saved {
if let Some(l) = index.get(&layer.name.to_ascii_lowercase()).and_then(|&i| d.layers.get_mut(i)) {
*l = layer;
for saved in st.layers {
if let Some(l) = d.layer_mut(&saved.name) {
*l = saved;
}
}
ok()
}
fn run_state_list(s: &mut Session, _p: &Value) -> Result<Value> {
@@ -473,22 +461,6 @@ mod tests {
use crate::Session;
/// Restoring many saved layers, most of them missing, takes one pass and matches without case.
#[test]
fn restore_layers_matches_by_name_in_linear_time() {
let mut d = cadcraft_doc::Drawing::new_imperial();
d.layers = (0..20_000).map(|i| cadcraft_doc::Layer::new(&format!("L{i}"))).collect();
let mut saved: Vec<cadcraft_doc::Layer> = (0..20_000).map(|i| cadcraft_doc::Layer::new(&format!("missing{i}"))).collect();
let mut back = cadcraft_doc::Layer::new("l7");
back.frozen = true;
saved.push(back);
let started = std::time::Instant::now();
super::restore_layers(&mut d, saved);
assert!(started.elapsed() < std::time::Duration::from_secs(2));
assert!(d.layer("L7").is_some_and(|l| l.frozen));
assert_eq!(d.layers.len(), 20_000);
}
#[test]
fn layer_states_save_list_restore_delete() {
let mut s = Session::new();
+4 -28
View File
@@ -1,8 +1,6 @@
//! The CAL / QuickCalc evaluator: numbers and points (`[x,y]`, `[d<a]`), arithmetic and vector
//! arithmetic, numeric and geometric functions; unit conversion; values picked from the drawing.
use std::collections::HashMap;
use cadcraft_geom::Vec2;
use serde_json::{Map, Value, json};
@@ -69,9 +67,6 @@ struct Parser<'a> {
s: &'a [u8],
i: usize,
vars: &'a Map<String, Value>,
/// `vars` by ASCII-lower-cased name (the first of names that differ only in case), so a
/// case-insensitive lookup is not a scan of the map per reference.
folded: HashMap<String, &'a Value>,
depth: usize,
unary_depth: usize,
}
@@ -235,7 +230,7 @@ impl Parser<'_> {
match name.as_str() {
"pi" => Ok(Val::Num(std::f64::consts::PI)),
"e" => Ok(Val::Num(std::f64::consts::E)),
_ => var(self.vars, &self.folded, &name).ok_or_else(|| format!("unknown name `{name}`")),
_ => var(self.vars, &name).ok_or_else(|| format!("unknown name `{name}`")),
}
}
_ => Err("expected a number".into()),
@@ -244,8 +239,8 @@ impl Parser<'_> {
}
/// A variable: a number, or a point as `[x, y]`. Names are case-insensitive.
fn var(vars: &Map<String, Value>, folded: &HashMap<String, &Value>, name: &str) -> Option<Val> {
let v = vars.get(name).or_else(|| folded.get(&name.to_ascii_lowercase()).copied())?;
fn var(vars: &Map<String, Value>, name: &str) -> Option<Val> {
let v = vars.get(name).or_else(|| vars.iter().find(|(k, _)| k.eq_ignore_ascii_case(name)).map(|(_, v)| v))?;
if let Some(n) = v.as_f64() {
return Some(Val::Num(n));
}
@@ -416,11 +411,7 @@ pub fn eval_val(expr: &str, vars: &Map<String, Value>) -> Result<Val, String> {
if expr.len() > MAX_LEN {
return Err("expression too long".into());
}
let mut folded = HashMap::with_capacity(vars.len());
for (k, v) in vars {
folded.entry(k.to_ascii_lowercase()).or_insert(v);
}
let mut p = Parser { s: expr.as_bytes(), i: 0, vars, folded, depth: 0, unary_depth: 0 };
let mut p = Parser { s: expr.as_bytes(), i: 0, vars, depth: 0, unary_depth: 0 };
let v = p.expr()?;
if p.peek().is_some() {
return Err(format!("unexpected `{}`", expr.get(p.i..).unwrap_or("")));
@@ -596,21 +587,6 @@ mod tests {
eval_val(s, &Map::new())
}
/// Variables are found without regard to case; an exact match wins over a case variant.
/// (Names are read lower-cased, so `X` finds `x` too.)
#[test]
fn variables_ignore_case_and_prefer_exact_names() {
let mut vars = Map::new();
for i in 0..10_000 {
vars.insert(format!("v{i}"), json!(i));
}
vars.insert("X".into(), json!(1));
vars.insert("x".into(), json!(2));
assert!(matches!(eval_val("V9999 + 1", &vars), Ok(Val::Num(n)) if n == 10_000.0));
assert!(matches!(eval_val("x", &vars), Ok(Val::Num(n)) if n == 2.0));
assert!(eval_val("nope", &vars).is_err());
}
#[test]
fn points_and_vectors() {
assert_eq!(ev("[1,2]+[3,4]*2"), Ok(Val::Pt(Vec2::new(7.0, 10.0))));
+7 -1
View File
@@ -185,7 +185,13 @@ fn restore(s: &mut Session, name: &str) -> Result<Value> {
if let Some(state) = &v.layer_state {
let d = s.doc_mut()?;
match d.layer_states.iter().find(|l| l.name.eq_ignore_ascii_case(state)).cloned() {
Some(saved) => super::super::layer::restore_layers(d, saved.layers),
Some(saved) => {
for layer in saved.layers {
if let Some(l) = d.layer_mut(&layer.name) {
*l = layer;
}
}
}
None => msg.push_str(&format!(" Its layer snapshot \"{state}\" is missing.")),
}
}
+18 -1
View File
@@ -484,6 +484,9 @@ pub struct Polyline {
pub closed: bool,
}
/// Sampled points one [`Polyline::tessellate`] call produces before arcs become chords.
pub const MAX_TESSELLATED_POINTS: usize = 4_000_000;
impl Polyline {
pub fn from_points(pts: &[Vec2], closed: bool) -> Self {
Polyline { vertices: pts.iter().map(|p| PolyVertex::new(*p)).collect(), closed }
@@ -516,10 +519,24 @@ impl Polyline {
b
}
pub fn tessellate(&self, tol: f64) -> Vec<Vec2> {
let mut budget = MAX_TESSELLATED_POINTS;
self.tessellate_within(tol, &mut budget)
}
/// [`Polyline::tessellate`] spending about `budget` sampled points (shared by the caller
/// across many polylines). The budget is checked before each segment, so the last segment
/// started may overshoot it by that one segment's samples. Once it is spent, the remaining arcs are drawn as chords, so the
/// output stays proportional to the input vertices. Each arc can sample thousands of points,
/// so a compact polyline could otherwise expand into gigabytes (CWE-400).
pub fn tessellate_within(&self, tol: f64, budget: &mut usize) -> Vec<Vec2> {
let mut out: Vec<Vec2> = Vec::new();
for s in self.segments() {
let mut pts = Vec::new();
s.tessellate(tol, &mut pts);
if *budget > 0 {
s.tessellate(tol, &mut pts);
*budget = budget.saturating_sub(pts.len());
} else {
pts.extend([s.start(), s.end()]);
}
let skip = usize::from(out.last().is_some_and(|l| pts.first().is_some_and(|f| l.near(*f, 1e-9))));
out.extend(pts.into_iter().skip(skip));
}
+2 -1
View File
@@ -20,7 +20,8 @@ mod vec;
pub use bounds::Bounds2;
pub use curve::{
Arc, Circle, Ellipse, Line, PolyVertex, Polyline, Segment, arc_to_bulge, bulge_to_arc, point_in_polygon, polygon_centroid, shoelace,
Arc, Circle, Ellipse, Line, MAX_TESSELLATED_POINTS, PolyVertex, Polyline, Segment, arc_to_bulge, bulge_to_arc, point_in_polygon,
polygon_centroid, shoelace,
};
pub use fillet::{CornerFit, TanCurve, tangent_circles};
pub use intersect::{circle_circle, intersect_ext, intersect_segments, line_circle, line_line, line_line_infinite};
+12
View File
@@ -221,6 +221,18 @@ fn segment_offset() {
assert!(s.offset(6.0).is_none());
}
/// A polyline of many fine arcs samples at most the point budget, then draws chords.
#[test]
fn polyline_tessellation_is_bounded() {
let vertices: Vec<PolyVertex> = (0..3000).map(|i| PolyVertex { bulge: 1.0, ..PolyVertex::new(Vec2::new(f64::from(i) * 1e6, 0.0)) }).collect();
let pl = Polyline { vertices, closed: false };
let pts = pl.tessellate(1e-9);
assert!(pts.len() <= MAX_TESSELLATED_POINTS + 2 * 3000 + 4096, "{}", pts.len());
assert!(pts.len() > 3000);
let mut budget = 10;
assert!(pl.tessellate_within(1e-9, &mut budget).len() <= 4096 + 2 * 3000);
}
#[test]
fn arc_segments_bounded() {
assert_eq!(arc_segments(0.0, 1.0, 0.1), 1);
+4 -1
View File
@@ -128,7 +128,10 @@ const MAX_DEDUP_COMPARISONS: u64 = 50_000_000;
/// within islands alternate), 1 outer keeps the outermost loops and their first islands, 2
/// ignore keeps only the outermost loops. Nesting depth = how many other loops enclose a loop.
pub fn fill_loops(h: &Hatch, tol: f64) -> Vec<Vec<Vec2>> {
let mut loops: Vec<Vec<Vec2>> = h.loops.iter().map(|l| Polyline { vertices: l.vertices.clone(), closed: true }.tessellate(tol)).collect();
// One point budget for all the loops: many short arcs across loops must not add up unbounded.
let mut budget = cadcraft_geom::MAX_TESSELLATED_POINTS;
let mut loops: Vec<Vec<Vec2>> =
h.loops.iter().map(|l| Polyline { vertices: l.vertices.clone(), closed: true }.tessellate_within(tol, &mut budget)).collect();
dedup_polygons(&mut loops);
let max_depth = match h.style {
1 => 1,