diff --git a/crates/engine/src/cmd/inquiry.rs b/crates/engine/src/cmd/inquiry.rs index c6fdebc6..26f074a9 100644 --- a/crates/engine/src/cmd/inquiry.rs +++ b/crates/engine/src/cmd/inquiry.rs @@ -401,8 +401,17 @@ pub(super) fn area_of(pts: &[Vec2]) -> (f64, f64) { (a, per) } +/// `points` as given, `None` when it is absent; a `points` that is not a list of points is an +/// error, never a fall back to the selection. +fn given_points(p: &Value, cmd: &str) -> Result>> { + match p.get("points") { + None | Some(Value::Null) => Ok(None), + Some(_) => points_param(p, "points").map(Some).ok_or_else(|| bad(cmd, "`points` must be a list of [x, y] points")), + } +} + fn run_area(s: &mut Session, p: &Value) -> Result { - let (a, per) = if let Some(pts) = points_param(p, "points") { + let (a, per) = if let Some(pts) = given_points(p, "area")? { area_of(&pts) } else { let h = targets(s, p)?.first().copied().ok_or_else(|| bad("area", "`points` or `handle` is required"))?; @@ -415,10 +424,10 @@ fn area_text(s: &Session, a: f64, per: f64) -> String { format!("Area = {}, Perimeter = {}", fmt_d(s, a), fmt_d(s, per)) } -/// MEASUREGEOM's Volume option: the area of a polygon of three or more points, or of a closed 2D -/// object (as AREA measures it), times the absolute value of a height. +/// MEASUREGEOM's Volume option: the area of a polygon of three or more points, or of a 2D object +/// as AREA measures it (an open polyline as if closed), times the absolute value of a height. pub(super) fn run_volume(s: &mut Session, p: &Value) -> Result { - let a = if let Some(pts) = points_param(p, "points") { + let a = if let Some(pts) = given_points(p, "volume.measure")? { if pts.len() < 3 { return Err(bad("volume.measure", "`points` needs 3 or more [x,y] points")); } @@ -429,6 +438,9 @@ pub(super) fn run_volume(s: &mut Session, p: &Value) -> Result { }; let height = f64_req("volume.measure", p, "height")?; let v = a * height.abs(); + if !v.is_finite() { + return Err(bad("volume.measure", "the volume is too large")); + } Ok(json!({ "volume": v, "area": a, "height": height, "message": volume_text(s, v) })) } @@ -940,14 +952,21 @@ impl AreaM { fn mode_keywords(&self) -> (&'static str, &'static str) { if self.volume { ("Add volume", "Subtract volume") } else { ("Add area", "Subtract area") } } + /// The running total with `v` added or subtracted; in single mode, the total as it is. + fn total_with(&self, v: f64) -> f64 { + match self.mode { + AreaMode::Single => self.total, + AreaMode::Add => self.total + v, + AreaMode::Subtract => self.total - v, + } + } /// Report a volume (and the running total in add/subtract mode); ends the command in single mode. fn volume_measured(&mut self, s: &mut Session, v: f64) -> Step { s.echo(volume_text(s, v)); - match self.mode { - AreaMode::Single => return Step::Done, - AreaMode::Add => self.total += v, - AreaMode::Subtract => self.total -= v, + if self.mode == AreaMode::Single { + return Step::Done; } + self.total = self.total_with(v); s.echo(format!("Total volume = {}", fmt_volume(s, self.total))); Step::Continue } @@ -1006,10 +1025,13 @@ impl Interactive for AreaM { if !self.pending.is_empty() { if let Input::Text(t) = i { match crate::units::parse_distance(&t).filter(|v| v.is_finite()) { - Some(h) => { - let a = self.pending.remove(0); - return Ok(self.volume_measured(s, a * h.abs())); - } + Some(h) => match self.pending.first().map(|a| a * h.abs()) { + Some(v) if v.is_finite() && self.total_with(v).is_finite() => { + self.pending.remove(0); + return Ok(self.volume_measured(s, v)); + } + _ => s.echo("The volume is too large."), + }, None => s.echo("Requires a distance."), } } diff --git a/crates/engine/tests/it/inquiry_keywords.rs b/crates/engine/tests/it/inquiry_keywords.rs index 166b374e..49b807a5 100644 --- a/crates/engine/tests/it/inquiry_keywords.rs +++ b/crates/engine/tests/it/inquiry_keywords.rs @@ -133,6 +133,49 @@ fn measuregeom_volume_multiplies_an_area_by_a_height() { assert!(s.execute("volume.measure", &json!({"points": [[0, 0], [1, 0], [1, 1]]})).is_err()); } +#[test] +fn measure_with_points_that_are_not_points_is_an_error_not_the_selection() { + let mut s = Session::new(); + let c = s.execute("circle", &json!({"center": [0, 0], "radius": 1})).unwrap(); + s.set_selection(vec![cadcraft_engine::doc::Handle::parse_hex(c["handle"].as_str().unwrap()).unwrap()]); + for bad in [json!([[0, 0], [4, 0], "north"]), json!([[0, 0], [4, 0], [4]]), json!(7)] { + assert!(s.execute("volume.measure", &json!({"points": bad, "height": 10})).is_err(), "{bad}"); + assert!(s.execute("area", &json!({"points": bad})).is_err(), "{bad}"); + } + // Without `points`, both measure the selected circle. + let r = s.execute("volume.measure", &json!({"height": 1})).unwrap(); + assert!((r["volume"].as_f64().unwrap() - std::f64::consts::PI).abs() < 1e-12, "{r}"); + let r = s.execute("area", &json!({"points": null})).unwrap(); + assert!((r["area"].as_f64().unwrap() - std::f64::consts::PI).abs() < 1e-12, "{r}"); +} + +#[test] +fn measuregeom_volume_too_large_to_hold_is_refused() { + // A 2e150 square has an area of 4e300; times 1e10 it is past f64's range. + let p = json!([[0, 0], [2e150, 0], [2e150, 2e150], [0, 2e150]]); + let mut s = Session::new(); + assert!(s.execute("volume.measure", &json!({"points": p, "height": 1e10})).is_err()); + assert!(s.execute("volume.measure", &json!({"points": p, "height": 1})).is_ok()); + + // Typed, the height is asked for again. + let (s, log) = run(&["measuregeom", "v", "0,0", "2e150,0", "2e150,2e150", "0,2e150", "", "1e10"]); + assert!(has(&log, "The volume is too large."), "{log:?}"); + assert!(!has(&log, "Volume = "), "{log:?}"); + assert!(s.prompt_text().contains("Specify height"), "{}", s.prompt_text()); + + // Two volumes that each fit can overflow the running total; the second is asked for again. + let sq = ["0,0", "1,0", "1,1", "0,1", ""]; + let mut lines = vec!["measuregeom", "v", "a"]; + lines.extend(sq); + lines.push("1e308"); + lines.extend(sq); + lines.push("1e308"); + let (s, log) = run(&lines); + assert!(has(&log, "The volume is too large."), "{log:?}"); + assert!(!has(&log, "Total volume = 0"), "{log:?}"); + assert!(s.prompt_text().contains("Specify height"), "{}", s.prompt_text()); +} + #[test] fn tools_inquiry_volume_adds_and_subtracts_a_running_total() { let spec = cadcraft_engine::find_command("volume.measure").expect("no command volume.measure");