Skip to content

feat(exchange): export_step_in for bodies in a unit other than metres #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Basnederveen wants to merge 2 commits into
base: main
Choose a base branch
from
+66 −12
Open

feat(exchange): export_step_in for bodies in a unit other than metres #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e007470
feat(exchange): export_step_in for bodies in a unit other than metres
Basnederveen Nov 1, 2026
d5f6b5e
Merge upstream main into feat/step-model-unit
Basnederveen Nov 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion main/opengeometry/src/exchange/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@ mod preflight;
mod step;

pub(super) use step::{export_bodies, StepBodyInput, StepExportFailure};
pub use step::{export_step, StepBodyReport, StepExportReport, StepReport, StepSkipped};
pub use step::{
export_step, export_step_in, StepBodyReport, StepExportReport, StepReport, StepSkipped,
};
37 changes: 27 additions & 10 deletions main/opengeometry/src/exchange/step/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,22 @@ use body::{emit_body, BodyEmission};
use product::{emit_context, emit_products, emit_representations};

pub fn export_step(brep: &BrepEnvelope, unit: &str) -> Result<(String, StepReport), GeometryError> {
export_step_in(brep, "metre", unit)
}

pub fn export_step_in(

@aka-blackboots aka-blackboots Nov 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

brep: &BrepEnvelope,
model_unit: &str,

@aka-blackboots aka-blackboots Nov 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

unit: &str,
) -> Result<(String, StepReport), GeometryError> {
let (text, report) = export_bodies(
&[StepBodyInput {
og_id: &brep.id,
shape_id: &brep.id,
shape_revision: brep.revision,
brep,
}],
model_unit,
unit,
"Y",
&brep.id,
Expand Down Expand Up @@ -50,22 +59,14 @@ pub fn export_step(brep: &BrepEnvelope, unit: &str) -> Result<(String, StepRepor

pub(crate) fn export_bodies(
bodies: &[StepBodyInput<'_>],
model_unit: &str,
unit: &str,
up_axis: &str,
name: &str,
timestamp: &str,
skipped: Vec<StepSkipped>,
) -> Result<(String, StepExportReport), StepExportFailure> {
let scale = match unit {
"metre" => 1.0,
"millimetre" => 1000.0,
_ => {
return Err(GeometryError::InvalidGeometry(
"STEP length unit must be metre or millimetre".into(),
)
.into())
}
};
let scale = unit_scale(model_unit, unit)?;
if bodies.is_empty() {
return Err(
GeometryError::InvalidGeometry("STEP export has no solid bodies".into()).into(),
Expand Down Expand Up @@ -121,6 +122,22 @@ pub(crate) fn export_bodies(
Ok((text, report))
}

fn unit_scale(model_unit: &str, unit: &str) -> Result<f64, GeometryError> {

@aka-blackboots aka-blackboots Nov 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if !matches!(unit, "metre" | "millimetre") {
return Err(GeometryError::InvalidGeometry(
"STEP length unit must be metre or millimetre".into(),
));
}
match (model_unit, unit) {
("metre", "millimetre") => Ok(1000.0),
("millimetre", "metre") => Ok(1e-3),

@aka-blackboots aka-blackboots Nov 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

("metre" | "millimetre", _) => Ok(1.0),
_ => Err(GeometryError::InvalidGeometry(
"STEP model length unit must be metre or millimetre".into(),
)),
}
}

fn add_bodies(
writer: &mut Part21Writer,
bodies: &[StepBodyInput<'_>],
Expand Down
36 changes: 35 additions & 1 deletion main/opengeometry/src/exchange/step/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{export_bodies, export_step, StepBodyInput};
use super::{export_bodies, export_step, export_step_in, StepBodyInput};
use crate::brep::{
coarse_accuracy, BrepEnvelope, Curve, CurveGeometry, EdgeGeometry, Frame3, GeometryError,
IntersectionDefinition, IntersectionSide, PcurveGeometry, Surface, TraceAnchor,
Expand Down Expand Up @@ -194,6 +194,39 @@ fn rejects_unrepresentable_exchange_and_preserves_large_revisions() {
));
}

#[test]
fn a_body_in_millimetres_exports_unscaled_in_millimetres() {

@aka-blackboots aka-blackboots Nov 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

let brep = primitives::cuboid(
"plate".into(),
Frame3::IDENTITY,
[100.0, 50.0, 2.0],
coarse_accuracy(1e-3),
)
.unwrap();
let (metres, _) = export_step(&brep, "metre").unwrap();
let (mm, report) = export_step_in(&brep, "millimetre", "millimetre").unwrap();
assert_eq!(report.length_unit, "millimetre");
assert_eq!(
mm.replace("SI_UNIT(.MILLI.,.METRE.)", "SI_UNIT($,.METRE.)"),
metres
);
let (scaled, _) = export_step(&brep, "millimetre").unwrap();
assert_ne!(scaled, mm);

@aka-blackboots aka-blackboots Nov 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

let (_, to_metres) = export_step_in(&brep, "millimetre", "metre").unwrap();
assert_eq!(
to_metres.exchange_error_bound,
report.exchange_error_bound * 1e-3
);
assert!(matches!(
export_step_in(&brep, "millimetre", "inch"),
Err(GeometryError::InvalidGeometry(_))
));
assert!(matches!(
export_step_in(&brep, "inch", "millimetre"),
Err(GeometryError::InvalidGeometry(_))
));
}

#[test]
fn two_million_entity_export_is_limit_exceeded() {
let brep = primitives::cuboid(
Expand All @@ -214,6 +247,7 @@ fn two_million_entity_export_is_limit_exceeded() {
let failure = export_bodies(
&inputs,
"metre",
"metre",
"Y",
"cap",
"1970-01-01T00:00:00",
Expand Down
1 change: 1 addition & 0 deletions main/opengeometry/src/world_graph/export.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ impl WorldGraph {
.collect::<Vec<_>>();
export_bodies(
&inputs,
"metre",

@aka-blackboots aka-blackboots Nov 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

&options.unit,
&options.up_axis,
&options.name,
Expand Down