Skip to content

Commit 64bf7d4

Browse files
authored
test(ffi): pin the ty and tz conjuncts of the site-local threshold (#2936) (#2949)
`normalize_to_site_local`'s guard is a three-way conjunction, but only the `tx` conjunct was pinned. Two thirds of it could be deleted outright with the whole suite still green. Mutation table, whole rust/ffi suite, before this change: tx: `<` -> `<=` KILLED by the bracketing test ty: `<` -> `<=` SURVIVED, 11/11 green tz: `<` -> `<=` SURVIVED, 11/11 green ty conjunct neutralised SURVIVED, 11/11 green tz conjunct neutralised SURVIVED, 11/11 green After, every one of the five FAILS (10 passed, 1 failed) -- including the tx control, which was already killed and must stay killed. Review reproduced the whole table independently, both directions. Two axis-swap mutations are also newly killed, which was not the goal but is the sharper proof that the tuples do real work: read (st[12], st[14], st[13]) ty<->tz swap survived before, fails now subtract chunk[1] -= site_tz swap in the loop survived before, fails now Those die via the per-component expectation on the `should_shift == true` cases. The `false` cases assert only "nothing changed", and a no-op has no axis, so they cannot observe a wrong read on their own. The failure the missing pins allowed is silent, not loud: a site far from the origin on y or z would skip site-local normalisation entirely, leaving the consumer to do its transform and render math on coordinates at georeferenced magnitude. `positions` is already `f32` by this point, so this is not where the first quantization happens -- what the normalisation prevents is the further collapse downstream. Wrong geometry, no crash, no error. This is the FFI surface, where the consumer count makes a quiet wrong answer expensive. One axis proves nothing about the other two in a conjunction. The x cases were correct and complete for x, and the test read as though it covered the guard. Also updates the doc comment, which said "ty and tz are held at 0.0 and are not independently exercised here". That was true when written and this commit makes it false, so it goes in the same change rather than becoming the next stale safety comment. Assertion messages now print the whole translation triple. The old `tx={tx}` would have printed `tx=0` on the six new cases -- a useless value rather than a wrong one, but it named the one axis those cases hold at zero.
1 parent 54735f9 commit 64bf7d4

1 file changed

Lines changed: 19 additions & 9 deletions

File tree

rust/ffi/src/tests.rs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,15 @@ fn raw_ifc_with_near_origin_site_translation_is_left_untouched() {
120120
assert_eq!(result.meshes[1].positions, original_positions_b);
121121
}
122122

123-
/// Pin the boundary's sharpness on the x-axis, *relative to whatever
124-
/// `LARGE_COORD_THRESHOLD` currently is*: tx just above the constant must
125-
/// shift, tx just below must not (ty and tz are held at 0.0 and are not
126-
/// independently exercised here). The other two fixtures straddle it by
127-
/// roughly five orders of magnitude (1.0 vs 123456.0), so any threshold
128-
/// anywhere in between would satisfy them both — this fixture closes that
129-
/// gap for the *boundary behavior*.
123+
/// Pin the boundary's sharpness on EACH axis independently, *relative to
124+
/// whatever `LARGE_COORD_THRESHOLD` currently is*: a translation just above
125+
/// the constant on any one axis must shift, just below must not. The guard is
126+
/// a three-way conjunction, so one axis proves nothing about the other two.
127+
/// With only the x cases present, the `ty` and `tz` conjuncts could each be
128+
/// deleted outright and all 11 tests still passed (#2936). The other two
129+
/// fixtures straddle it by roughly five orders of magnitude (1.0 vs
130+
/// 123456.0), so any threshold anywhere in between would satisfy them both —
131+
/// this fixture closes that gap for the *boundary behavior*.
130132
///
131133
/// This does **not** pin the constant's *value*: every value used below is
132134
/// derived from `LARGE_COORD_THRESHOLD` itself, so the fixture is green for
@@ -151,6 +153,14 @@ fn the_large_coordinate_threshold_is_bracketed_on_both_sides() {
151153
// tell `<` from `<=` apart — both compile and pass identically
152154
// either way. This closes that gap.
153155
([LARGE_COORD_THRESHOLD, 0.0, 0.0], true),
156+
// The same three brackets on y and on z: the guard ANDs the three
157+
// axes, so each conjunct needs its own boundary.
158+
([0.0, LARGE_COORD_THRESHOLD + 0.5, 0.0], true),
159+
([0.0, LARGE_COORD_THRESHOLD - 0.5, 0.0], false),
160+
([0.0, LARGE_COORD_THRESHOLD, 0.0], true),
161+
([0.0, 0.0, LARGE_COORD_THRESHOLD + 0.5], true),
162+
([0.0, 0.0, LARGE_COORD_THRESHOLD - 0.5], false),
163+
([0.0, 0.0, LARGE_COORD_THRESHOLD], true),
154164
] {
155165
let [tx, ty, tz] = translation;
156166
let original = processing_result(
@@ -166,7 +176,7 @@ fn the_large_coordinate_threshold_is_bracketed_on_both_sides() {
166176
assert_eq!(
167177
result.mesh_coordinate_space.as_deref(),
168178
Some(SITE_LOCAL_MESH_COORDINATE_SPACE),
169-
"tx={tx} is past the threshold and must be shifted"
179+
"({tx}, {ty}, {tz}) is at or past the threshold and must be shifted"
170180
);
171181
let expected_a: Vec<f32> = original_positions_a
172182
.chunks_exact(3)
@@ -183,7 +193,7 @@ fn the_large_coordinate_threshold_is_bracketed_on_both_sides() {
183193
assert_eq!(
184194
result.mesh_coordinate_space.as_deref(),
185195
Some(RAW_IFC_MESH_COORDINATE_SPACE),
186-
"tx={tx} is inside the threshold and must be left alone"
196+
"({tx}, {ty}, {tz}) is inside the threshold and must be left alone"
187197
);
188198
assert_eq!(result.meshes[0].positions, original_positions_a);
189199
}

0 commit comments

Comments
 (0)