Record the sixth sweep, and the base every sweep should have used
PR #19's base is upstream/main at ca2b4b2, not the local `main`, which tracks the fork's divergent line and sits four merged pull requests behind it. Diffing against `main` shows #10, #12, #16 and #17 as this branch's work; that is how the parley text migration's undo path kept surfacing in sweeps. Git cannot record a pull request's base and `main` cannot be renamed, so the handoff carries the note and the checkout carries `git iris-base`/`git iris-diff`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
734c521f5d
commit
0ace0e017c
2 files changed
+133
-2
No files matched your search
+99
-2
@@ -6,6 +6,99 @@ nothing here is rediscovered. Each entry says who found it and when.
|
||||
it (settled design, the measurement method) belongs in `docs/LAYOUT.md`, and
|
||||
the current plan is in `docs/HANDOFF.md`.
|
||||
|
||||
## Sixth sweep: the shader boundary and the position widgets (2026-09-20)
|
||||
|
||||
Over what the five earlier rounds did not name -- the WGSL prelude and how
|
||||
it is assembled, the position widgets, `orientation/`, and the sensor walk.
|
||||
**Scoped against `upstream/main` at `ca2b4b2`, which is PR #19's real base**;
|
||||
the first half of this sweep used the local `main` and had to be redone, for
|
||||
which see "The base is `upstream/main`" in `docs/HANDOFF.md`. Two findings.
|
||||
The cold dump is byte-identical to `1096c31` and all three seed scans pass
|
||||
(400 at depth 5 in 69.07s, 1,000 at depth 6 in 169.29s, 2,000 at depth 4 in
|
||||
300.75s).
|
||||
|
||||
**A number both sides count in, written twice.** `module_source` already
|
||||
builds each shader's preamble from `iris_core`'s own constants, with a
|
||||
comment saying why: "a grid the two disagree about puts every coordinate
|
||||
somewhere else". The move-chain work then added, to the very file it
|
||||
prepends, a second copy of two of its own numbers -- `const MOVE_NONE` and
|
||||
`const CHAIN_LIMIT`, under "Keep in step with `iris_core::CHAIN_LIMIT`" --
|
||||
asking a reader by hand for what the mechanism beside it exists to do, and
|
||||
duplicating the reasoning `CHAIN_LIMIT`'s Rust declaration already carries.
|
||||
Both are injected now and the shader declares neither.
|
||||
`every_shader_validates` composes the real preamble so it covers the change;
|
||||
what it could never have caught is the two numbers drifting apart, which is
|
||||
now unrepresentable.
|
||||
|
||||
`MASK_NONE` went in beside them, replacing a bare `4294967295u` in `masked`
|
||||
-- where the CPU deliberately keeps `MaskIdx` and `MoveIdx` as separate
|
||||
types so the two cannot be swapped. **That literal pre-dates this PR**; it
|
||||
is a one-line drive-by in a block the PR was already rewriting, taken
|
||||
because leaving it means a named sentinel for one index and a magic number
|
||||
for its sibling four lines apart. Drop it if the scope matters more.
|
||||
|
||||
**A scroll positioning content it does not position.** Two halves, one
|
||||
mistake, both new in this PR -- the base's `Scroll` has none of this
|
||||
machinery. The author believed `Scroll` places its own content.
|
||||
|
||||
`content_len` is `answer_px.max(container_len)`, so it is never less than
|
||||
the box. Two lines on, `slack` was `(container_len - content_len).max(ZERO)`
|
||||
and `anchor` was `slack * align.rel()` -- provably always zero, whatever the
|
||||
alignment, with `moved` then testing `anchor != ZERO` for nothing.
|
||||
Instrumented at `1096c31`, a centred scroll over 50 px of content in a
|
||||
200 px box prints `slack=0 align=AxisAlign(0.5) anchor=0` and centres the
|
||||
content anyway: the framework does it, by placing the inner's answer in the
|
||||
whole box, which comes out as `rel 0.5 - px 25` and resolves at any length.
|
||||
The comment credited the arithmetic for behaviour it could not produce.
|
||||
|
||||
The same belief cost a redraw. The contract for content that fits was
|
||||
guarded by `align == AxisAlign::NEG`, on the reasoning that at any other
|
||||
alignment "it moves with every length the box takes and the drawing holds
|
||||
for that length alone". It does not move -- the framework's placement is a
|
||||
fraction of the box -- and the default alignment is the middle, so the
|
||||
common case was the guarded one. `Painter::px_len` holds a drawing to the
|
||||
length it read unless the widget says otherwise, so with no `holds` stated
|
||||
the scroll redrew on every box change. Measured with `distinct_widgets`: 1
|
||||
at `CENTER`, 0 at `TOP_LEFT`. Dropping the alignment test gives 0 at all
|
||||
three, which is what `a_fitting_scroll_holds_for_every_box_its_content_fits_in`
|
||||
asserts; it fails at `1096c31` on the `CENTER` case. `align` had no other
|
||||
reader, so the widget no longer asks its own alignment at all -- which is
|
||||
the tell that both halves were one mistake.
|
||||
|
||||
**`UiSpan::translated` and `UiRegion::translated`** arrived on this branch
|
||||
and are reachable only from each other, which is why a plain "is this name
|
||||
used anywhere else" scan does not see them: neither looks unused on its own.
|
||||
The region one carried a performance argument for a function nobody calls.
|
||||
Both deleted.
|
||||
|
||||
### Kept although they pre-date this PR
|
||||
|
||||
`GlyphAtlas::glyph_count`, `TextBuffer::new_empty` and the let-chain in
|
||||
`TextEdit::apply_event` that #10 had expanded into a nested `if`. All three
|
||||
were found under the wrong base and are outside #19's diff; Bryan said to
|
||||
keep them anyway (2026-09-20), since the two are dead either way and the
|
||||
third is a straight restoration.
|
||||
|
||||
### Tripped a rule and left as it stands
|
||||
|
||||
- `diag::untrace_widget` is new here with no caller, which is the test the
|
||||
`translated` pair was deleted by. Left: it is the "removed" half of what
|
||||
`trace_widget`'s own doc promises ("until explicitly removed or cleared"),
|
||||
and `clear_traced_widgets` is the "cleared" half and does have one. A rig
|
||||
drives this API from outside.
|
||||
- `UiSpan::flip` swaps `start.rel` with `end.rel` and `start.px` with
|
||||
`end.px`, where `Len` has exactly those two fields and one swap of the
|
||||
structs would do. Pre-dates this PR.
|
||||
- `UiVec2` translates under three names -- `shift`, `offset`, and
|
||||
`UiRegion::offset` -- beside `Len::offset`, which adds pixels to one end
|
||||
and is a different operation. Pre-existing, and one vocabulary is Bryan's
|
||||
call rather than a sweep's.
|
||||
- The nine `#[allow(unused_variables)]` are all on trait methods with empty
|
||||
default bodies, where the parameter names are the signature's
|
||||
documentation and underscoring them would hide it from implementors.
|
||||
- `GpuPages::update` grows before it drains and both go through the one
|
||||
queue, so the copy is ordered before the writes. Correct as it stands.
|
||||
|
||||
## Fifth sweep: the retained path, the renderer and the diagnostics (2026-09-20)
|
||||
|
||||
Over the parts the four earlier rounds did not read -- the renderer, the
|
||||
@@ -79,8 +172,12 @@ throughout, `LayoutLen::{is_px, is_only_leftover, declared, fills}` and
|
||||
same words share an answer" means, which is a design question.
|
||||
- `TextEdit`'s undo history pushes a whole copy of the text per changed
|
||||
keystroke and is never bounded, and `apply_event` clones the text on every
|
||||
event including the arrow keys. Both are on `main` verbatim and unchanged
|
||||
by this branch, so they belong to a change of their own.
|
||||
event including the arrow keys. **`apply_event` is not in this PR's diff at
|
||||
all**: it was last touched by #10 and #16, both already on `upstream/main`.
|
||||
It keeps resurfacing in sweeps because they diffed against the local `main`
|
||||
-- see "The base is `upstream/main`" in `docs/HANDOFF.md`. Bryan wants the
|
||||
unbounded push and the clone dealt with as a change of their own
|
||||
(2026-09-20).
|
||||
- `ActivationState::update` writes four arms where the `Start`/`On` and
|
||||
`End`/`Off` pairs are identical, and `is_off` is `!is_on`. Also verbatim
|
||||
from `main`.
|
||||
|
||||
Reference in new issue
Block a user