Offered by PR #773's independent reviewer at round 2 as a middle option between what #773 does
and the newtype it declined. Explicitly marked reasoned-not-prototyped by the reviewer — the
cost claim below is measured, the design is not built. Treat the shape as a proposal to be
tested, not a verified plan.
Background — what #773 has, and what it does not
#773 fixed a static model being lifted a full model height above its stored z. What holds the
fix is a source-text pin, every_static_placement_in_pass_rs_is_written_exactly_as_reviewed,
which requires each of the four pass.rs call sites to be spelled as reviewed. That is tier 3
in this repo's verification hierarchy: it bounds four call sites in one file, does not reach
another file, and does not constrain what the names in those spellings denote. The over-lift is
not unrepresentable — the merged code says so, and two routes back to it were measured green.
Why the newtype was declined, and why the reason is sound
Making the bad state unrepresentable would mean static_placement taking the model's bounds as
a value only the loader can mint. The reviewer found the decisive argument, which the PR body
did not make:
floating_placement.rs is an integration test, so it links as an external crate and cannot
see #[cfg(test)] mints. An unforgeable newtype therefore forces either a pub mint that
pass.rs can also call — defeating the point — or a feature flag. Declining was correct.
Cost also holds independently: there are only two GpuStaticModel construction sites,
renderer.rs:751 and render_model.rs:851.
The proposal
Have static_placement take a plain ModelBounds parameter instead of a loose y_bottom
float, so there is no lift argument at the call site to do arithmetic in. This does not make
the state unrepresentable — a caller can still construct a wrong ModelBounds — but it removes
the specific shape both measured evasions used, which was arithmetic on a float argument at the
call site.
What to decide before building
Whether this buys enough over the existing source-text pin to be worth touching a shared helper
and two construction sites. If it does not, close this as wontfix and leave the pin — that is
a legitimate outcome and the reviewer did not argue otherwise.
Verification
If built: re-run the two evasions #773 measured green (passing 2.0 * model.y_extent * p.mesh_scale as visual_scale, and passing model.y_bottom + model.y_extent as y_bottom)
and record which, if either, the new shape now makes unwritable. If neither, say so plainly
rather than claiming a guarantee the change does not have — overstating exactly this boundary
is what two of #773's own commit messages had to retract.
Refs #768, #773.
Offered by PR #773's independent reviewer at round 2 as a middle option between what #773 does
and the newtype it declined. Explicitly marked reasoned-not-prototyped by the reviewer — the
cost claim below is measured, the design is not built. Treat the shape as a proposal to be
tested, not a verified plan.
Background — what #773 has, and what it does not
#773 fixed a static model being lifted a full model height above its stored z. What holds the
fix is a source-text pin,
every_static_placement_in_pass_rs_is_written_exactly_as_reviewed,which requires each of the four
pass.rscall sites to be spelled as reviewed. That is tier 3in this repo's verification hierarchy: it bounds four call sites in one file, does not reach
another file, and does not constrain what the names in those spellings denote. The over-lift is
not unrepresentable — the merged code says so, and two routes back to it were measured green.
Why the newtype was declined, and why the reason is sound
Making the bad state unrepresentable would mean
static_placementtaking the model's bounds asa value only the loader can mint. The reviewer found the decisive argument, which the PR body
did not make:
floating_placement.rsis an integration test, so it links as an external crate and cannotsee
#[cfg(test)]mints. An unforgeable newtype therefore forces either apubmint thatpass.rscan also call — defeating the point — or a feature flag. Declining was correct.Cost also holds independently: there are only two
GpuStaticModelconstruction sites,renderer.rs:751andrender_model.rs:851.The proposal
Have
static_placementtake a plainModelBoundsparameter instead of a loosey_bottomfloat, so there is no lift argument at the call site to do arithmetic in. This does not make
the state unrepresentable — a caller can still construct a wrong
ModelBounds— but it removesthe specific shape both measured evasions used, which was arithmetic on a float argument at the
call site.
What to decide before building
Whether this buys enough over the existing source-text pin to be worth touching a shared helper
and two construction sites. If it does not, close this as
wontfixand leave the pin — that isa legitimate outcome and the reviewer did not argue otherwise.
Verification
If built: re-run the two evasions #773 measured green (passing
2.0 * model.y_extent * p.mesh_scaleasvisual_scale, and passingmodel.y_bottom + model.y_extentasy_bottom)and record which, if either, the new shape now makes unwritable. If neither, say so plainly
rather than claiming a guarantee the change does not have — overstating exactly this boundary
is what two of #773's own commit messages had to retract.
Refs #768, #773.