Record the round's last two findings, and the review the old code is owed
`pre-submit-review` arrived after the Rust port and most of Iris were written, so all of that went in unreviewed and no sweep has covered it. The surface-texture defect is the argument for a pass of its own. Nothing in Iris is kept for the app's sake either: the app is to be rewritten against the new API rather than ported call by call.
This commit is contained in:
1 parent
718923c217
commit
734c521f5d
2 files changed
+47
-23
No files matched your search
+19
-9
@@ -6,7 +6,7 @@ found is in `docs/LAYOUT_LOG.md`.
|
|||||||
|
|
||||||
## The Iris layout repair is submitted
|
## The Iris layout repair is submitted
|
||||||
|
|
||||||
**Iris PR #19** (`layout/one-ask`) replaces closed #18. The tip is `7502176`,
|
**Iris PR #19** (`layout/one-ask`) replaces closed #18. The tip is `1096c31`,
|
||||||
and past the reviewed `cadfba0` it is seven rounds, each described in
|
and past the reviewed `cadfba0` it is seven rounds, each described in
|
||||||
`docs/LAYOUT_LOG.md`:
|
`docs/LAYOUT_LOG.md`:
|
||||||
|
|
||||||
@@ -24,9 +24,10 @@ and past the reviewed `cadfba0` it is seven rounds, each described in
|
|||||||
read, three reuse rejections the diagnostics could not see, and a fuzz case
|
read, three reuse rejections the diagnostics could not see, and a fuzz case
|
||||||
that ran only in the long scan.
|
that ran only in the long scan.
|
||||||
- **A sweep over the renderer, the text store and the retained path**,
|
- **A sweep over the renderer, the text store and the retained path**,
|
||||||
`d8d5122` through `7502176` -- a contract kept where the new window left
|
`d8d5122` through `1096c31` -- a contract kept where the new window left
|
||||||
it out, a surface configured under its own texture, a counter naming the
|
it out, a surface configured under its own texture, a counter naming the
|
||||||
wrong contract, and three things nothing reads.
|
wrong contract, things nothing reads, and a question asked through a value
|
||||||
|
rather than a reference.
|
||||||
|
|
||||||
The settled design of the last three is in `docs/LAYOUT.md` under "Three
|
The settled design of the last three is in `docs/LAYOUT.md` under "Three
|
||||||
names, and the one argument that says them". Bryan settled the API over
|
names, and the one argument that says them". Bryan settled the API over
|
||||||
@@ -61,14 +62,23 @@ a compile error, which reads exactly like a fuzzer failure.
|
|||||||
`pre-submit-review` over each round's changes, and apply the gate above to
|
`pre-submit-review` over each round's changes, and apply the gate above to
|
||||||
whatever each one touched. The ordinary oracle does not replace absolute
|
whatever each one touched. The ordinary oracle does not replace absolute
|
||||||
geometry and retained-primitive expectations.
|
geometry and retained-primitive expectations.
|
||||||
2. **Integrate the app's Iris capabilities before changing its pin.**
|
2. **A review of everything written before the review gate existed.**
|
||||||
|
`pre-submit-review` and the rule that nothing is submitted unreviewed
|
||||||
|
arrived on 2026-09-13, well after the Rust port and most of Iris were
|
||||||
|
written, so all of that code went in unreviewed and none of the five
|
||||||
|
sweeps above covered more than the layout branch. It wants a pass of its
|
||||||
|
own (Bryan, 2026-09-20). The surface-texture defect in `02048ea` is the
|
||||||
|
argument: nothing about that arm was hard, and it was written wrong
|
||||||
|
anyway, which is what a first reader catches and a later sweep of some
|
||||||
|
other subject does not.
|
||||||
|
3. **Integrate the app's Iris capabilities before changing its pin.**
|
||||||
`32f6ad8` has 45 commits not reachable from the review branch; shared UI
|
`32f6ad8` has 45 commits not reachable from the review branch; shared UI
|
||||||
ownership, richer masks, Android support, and app-side performance work
|
ownership, richer masks, Android support, and app-side performance work
|
||||||
must survive the integration. The app then absorbs the branch's
|
must survive the integration. What has to survive is those capabilities,
|
||||||
vocabulary and container API, all of it already upstream: `rest` ->
|
not the calls the app makes today: the app is to be largely rewritten
|
||||||
`leftover`, `SizeRule` beside widgets, alignment properties, the removal
|
against the new API rather than ported call by call, so nothing in Iris
|
||||||
of `OrthoSize`, region/placement/rel_base, and `PlaceDesc`.
|
is kept alive for the app's sake (Bryan, 2026-09-20).
|
||||||
3. **Round-to-nearest**, CPU and shader together as one verified change.
|
4. **Round-to-nearest**, CPU and shader together as one verified change.
|
||||||
Bryan approved it on 2026-09-17 and neither half has landed; the
|
Bryan approved it on 2026-09-17 and neither half has landed; the
|
||||||
derivation, the form to use and what to re-check are in `docs/LAYOUT.md`
|
derivation, the form to use and what to re-check are in `docs/LAYOUT.md`
|
||||||
under "Rendering the grid (pending)".
|
under "Rendering the grid (pending)".
|
||||||
|
|||||||
+28
-14
@@ -10,9 +10,10 @@ the current plan is in `docs/HANDOFF.md`.
|
|||||||
|
|
||||||
Over the parts the four earlier rounds did not read -- the renderer, the
|
Over the parts the four earlier rounds did not read -- the renderer, the
|
||||||
text store, the input default, the harness -- and once more over `redraw`.
|
text store, the input default, the harness -- and once more over `redraw`.
|
||||||
Four findings, `d8d5122` through `7502176`. The cold dump is byte-identical
|
Six findings, `d8d5122` through `1096c31`, the last two from Bryan's reading
|
||||||
to `781199a` and all three seed scans pass (400 at depth 5 in 69.45s, 1,000
|
of the first four. The cold dump is byte-identical to `781199a` and all three
|
||||||
at depth 6 in 214.69s, 2,000 at depth 4 in 419.50s).
|
seed scans pass (400 at depth 5 in 69.45s, 1,000 at depth 6 in 214.69s, 2,000
|
||||||
|
at depth 4 in 419.50s).
|
||||||
|
|
||||||
**A contract was kept where it no longer held** (`d8d5122`). The sibling of
|
**A contract was kept where it no longer held** (`d8d5122`). The sibling of
|
||||||
`713e3e7`, in the same function. `redraw` keeps the narrower of the old and
|
`713e3e7`, in the same function. `redraw` keeps the narrower of the old and
|
||||||
@@ -37,6 +38,10 @@ the swapchain. The texture is good for that frame, so it is drawn with and
|
|||||||
presented and the rebuild happens after `present` consumes it. Not
|
presented and the rebuild happens after `present` consumes it. Not
|
||||||
reproducible on demand here; the claim rests on wgpu's own documented panic.
|
reproducible on demand here; the claim rests on wgpu's own documented panic.
|
||||||
|
|
||||||
|
This one is the reason for the sweep recorded under "The code written before
|
||||||
|
the review gate" below: nothing about the arm was hard, and it was written
|
||||||
|
that way anyway.
|
||||||
|
|
||||||
**A counter named the wrong contract** (`9b4cc32`). `AxisHolds` is four
|
**A counter named the wrong contract** (`9b4cc32`). `AxisHolds` is four
|
||||||
contracts and `diag::outside` counted three: a refusal because this window
|
contracts and `diag::outside` counted three: a refusal because this window
|
||||||
is outside the range the drawing was made for bumped "reuse outside: a rel
|
is outside the range the drawing was made for bumped "reuse outside: a rel
|
||||||
@@ -44,20 +49,29 @@ base". A window range is pixels and a rel base pin is a window-unit length an
|
|||||||
unchanged window can still change, so the rig answered "why did that
|
unchanged window can still change, so the rig answered "why did that
|
||||||
redraw?" with the wrong one for every resize. Same class as `8088a1f`.
|
redraw?" with the wrong one for every resize. Same class as `8088a1f`.
|
||||||
|
|
||||||
**Three things nothing reads** (`7502176`). `Axis::pair` and
|
**Things nothing reads** (`7502176`, `1096c31`). `Axis::pair`,
|
||||||
`RegionAlign::NEAR` arrived on this branch with no caller and never got one;
|
`RegionAlign::NEAR` and `Painter::text_data` arrived on this branch with no
|
||||||
`Holds::contains` took `&self` where its five siblings on the same `Copy`
|
caller and never got one. The comment beside a span's cross-axis accumulator
|
||||||
pair take `self`. The comment beside a span's cross-axis accumulator said a
|
said a scalable child "makes Children scalable too" -- `Children` names
|
||||||
scalable child "makes Children scalable too" -- `Children` names nothing in
|
nothing in this repository, and what it makes scalable is the span.
|
||||||
this repository, and what it makes scalable is the span.
|
|
||||||
|
`text_data` was held back a round on the grounds that it is the only way a
|
||||||
|
widget inside `draw` can reach `TextData`, and the app's integration might
|
||||||
|
want it. That reasoning is wrong: nothing in iris is kept for the app's sake,
|
||||||
|
because the app is to be largely rewritten against this API rather than
|
||||||
|
ported call by call (Bryan, 2026-09-20).
|
||||||
|
|
||||||
|
**A question asked through a value** (`445287c`). `7502176` moved
|
||||||
|
`Holds::contains` from `&self` to `self` to match its five siblings, which
|
||||||
|
was the wrong way to reconcile them. A method taking `self` can only be
|
||||||
|
called on a value, so a caller holding a reference has to dereference to ask
|
||||||
|
-- `Copy` or not (Bryan, 2026-09-20). Every method that answers a question
|
||||||
|
about a value now takes `&self`: `Holds`, `AxisHolds` and `LayoutHolds`
|
||||||
|
throughout, `LayoutLen::{is_px, is_only_leftover, declared, fills}` and
|
||||||
|
`Size::within_box`. Builders that return a changed copy still take `self`.
|
||||||
|
|
||||||
### Tripped a rule and left as it stands
|
### Tripped a rule and left as it stands
|
||||||
|
|
||||||
- `Painter::text_data` has no caller either, and is the only way a widget in
|
|
||||||
`draw` can reach `TextData` at all (`painter.rsc` is private). It replaces
|
|
||||||
`main`'s `text_resources`, which the app does not call today but which the
|
|
||||||
pending integration may want. Deleting a framework's one escape hatch to
|
|
||||||
the text store while the app is still on the old pin is not a sweep's call.
|
|
||||||
- `TextData`'s spare store clones the whole string into `Placed` on every
|
- `TextData`'s spare store clones the whole string into `Placed` on every
|
||||||
re-break. Bounded at 128 entries, but the clone is per re-break and
|
re-break. Bounded at 128 entries, but the clone is per re-break and
|
||||||
proportional to the text; a transcript-sized text would pay it on every
|
proportional to the text; a transcript-sized text would pay it on every
|
||||||
|
|||||||
Reference in new issue
Block a user