Record that padding is an inset and plan the frame-as-length representation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
1c52bc4e19
commit
f33981e1e6
2 files changed
+120
-43
No files matched your search
+110
-36
@@ -198,42 +198,30 @@ the rest are laid out) all narrow the frame.
|
||||
length until the span divides its room, and it narrows the frame at the
|
||||
placing ask instead.
|
||||
|
||||
### `Pad` remains an outset -- and is not yet what was decided
|
||||
### Padding is an inset, and the frame is a length while the box is a region
|
||||
|
||||
Bryan: padding goes outside what it pads; the pad forwards its frame less the
|
||||
padding and draws the child inside that area, so `rel(1.0)` inside `.pad(16)`
|
||||
inside a 450 px share is 418 px. **The experiment does not implement this.**
|
||||
`Pad` still forwards the frame whole and insets only the box:
|
||||
Bryan, 2026-09-18: padding is an inset. It subtracts from both the child's
|
||||
frame and its box and adds itself to the reported size, so `rel(1.0)` inside
|
||||
padding fills the parent without overflowing. A span's frame never subtracts
|
||||
siblings; only the padding subtracts from it. No outset kind and no mixed
|
||||
kind for now; the name stays `Pad`. Worked example, 900 px row:
|
||||
|
||||
```rust
|
||||
let inset = |lead: Px, trail: Px| {
|
||||
Place::Within(Part::Of(UiSpan::new(
|
||||
Len::from_parts(Rel::ZERO, lead),
|
||||
Len::from_parts(Rel::ONE, -trail),
|
||||
)))
|
||||
};
|
||||
let row = (rect(Color::RED).width(24), wtext(PARAGRAPH).wrap(true).pad(16)).span(Dir::RIGHT);
|
||||
```
|
||||
|
||||
so `rel(1.0)` there is 450 and overflows by the padding -- the clipped
|
||||
`text.rs` render. The decided rule cannot be written in the current
|
||||
representation without a cost Bryan has not seen:
|
||||
The text is asked in 900 − 24 − 32 = **844** px and wraps there; a
|
||||
`rel(1.0)` inside the same padding is 900 − 32 = **868** px and overflows the
|
||||
row by exactly the icon's width. With the pad in a share instead, both are
|
||||
the share less 32. An icon *after* the padded text overflows; a user who
|
||||
wanted otherwise meant `leftover`.
|
||||
|
||||
- Narrowing the child's frame by the padding (`narrow = FULL - 32px`) makes
|
||||
that length the child's *box* too, since a narrowed frame is its own box.
|
||||
In a share or a declared box, frame and box coincide and the rule holds.
|
||||
As a fixed child of a span measured from its cursor, the padded child is
|
||||
then asked in `row - 32` rather than `room - 32`: a padded wrapped text
|
||||
after a 24 px icon wraps as if the icon were not there and overflows the
|
||||
row by 24 px. That row is the commonest thing in the app.
|
||||
- Keeping `Part::Of` (status quo) keeps the text wrapping in the room and
|
||||
makes `rel(1.0)` under a pad mean the whole frame.
|
||||
- Having both -- `rel` of the frame less padding *and* a box that is the
|
||||
room less padding -- needs the frame to be a length and the box a region in
|
||||
the parent's coordinates, with `Part::From` scaling its span by the frame
|
||||
length. That is a coordinate rewrite of `painter.rs`/`render_state.rs`, not
|
||||
a widget change.
|
||||
|
||||
Ask Bryan which. Do not decide it in a worker session.
|
||||
**The experiment does not implement this**, and cannot in its representation:
|
||||
a narrowed frame is its own box there (`frame_and_extent` sets the extent to
|
||||
`FULL` of it), so 868 for `rel` and 844 for the wrap cannot both hold. `Pad`
|
||||
still insets only the box through `Part::Of` and forwards the frame whole,
|
||||
which makes `rel(1.0)` under it 900. Step 2 below is the representation
|
||||
change that expresses the rule.
|
||||
|
||||
### A share never adds room beyond the deciding box
|
||||
|
||||
@@ -260,7 +248,95 @@ SHRINK_DEPTH=<depth> SHRINK_CASE=<case>`), pinned as a named test in
|
||||
`tests/cases/unsettled.rs`, then fixed under the rule above. Do not add a
|
||||
second draw back.
|
||||
|
||||
### 2. Render and replay
|
||||
### 2. Make the frame a length and the box a region
|
||||
|
||||
The frame stops being a coordinate system. Coordinates start at a region
|
||||
node (or the root) and every box under it is a region in that node's
|
||||
coordinates; a widget's frame is a *length* of the node's box, per axis, and
|
||||
is only what fractions resolve against:
|
||||
|
||||
```rust
|
||||
pub struct Painter<'a> {
|
||||
/// This widget's frame as a length of its region node's box, per axis:
|
||||
/// what a fraction it declares or reports, and a `From` span under it,
|
||||
/// are fractions of. A length and not a region, so the box need not be
|
||||
/// the frame -- padding takes from both without either becoming the other.
|
||||
pub(super) frame: UiVec2,
|
||||
/// Where this widget's drawing goes, in the node's coordinates.
|
||||
pub(super) extent: UiRegion,
|
||||
/// The node's box in pixels. The frame in pixels is `frame.to_px(node_px)`
|
||||
/// and the box's is `extent.size().to_px(node_px)`.
|
||||
pub(super) node_px: PxVec2,
|
||||
...
|
||||
}
|
||||
|
||||
impl Part {
|
||||
/// Where it lands in the coordinates `extent` is in. A `From` span is in
|
||||
/// the asking widget's frame lengths, which `frame` says in the node's.
|
||||
pub(crate) fn of(self, extent: UiSpan, frame: Len) -> UiSpan {
|
||||
match self {
|
||||
Self::All => extent,
|
||||
Self::From(span) => UiSpan::new(
|
||||
extent.start + span.start.within_len(frame),
|
||||
extent.start + span.end.within_len(frame),
|
||||
),
|
||||
Self::Of(span) => span.within(&extent),
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
A child's frame is `narrow.within_len(parent.frame)` where the caller or a
|
||||
rule narrowed it, else the parent's; a declared length also decides the box
|
||||
(`len` placed in the part by alignment, as `frame_and_extent` does today),
|
||||
where a caller's `narrow` decides the frame alone and the place decides the
|
||||
box. `Pad` then says both halves of the rule:
|
||||
|
||||
```rust
|
||||
let inset = |lead: Px, trail: Px| {
|
||||
Place::Within(Part::Of(UiSpan::new(
|
||||
Len::from_parts(Rel::ZERO, lead),
|
||||
Len::from_parts(Rel::ONE, -trail),
|
||||
)))
|
||||
};
|
||||
let less = |lead: Px, trail: Px| Some(Len::from_parts(Rel::ONE, -(lead + trail)));
|
||||
let inner = painter
|
||||
.widget_at(
|
||||
&self.inner,
|
||||
[less(self.padding.left, self.padding.right), less(self.padding.top, self.padding.bottom)],
|
||||
[inset(self.padding.left, self.padding.right), inset(self.padding.top, self.padding.bottom)],
|
||||
)
|
||||
.size();
|
||||
```
|
||||
|
||||
What this changes in the core, and why it is not merely more code:
|
||||
|
||||
- `Painter::resolve` becomes `region.within(&self.extent)`; the second
|
||||
`within` through the frame goes.
|
||||
- `in_parent` no longer maps a child's frame holds through the frame length
|
||||
per level: every widget under one node holds for the node's pixel box, so
|
||||
the holds `and` directly, and only a region node maps through its box.
|
||||
- `Part::of` gains one `within_len` per `From` end. `Of` and `All` are as
|
||||
they were.
|
||||
- `DrawInfo::frame`/`frame_abs`, `ActiveData::frame`/`frame_abs`, `local`
|
||||
in `Placing`, `recompose_subtree` and `asked_px` all change meaning:
|
||||
`frame` is a `UiVec2` length, `frame_abs` goes, a node's move entry is the
|
||||
node's frame length anchored at its box start (so `rel` inside means the
|
||||
frame), and `asked_px` walks to the node rather than the root.
|
||||
- `Stack` needs a part that is "a box of this length, placed by the child's
|
||||
alignment" (`Part::Sized(Len)`, the same thing a declared length does),
|
||||
since `narrow` no longer makes the box.
|
||||
|
||||
The random rig and the shrunk tests do not change. Expected precision is the
|
||||
same as today (one `rel × rel` per level of narrowing). Checks: the suite,
|
||||
the fast oracle, the 400/5 shrinker, then both long fuzzers; the `many`,
|
||||
`resize` and `cold` counters must not move by more than the reposition work
|
||||
this removes; and two new tests, `rel(1.0)` inside `.pad(16)` after a 24 px
|
||||
icon is 868 px while the wrapped text beside it wraps at 844, and the same
|
||||
inside a 450 px share is 418 for both. `examples/text.rs` then renders
|
||||
inside its padding.
|
||||
|
||||
### 3. Render and replay
|
||||
|
||||
Read the installed graphics skill and confirm the renderer. Render `view`,
|
||||
`minimal`, `random`, `tabs` and `text` at 1920x1200 against `34cafb6` and
|
||||
@@ -269,7 +345,7 @@ render at the same size (commands under **Full verification** below). A
|
||||
text placed by re-expression rather than a second draw is the change most
|
||||
likely to show here; inspect every intentional difference and record it.
|
||||
|
||||
### 3. Rename and delete
|
||||
### 4. Rename and delete
|
||||
|
||||
Rename `ActiveData::offer_part` to `part`, `offer_place` to `asked`, `place`
|
||||
to `placed`, and `DrawInfo` likewise; delete `ActiveData::measured` in favour
|
||||
@@ -277,7 +353,7 @@ of reading `answer`; delete `answers_at` if `resize` is its only caller and
|
||||
inline it. Every use was written against the old names on purpose to keep
|
||||
the probe's diff readable; do this as one mechanical commit. Suite, oracle.
|
||||
|
||||
### 4. Restore the expected retained cost
|
||||
### 5. Restore the expected retained cost
|
||||
|
||||
Work counters at `3091fb8`, seed 1 and 13, depth 8, widget draws / distinct
|
||||
widgets, beside `e44dea3` (#18) and `49cec82` (the branch head before this):
|
||||
@@ -321,7 +397,7 @@ at seed 1 draws 40 times where #18 drew 13. Two mechanisms, both understood:
|
||||
Report every phase at both seeds, work counters first, medians only when the
|
||||
work agrees.
|
||||
|
||||
### 5. Full verification and landing
|
||||
### 6. Full verification and landing
|
||||
|
||||
Run, in the experiment checkout:
|
||||
|
||||
@@ -377,8 +453,6 @@ pin only when the Iris change is ready, and push every coherent commit.
|
||||
|
||||
## Follow-on work, not part of this repair
|
||||
|
||||
- The `Pad` decision above, and the coordinate rewrite if Bryan wants both
|
||||
halves of the rule.
|
||||
- CPU round-to-nearest and shader nearest-pixel snapping are approved as one
|
||||
separately verified change. Neither has landed. Re-derive `Holds::through`
|
||||
for the new rounding and run both long fuzzers plus the render set.
|
||||
|
||||
Reference in new issue
Block a user