Repository navigation
feat(exchange): export_step_in for bodies in a unit other than metres - #35
Basnederveen wants to merge 2 commits into
Conversation
export_step takes a body's coordinates as metres and scales them to the
file unit. A body built in millimetres had no way to say so: exporting
it as millimetres multiplied every coordinate by 1000 (a 100 mm plate
came out 100 m wide).
export_step_in(brep, model_unit, unit) takes the unit the body is built
in ("metre" or "millimetre") and scales from it to the file unit once.
export_step keeps its behaviour and delegates with model_unit = "metre".
The scale is a fixed table, so metre bodies export the same digits as
before. export_bodies takes the model unit; the world graph passes
"metre".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VFZvqx1RmXzPtbL9VXP4H7
| export_step_in(brep, "metre", unit) | ||
| } | ||
|
|
||
| pub fn export_step_in( |
There was a problem hiding this comment.
High: the unit exists only for STEP.
After this change a body can be in millimetres, but only export_step_in knows that. The IFC body export still writes length_unit: "metre" (exchange/ifc_body/mod.rs), and the world graph assumes metres too. So the same millimetre body comes out right in STEP and 1000× too large in IFC.
|
|
||
| pub fn export_step_in( | ||
| brep: &BrepEnvelope, | ||
| model_unit: &str, |
There was a problem hiding this comment.
Medium: the body's tolerances change meaning too.
Accuracy values are absolute lengths in the body's own unit, and Accuracy::STANDARD is sized for metres:
| value | in metres | in millimetres | |
|---|---|---|---|
| geometric | 1e-8 | 10 nm | 0.01 nm |
| exchange | 1e-6 | 1 µm | 1 nm |
| tessellation | 0.01 | 1 cm | 0.01 mm |
On a millimetre body, the exchange check becomes 1000× stricter and display meshes become 1000× finer. At building scale in millimetres, with coordinates around 1e5 to 1e6, the 1e-9 intersection budget is only a few ulps of a double.
Could the new test build its plate with accuracy scaled to millimetres, so it covers the case a real user would hit?
| .collect::<Vec<_>>(); | ||
| export_bodies( | ||
| &inputs, | ||
| "metre", |
There was a problem hiding this comment.
High: JavaScript users can't reach this yet.
The only STEP export in the wasm bindings goes through WorldGraph::export_step, and it passes "metre" here. So the npm package behaves exactly as before, and the 100 mm plate from the description still can't be exported unscaled from JS.
| } | ||
| match (model_unit, unit) { | ||
| ("metre", "millimetre") => Ok(1000.0), | ||
| ("millimetre", "metre") => Ok(1e-3), |
There was a problem hiding this comment.
Low: dividing gives cleaner numbers.
0.001 isn't exact in binary, so x * 1e-3 is sometimes one ulp off the correctly rounded x / 1000.0. For whole millimetre values from 1 to 10 000, 1 338 differ. For example, 9 mm becomes 0.009000000000000001 instead of 0.009. real() prints 18 significant digits, so the difference shows in the file, and a millimetre model written in metres doesn't match the same part built in metres.
Dividing by 1000 for this direction fixes it.
| Ok((text, report)) | ||
| } | ||
|
|
||
| fn unit_scale(model_unit: &str, unit: &str) -> Result<f64, GeometryError> { |
There was a problem hiding this comment.
Nit: a per-unit factor would be simpler.
Each unit name appears three times: in the guard, in the pair table and in the catch-all. A small helper that returns metres per unit (and errors on an unknown name), called once for each side, does the same job. It also turns adding inches into one line instead of a growing pair table. Inches are common in US STEP files, where they're written as a conversion-based unit of 25.4 mm.
This isn't needed for this PR, but it's worth doing if more units are coming.
| } | ||
|
|
||
| #[test] | ||
| fn a_body_in_millimetres_exports_unscaled_in_millimetres() { |
There was a problem hiding this comment.
Low: the name covers only one of the five checks.
Besides the millimetre case, this test checks that the default path still scales, that millimetre to metre scales the bounds, and that unknown file and model units are refused. If one of the rejection checks breaks, the failure still reports "exports unscaled in millimetres".
Could you split it into one test per behaviour? For example unknown_model_unit_is_rejected and millimetre_body_to_metre_file_scales_by_a_thousandth.
| metres | ||
| ); | ||
| let (scaled, _) = export_step(&brep, "millimetre").unwrap(); | ||
| assert_ne!(scaled, mm); |
There was a problem hiding this comment.
Low: these checks are weak.
assert_ne!(scaled, mm) passes on any difference, including a wrong factor. The millimetre to metre case checks only exchange_error_bound in the report and never reads the file.
A check that the metre file contains SI_UNIT($,.METRE.), and that a known corner of the plate (for example the one at 100 mm) is written as 0.1 m, would cover what users actually open.
What
export_steptakes a body's coordinates as metres and scales them to the file unit. A body built in millimetres had no way to say so: exporting it as millimetres multiplied every coordinate by 1000, so a 100 mm plate came out 100 m wide.export_step_in(brep, model_unit, unit)takes the unit the body is built in ("metre"or"millimetre") and scales from it to the file unit once.export_step(brep, unit)keeps its behaviour and delegates withmodel_unit = "metre".InvalidGeometrytoo.export_bodiesgets the model unit as a parameter. The world graph passes"metre", so world-graph STEP export (and its binding) behaves as before. A model-unit option there could follow if wanted.Tests
exchange::step::tests::a_body_in_millimetres_exports_unscaled_in_millimetres: a millimetre body exported to millimetres is the metre export digit for digit except theSI_UNITprefix; the default path still scales; millimetre to metre scales the report's bounds by 1e-3; unknown file or model units are refused.cargo fmt --check,cargo clippy --all-targets -D warnings: clean.cargo test --all-targets --no-fail-fast(withCI=true, Windows x86_64): 786 pass, 0 fail, 1 ignored. Cleanmaingives 785 pass, 0 fail, the same 1 ignored; the extra test is the new one.npm run check:source,check:duplicates,check:cycles: clean.check:apinot run (needs the SDK build); no TypeScript or binding signature changes.Files
main/opengeometry/src/exchange/step/mod.rsmain/opengeometry/src/exchange/step/tests.rsmain/opengeometry/src/exchange/mod.rsmain/opengeometry/src/world_graph/export.rs🤖 Generated with Claude Code