From 42a4cb70f9899bebd5d59f2538f4002c2019434e Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 05:06:53 -0700 Subject: [PATCH 1/9] docs(followups): campaign plan for draining open follow-ups Triage workflow (6 cluster analyzers -> synthesis -> skeptical review) classified every open follow-up in follow-ups.md. 12 actionable slices across layout (sequential, shared systems.rs), render (sequential, shared PackedInstance contract), and text-editing tracks. Speculative/unused items (position_try_max_depth cap, multi-ref reftest aggregation, golden-prune) deferred with documented re-open triggers; superseded items doc-flipped; subsystem/renderer-blocked items confirmed blocked. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- docs/plans/2026-06-18-followups-drain.md | 72 ++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 docs/plans/2026-06-18-followups-drain.md diff --git a/docs/plans/2026-06-18-followups-drain.md b/docs/plans/2026-06-18-followups-drain.md new file mode 100644 index 0000000..00d23b3 --- /dev/null +++ b/docs/plans/2026-06-18-followups-drain.md @@ -0,0 +1,72 @@ +# Buiy follow-ups drain — campaign plan + +**Date:** 2026-06-18 +**Status:** active +**Spec:** realizes the named deferrals in `docs/plans/follow-ups.md` against their originating specs (layout, render-pipeline, text-editing). + +**Goal:** Drain every *actionable* open follow-up in `docs/plans/follow-ups.md`, update the spec + follow-ups entry as each lands, and explicitly re-classify the ones that are blocked, superseded, speculative, or deliberately deferred — so the backlog reflects reality. + +**Architecture:** Slices are grouped into tracks by file-locality. The single ~5700-line `crates/buiy_core/src/layout/systems.rs` is the master serialization point — every layout follow-up edits it, so the layout track is **strictly sequential**. Render and text-editing tracks own disjoint file domains. Execution per slice: a workflow drafts a TDD plan → fresh skeptical plan review → fix → TDD implement task-by-task with per-task review (no commits) → **I run the full gate myself** (headless `xvfb-run cargo test --workspace`; GPU `--ignored` lane for slices with `#[ignore]` GPU tests) → commit → PR. The per-slice full-gate discipline is load-bearing: it has caught cross-phase regressions that per-task reviews missed. + +This plan was produced by the `followups-triage` workflow (6 cluster analyzers → synthesis → skeptical review). The review's one MAJOR finding (render PackedInstance contract coupling) is folded into the render track below. + +--- + +## Build list (actionable — ordered) + +### Track LAYOUT — sequential (shared `layout/systems.rs`), branch `followups-layout` + +1. **Refactor `anchor_resolution` into sub-helpers** — behavior-preserving base, sequenced FIRST so the anchor body-edits rebase onto clean helpers. `build_anchor_edge_map` / `apply_anchor_broken_markers` / `emit_anchor_warns`. Pinned by the existing anchor suite staying green. (`systems.rs`) +2. **`anchor-size()` term** — add `Length::AnchorSize(AxisDimension)` + `AxisDimension{Width,Height}`; resolve against the per-try anchor box (plumbing already in scope at `try_anchored_position`); retire/repurpose `AnchorErrorKind::AnchorSizeUsed`. **Reverses the § 3.4 v1.x deferral** — update spec § 3.4 + README § 5. Drop the over-specified `AnchorRef` payload from the sketch. (`types.rs`, `systems.rs`, `tests/layout_anchor_positioning.rs`) +3. **`AnchorRef::Entity` end-to-end test** — test-only; fills the coverage gap (all 11 existing cases use `AnchorRef::Name`). Add a direct-ref positive + a despawned/`Display::None` `TargetMissing` negative. (`tests/layout_anchor_positioning.rs`) +4. **Sticky `Length::Cq*` inset resolution** — reuse `resolve_cq_unit_px`; sticky's CQ frame is the sticky entity's own nearest CQ ancestor. (`systems.rs`, `tests/layout_sticky.rs`) +5. **Sticky both-top-and-bottom dual clamp** — band-clamp in `compute_sticky_displacement`; flip the `sticky_both_top_and_bottom_inset_top_wins` regression test. (`systems.rs`, `tests/layout_sticky.rs`) +6. **Sticky inside sticky** — `world_position` consults `PostTaffyPositionOverrides` when walking ancestors; depth-sort so outer sticky resolves before inner. (`systems.rs`, `tests/layout_sticky.rs`) +7. **will-change SC trigger former** — both will-change headings as ONE `forms_stacking_context` edit; an SC-forming `WillChangeProperty` forms a `StackingContext`. The **layer-promotion half stays deferred** (no composition-layer/`RenderLayers` concept exists in render/) — flip only the SC-former half in follow-ups.md. (`systems.rs`, `tests/layout_stacking.rs`) +8. **Non-px translate units in `compose_transform`** — resolve percent translate against the entity's own resolved box; `Cq*` translate split out as a residual note. (`systems.rs`, `tests/layout_transforms.rs`) + +### Track RENDER — **sequential** (shared PackedInstance contract), branch `followups-render` + +> **Hard constraint (review MAJOR):** the two slices are NOT parallel-safe despite disjoint files. UiTransform grows `PackedInstance` (+affine) and degraded-groups re-tints alpha at float index 7. UiTransform lands FIRST; its affine columns are appended **after** the existing 13 floats so alpha stays at index 7; the color/alpha offset is read from a **named const** (not a literal). Degraded-groups rebases onto the new layout. + +1. **UiTransform affine paint** — extract the full affine; `PackedInstance` basis columns appended after the existing floats; quad/shadow WGSL applies the affine about `TransformOrigin` (default 50% 50%) before the logical→clip map; GPU rotate reftest. **Scope = transform-paint only**; PAINT-clip is already done (`clip.rs:196`), perspective/`Preserve3d`/`BackfaceVisibility` stay C-tier deferred (narrow the follow-up, don't close it). (`extract.rs`, `instance.rs`, `pipeline.rs`, `shader.wgsl`, tests) +2. **Degraded effect groups forward-composite flat** — re-route degraded group ranges into the flat draw with per-instance opacity folded in (spec § 2.3 mandates forward-compositing; skip-as-degradation contradicts the landed spec). **Design note first:** fold opacity via **re-tint-in-place** in `prepare_effect_groups` (lower risk than reordering `prepare_effect_groups` ahead of `prepare_buiy_instances`). Re-tint reads the alpha offset from the named const. (`compositor.rs`, `node.rs`, `prepare.rs`, tests) + +### Track TEXT-EDITING — disjoint files (impl serialized in-worktree for FS safety), branch `followups-text` + +1. **BiDi split caret secondary indicator** — `secondary_caret_rect_for` from glyph-level affinity geometry (`cursor_from_glyph_left/right`), `SecondaryCaretVisual` seat, extract a second solid stamp. The primary caret is already correct; this adds the mixed-direction secondary indicator only. (`text/edit/caret.rs`, `text/components.rs`, `text/extract.rs`, tests) +2. **Compose-over-selection** — `delete_selection` (as one undo unit, paired with the composition group per § 6.2c) before the first preedit splice, then re-anchor the span. **Revised contract:** emit `TextChanged` when the pre-splice delete removed text. (`text/edit/ime.rs`, `text/edit/state.rs`, tests) +3. **HTML + image clipboard flavors** — `ClipboardProvider` html get/set; image behind a `clipboard-image` cargo feature. arboard 3.6.1 `Get::html()` is on the cross-platform base builder (OQ#3 resolved) — sanity-confirm the locked version first. (`text/edit/clipboard.rs`, `input.rs` copy/cut/paste block, `command.rs`, `Cargo.toml` ×2, tests) + +--- + +## Deferred — documented, not built (speculative / unused-now) + +These are flagged because building them now is the speculative work the dev guidelines warn against. Each follow-ups.md entry is updated with the reason + a concrete re-open trigger. + +- **`position_try_max_depth` cap** — both spec § 3.5 and README § 5 gate it on "if profiling surfaces a hot path"; no profiling evidence exists. An unused knob. **Re-open trigger:** a measured deeply-nested-fallback hot path. +- **Multi-reference reftest aggregation** — `reftests.md` notes no current pairing needs it; tested-but-unused machinery. **Re-open trigger:** a real logical-vs-physical (≥2 reference) pairing appears. +- **`golden-prune` advisory bin** — corpus is 2 cells; nothing to prune. **Re-open trigger:** golden corpus grows enough that stale positives are plausible. + +## Doc-flip → LANDED (already done in code, follow-up entry stale) + +- **Anchor target IS sticky/table/multicol** — closed by Phase 7 Task 9 (`systems.rs:1757` reads `PostTaffyPositionOverrides.by_entity` first; tested in `layout_sticky.rs::anchor_target_is_sticky_anchored_tracks_displaced_position`). +- **Node-draw model: per-entity clip + composite passes** — superseded by Option C (R8b per-instance fragment-discard clip) + R9 effect-compositor GPU orchestration (`c0a5fe0`). + +## Confirmed blocked — stay deferred (gated on unbuilt subsystems / renderer features) + +- Cross-window anchor targets, per-window top layer → `buiy-window-and-surface-design` (not chartered). +- Sticky em/rem/Vh/Vw/Vmin/Vmax insets → those `Length` variants don't exist (Phase 10 viewport units + a font phase). +- `clear_warned_once_on_exit` wire-up → `BuiyState`/`BuiyExit` lifecycle states don't exist. +- R11 forced-colors BoxShadow draw-skip, shadow-blur residue golden, color-emoji residue golden, FC BoxShadow visual reftest → no real BoxShadow/color-emoji extract+draw pipeline. +- Multi-range selection *behavior* → needs an N-editor/N-selection design decision; also reopens the caret.rs surface BiDi-caret owns. +- Object-store golden migration → scale trigger (>50 MB / >500 positives) not fired; `goldens.md` forbids building it now. + +--- + +## Done criteria (per slice) + +1. New/changed tests written test-first and green at the lowest tier (headless `tests/*.rs`; GPU tests `#[ignore]`). +2. Full gate green: `xvfb-run -a cargo test --workspace` + `cargo fmt --check` + `cargo clippy -D warnings` + `cargo doc`; GPU `--ignored` lane for slices with GPU tests (RX 6700 XT / lavapipe). +3. Spec touchpoint updated (remove the deferral / close the open question). +4. The follow-ups.md entry flipped to **LANDED** with a one-paragraph "as landed" note. From 7cf24630cd285b8cc2f2062b722f61a6632ed0c0 Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 07:23:34 -0700 Subject: [PATCH 2/9] refactor(layout): extract anchor_resolution into sub-helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behavior-preserving extraction of the ~265-line anchor_resolution into three private free helpers, leaving the system a thin driver: - build_anchor_edge_map (steps 2+3: resolve_target + entity_epochs closures, edge insert + TargetMissing warn, kahn_anchor_sort, InCycle/dropped_targets) - apply_anchor_broken_markers (step 6: the three idempotent LayoutAnchorBroken marker loops, order + cleanup guard verbatim) - emit_anchor_warns (step 7: dedupe gate + exhaustive AnchorErrorKind warn match) The step-5 topological walk stays inline (most entangled body; out of the follow-up's named scope). dropped/dropped_targets keep HashSet semantics (mirrors kahn_anchor_sort's return). Zero observable change — the existing anchor suite is the red/green signal; no new tests. Gives later anchor slices clean helpers to rebase onto. First slice of the follow-ups-drain campaign (docs/plans/2026-06-18-followups-drain.md). follow-ups.md entry flipped to LANDED. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- crates/buiy_core/src/layout/systems.rs | 163 +++++++++++++++++-------- docs/plans/follow-ups.md | 43 +++++-- 2 files changed, 146 insertions(+), 60 deletions(-) diff --git a/crates/buiy_core/src/layout/systems.rs b/crates/buiy_core/src/layout/systems.rs index 8da3386..4ca27ff 100644 --- a/crates/buiy_core/src/layout/systems.rs +++ b/crates/buiy_core/src/layout/systems.rs @@ -1653,51 +1653,10 @@ pub(super) fn anchor_resolution( .map(|w| Vec2::new(w.resolution.width(), w.resolution.height())) .unwrap_or(Vec2::splat(f32::MAX)); - // 2. Build edge map. The Kahn helper does its own pre-pass for - // external target nodes (D10), so we don't insert plain-Node - // targets here. - let mut edges: std::collections::HashMap> = - std::collections::HashMap::new(); - let mut new_warns: Vec<(Entity, AnchorErrorKind)> = Vec::new(); - - // Helper: target resolution honoring Display::None (D9). Returns - // Some(entity) only when the target is name-resolvable AND not - // Display::None. - let resolve_target = |r: &AnchorRef| -> Option { - let candidate = match r { - AnchorRef::Entity(t) => Some(*t), - AnchorRef::Name(n) => reg.find_entity_by_name(n), - }?; - if let Ok(Display::None) = display_query.get(candidate) { - return None; - } - Some(candidate) - }; - - for (e, anchor, _) in anchored_query.iter() { - let target = anchor.position_anchor.as_ref().and_then(&resolve_target); - edges.insert(e, target); - if anchor.position_anchor.is_some() && target.is_none() { - new_warns.push((e, AnchorErrorKind::TargetMissing)); - } - } - - // 3. Kahn sort. The helper handles external-target pre-pass and - // cycle-edge dropping. - let entity_epochs_fn = |e: Entity| reg.entity_epoch(e); - let (order, dropped) = kahn_anchor_sort(&edges, &entity_epochs_fn); - - // D8 — both endpoints of a dropped cycle edge get - // `LayoutAnchorBroken`. `dropped_targets`: the target Entity at the - // other end of each dropped edge (read from the pre-drop edges - // map). - let mut dropped_targets: std::collections::HashSet = std::collections::HashSet::new(); - for d in &dropped { - new_warns.push((*d, AnchorErrorKind::InCycle)); - if let Some(Some(target)) = edges.get(d).copied() { - dropped_targets.insert(target); - } - } + // 2 + 3. Build edge map and topologically order the anchored + // entities (Kahn sort with cycle-edge dropping). + let (edges, order, dropped, dropped_targets, mut new_warns) = + build_anchor_edge_map(&anchored_query, ®, &display_query); // 4. DuplicateName detection (D11). Scan registry buckets; // `bucket.len() > 1` means duplicate; the last entry is the @@ -1821,11 +1780,102 @@ pub(super) fn anchor_resolution( } } - // 6. Idempotent `LayoutAnchorBroken` marker management. Iterate - // over every entity that could currently have or need the marker — - // anchored entities (anchored_query) AND dropped_targets (which - // may be plain Nodes without Anchor). Use `broken_query` to read - // the current marker state for the non-anchored set. + // 6. Idempotent `LayoutAnchorBroken` marker management. + apply_anchor_broken_markers( + &mut commands, + &broken_set, + &dropped_targets, + &anchored_query, + &broken_query, + ); + + // 7. Emit warns (one per unique `(entity, kind)` per frame). + emit_anchor_warns(new_warns, &mut warned); +} + +/// Steps 2 + 3 of `anchor_resolution` — build the anchor edge map and +/// produce the topological order via `kahn_anchor_sort`. +/// +/// The Kahn helper does its own pre-pass for external target nodes +/// (D10), so plain-Node targets are not inserted here. `dropped` is a +/// `HashSet` (mirroring `kahn_anchor_sort`'s return) — the driver +/// consumes it with set semantics. `TargetMissing` (edge build) and +/// `InCycle` (per dropped endpoint) warns are accumulated into the +/// returned `new_warns`; the driver appends the remaining kinds. +#[allow(clippy::type_complexity)] +fn build_anchor_edge_map( + anchored_query: &Query<(Entity, &Anchor, Option<&LayoutAnchorBroken>), With>, + reg: &AnchorNameRegistry, + display_query: &Query<&Display>, +) -> ( + std::collections::HashMap>, + Vec, + std::collections::HashSet, + std::collections::HashSet, + Vec<(Entity, AnchorErrorKind)>, +) { + let mut edges: std::collections::HashMap> = + std::collections::HashMap::new(); + let mut new_warns: Vec<(Entity, AnchorErrorKind)> = Vec::new(); + + // Helper: target resolution honoring Display::None (D9). Returns + // Some(entity) only when the target is name-resolvable AND not + // Display::None. + let resolve_target = |r: &AnchorRef| -> Option { + let candidate = match r { + AnchorRef::Entity(t) => Some(*t), + AnchorRef::Name(n) => reg.find_entity_by_name(n), + }?; + if let Ok(Display::None) = display_query.get(candidate) { + return None; + } + Some(candidate) + }; + + for (e, anchor, _) in anchored_query.iter() { + let target = anchor.position_anchor.as_ref().and_then(&resolve_target); + edges.insert(e, target); + if anchor.position_anchor.is_some() && target.is_none() { + new_warns.push((e, AnchorErrorKind::TargetMissing)); + } + } + + // 3. Kahn sort. The helper handles external-target pre-pass and + // cycle-edge dropping. + let entity_epochs_fn = |e: Entity| reg.entity_epoch(e); + let (order, dropped) = kahn_anchor_sort(&edges, &entity_epochs_fn); + + // D8 — both endpoints of a dropped cycle edge get + // `LayoutAnchorBroken`. `dropped_targets`: the target Entity at the + // other end of each dropped edge (read from the pre-drop edges + // map). + let mut dropped_targets: std::collections::HashSet = std::collections::HashSet::new(); + for d in &dropped { + new_warns.push((*d, AnchorErrorKind::InCycle)); + if let Some(Some(target)) = edges.get(d).copied() { + dropped_targets.insert(target); + } + } + + (edges, order, dropped, dropped_targets, new_warns) +} + +/// Step 6 of `anchor_resolution` — idempotent `LayoutAnchorBroken` +/// marker management. Iterates over every entity that could currently +/// have or need the marker: anchored entities (`anchored_query`) AND +/// `dropped_targets` (which may be plain Nodes without `Anchor`), then +/// cleans up the marker on stale non-anchored entities. The loop order +/// and the `anchored_query.get(t).is_err()` cleanup guard are +/// load-bearing and must stay verbatim. +fn apply_anchor_broken_markers( + commands: &mut Commands, + broken_set: &std::collections::HashSet, + dropped_targets: &std::collections::HashSet, + anchored_query: &Query<(Entity, &Anchor, Option<&LayoutAnchorBroken>), With>, + broken_query: &Query<(Entity, Option<&LayoutAnchorBroken>)>, +) { + // Use `broken_query` to read the current marker state for the + // non-anchored set. for (e, _, existing_broken) in anchored_query.iter() { let is_broken = broken_set.contains(&e); if is_broken && existing_broken.is_none() { @@ -1836,7 +1886,7 @@ pub(super) fn anchor_resolution( } // Also handle plain-Node targets in `dropped_targets` (they may not // be in anchored_query but still need the marker per D8). - for &t in &dropped_targets { + for &t in dropped_targets { if let Ok((_, existing_broken)) = broken_query.get(t) && existing_broken.is_none() { @@ -1853,8 +1903,17 @@ pub(super) fn anchor_resolution( commands.entity(t).remove::(); } } +} - // 7. Emit warns (one per unique `(entity, kind)` per frame). +/// Step 7 of `anchor_resolution` — emit one `warn!` per unique +/// `(entity, kind)` pair this frame. `warned.set.insert` is the +/// per-frame dedupe gate; the match must stay exhaustive over all +/// `AnchorErrorKind` arms (including `AnchorSizeUsed`, which is +/// currently unreachable from this path but kept for exhaustiveness). +fn emit_anchor_warns( + new_warns: Vec<(Entity, AnchorErrorKind)>, + warned: &mut LayoutAnchorWarnedThisFrame, +) { for (entity, kind) in new_warns { if warned.set.insert((entity, kind)) { match kind { diff --git a/docs/plans/follow-ups.md b/docs/plans/follow-ups.md index 13bb82a..47061e2 100644 --- a/docs/plans/follow-ups.md +++ b/docs/plans/follow-ups.md @@ -189,17 +189,44 @@ its `Entity` handle, and uses `AnchorRef::Entity(e)` as the anchor reference (no name registry involved). Assert the anchored entity's position tracks the target. -## Anchor positioning — extract `anchor_resolution` into sub-helpers +## Anchor positioning — extract `anchor_resolution` into sub-helpers — LANDED **Originated:** Phase 6 (final whole-branch review). -**Symptom:** `anchor_resolution` spans ~252 lines (`systems.rs` lines -497-749) following 7 numbered algorithm steps. Pure refactor candidate -when Phase 7 extends the sub-pass set. - -**Implementation sketch:** split into `build_anchor_edge_map`, -`apply_anchor_broken_markers`, `emit_anchor_warns`. No behavior change; -makes future extension cleaner. +**Symptom:** `anchor_resolution` (`crates/buiy_core/src/layout/systems.rs`, +~265 lines) followed 7 numbered algorithm steps inline. Pure refactor +candidate to keep later anchor edits rebasing onto clean helpers. + +**Status:** **Landed.** Behavior-preserving extraction — zero observable +change; the existing anchor + pipeline-order suites stayed green throughout. +`anchor_resolution` is now a thin driver delegating to three private free +helpers, co-located directly below it (matching the existing +`kahn_anchor_sort` / `try_anchored_position` / `try_conditions_pass` +neighbors): + +- `build_anchor_edge_map` (steps 2 + 3) — owns the `resolve_target` and + `entity_epochs_fn` closures (capturing `&AnchorNameRegistry` / + `&Query<&Display>` by shared ref), the per-entity edge insert + + `TargetMissing` warn, the `kahn_anchor_sort` call, and the + cycle-endpoint `InCycle` warn / `dropped_targets` loop. Returns + `(edges, order, dropped, dropped_targets, new_warns)`; `dropped` is a + `HashSet` mirroring `kahn_anchor_sort`'s return so the driver's + set-membership consumption (`dropped.contains`, the seeding loops) is + unchanged. +- `apply_anchor_broken_markers` (step 6) — idempotent `LayoutAnchorBroken` + insert/remove across the anchored set, the `dropped_targets` plain-Node + set, and the non-anchored cleanup set (loop order + the + `anchored_query.get(t).is_err()` cleanup guard kept verbatim). +- `emit_anchor_warns` (step 7) — the `warned.set.insert` dedupe gate + the + exhaustive per-`AnchorErrorKind` `warn!` match. + +The step-5 topological walk was **intentionally left inline** in the driver +(out of the named sketch): it is the most entangled body — it closes over +`tree` + `overrides` + `reg` + `display_query` + `viewport` + +`anchored_query` and mutates `overrides` / `broken_set` / `new_warns` — so +extracting it was out of this slice's scope. + +**Spec touchpoint:** none — pure internal refactor, no target-state change. ## Layout — `Position::Fixed` implementation — LANDED From a55e1fba437775099291a917cc9bbb9f09dac72d Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 08:02:43 -0700 Subject: [PATCH 3/9] =?UTF-8?q?feat(layout):=20anchor-size()=20term=20?= =?UTF-8?q?=E2=80=94=20Length::AnchorSize(AxisDimension)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve CSS anchor-size() inside PositionTry insets against the per-try anchor box. try_anchored_position already has the override-aware anchor_size: Vec2 in scope, so AnchorSize(Width)->x / (Height)->y short- circuits before delegating to length_inset_to_px — honored independent of which edge the inset sits on (cross-axis covered by test). The closed Length enum forces an arm at every match site; non-anchor contexts (track sizing, taffy translation, transforms) degrade AnchorSize to 0/auto exactly like Cq* outside its query. Sticky has no anchor box: resolve_sticky_inset resolves AnchorSize to 0.0 + a StickyAnchorSizeUnsupported warn-once. Deletes the now-unreachable AnchorErrorKind::AnchorSizeUsed forward-compat hook (the warn is semantically wrong now that the term actually resolves; no Reflect consumer enumerated it). AxisDimension registered for reflection. Reverses the spec 3.4 'tier-C deferred to v1.x' decision (user-authorized via the follow-ups drain): updates display-and-positioning.md 3.4 + README 5. follow-ups.md flipped to LANDED. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- crates/buiy_core/src/layout/mod.rs | 3 +- crates/buiy_core/src/layout/systems.rs | 88 +++++++++++++++---- crates/buiy_core/src/layout/translate.rs | 27 ++++-- crates/buiy_core/src/layout/types.rs | 31 ++++++- .../tests/layout_anchor_positioning.rs | 62 ++++++++++++- docs/plans/follow-ups.md | 20 ++++- .../2026-05-08-buiy-layout-design/README.md | 1 + .../display-and-positioning.md | 2 +- 8 files changed, 204 insertions(+), 30 deletions(-) diff --git a/crates/buiy_core/src/layout/mod.rs b/crates/buiy_core/src/layout/mod.rs index 38bb7db..5596e66 100644 --- a/crates/buiy_core/src/layout/mod.rs +++ b/crates/buiy_core/src/layout/mod.rs @@ -25,7 +25,7 @@ pub use systems::{ }; pub use tree::LayoutTree; pub use types::{ - AlignContent, AlignItems, AnchorErrorKind, AnchorName, AnchorRef, AspectRatio, + AlignContent, AlignItems, AnchorErrorKind, AnchorName, AnchorRef, AspectRatio, AxisDimension, BackfaceVisibility, BoxSizing, BreakAfter, BreakBefore, BreakInside, ColumnCount, ColumnFill, ColumnRule, ColumnRuleStyle, ColumnSpan, ContainFlags, ContainerType, ContentVisibility, Direction, Edges, FlexAxis, FlexGap, FlexWrap, GridAreas, GridAutoFlow, GridLine, Inset, @@ -139,6 +139,7 @@ impl Plugin for LayoutPlugin { .register_type::() .register_type::() .register_type::() + .register_type::() .register_type::() .register_type::() .register_type::() diff --git a/crates/buiy_core/src/layout/systems.rs b/crates/buiy_core/src/layout/systems.rs index 4ca27ff..e453940 100644 --- a/crates/buiy_core/src/layout/systems.rs +++ b/crates/buiy_core/src/layout/systems.rs @@ -24,10 +24,10 @@ use super::components::{ use super::translate::{ContainerSnapshot, StyleView, style_to_taffy}; use super::tree::LayoutTree; use super::types::{ - AnchorErrorKind, AnchorName, AnchorRef, BreakAfter, BreakBefore, ColumnCount, ColumnFill, - ContainFlags, ContainerType, ContentVisibility, GridAreas, Inset, Isolation, LayoutWarnOnceKey, - Length, PositionKind, QueryCondition, Sizing, TopLayer, TransformMatrix, TryCondition, - WritingModeKind, ZIndex, + AnchorErrorKind, AnchorName, AnchorRef, AxisDimension, BreakAfter, BreakBefore, ColumnCount, + ColumnFill, ContainFlags, ContainerType, ContentVisibility, GridAreas, Inset, Isolation, + LayoutWarnOnceKey, Length, PositionKind, QueryCondition, Sizing, TopLayer, TransformMatrix, + TryCondition, WritingModeKind, ZIndex, }; use crate::components::{Node, ResolvedLayout, ResolvedTransform}; use crate::render::components::{Filter, MixBlendMode, Opacity}; @@ -475,6 +475,21 @@ fn resolve_sticky_inset( } 0.0 } + // `anchor-size()` reads an anchor box; sticky has none. Resolve + // to 0.0, warn once per (entity, session) — same channel + dedup + // shape as the Fr / Cq* sticky-deferred arms above. + Length::AnchorSize(_) => { + if warned + .set + .insert(LayoutWarnOnceKey::StickyAnchorSizeUnsupported(entity)) + { + warn!( + "Sticky entity {:?} uses anchor-size() inset; sticky has no anchor box, so it resolves to 0.0.", + entity, + ); + } + 0.0 + } }) } @@ -1467,6 +1482,10 @@ fn length_inset_to_px(l: Length, axis: f32, _viewport: Vec2) -> f32 { | Length::Cqb(_) | Length::Cqmin(_) | Length::Cqmax(_) => 0.0, + // `anchor-size()` is intercepted by `try_anchored_position`'s + // `to_px` closure before delegating here, so this arm is the + // defensive no-anchor-box fallback (resolve to 0.0). + Length::AnchorSize(_) => 0.0, } } @@ -1502,6 +1521,11 @@ fn try_anchored_position( match s { Sizing::Auto => 0.0, Sizing::None => 0.0, + // `anchor-size()` resolves to the per-try anchor box's + // size on the *named* axis (independent of which edge it sits + // on); every other `Length` delegates to `length_inset_to_px`. + Sizing::Length(Length::AnchorSize(AxisDimension::Width)) => anchor_size.x, + Sizing::Length(Length::AnchorSize(AxisDimension::Height)) => anchor_size.y, Sizing::Length(l) => length_inset_to_px(*l, axis, viewport), // B4: `FitContent` is a tuple variant `FitContent(Length)`; // the wildcard `(_)` discards the inner Length (no @@ -1907,9 +1931,8 @@ fn apply_anchor_broken_markers( /// Step 7 of `anchor_resolution` — emit one `warn!` per unique /// `(entity, kind)` pair this frame. `warned.set.insert` is the -/// per-frame dedupe gate; the match must stay exhaustive over all -/// `AnchorErrorKind` arms (including `AnchorSizeUsed`, which is -/// currently unreachable from this path but kept for exhaustiveness). +/// per-frame dedupe gate; the match stays exhaustive over every +/// `AnchorErrorKind` arm. fn emit_anchor_warns( new_warns: Vec<(Entity, AnchorErrorKind)>, warned: &mut LayoutAnchorWarnedThisFrame, @@ -1935,12 +1958,6 @@ fn emit_anchor_warns( "buiy: duplicate anchor_name — late inserter wins, shadowed entries lose name lookup" ); } - AnchorErrorKind::AnchorSizeUsed => { - warn!( - ?entity, - "buiy: anchor-size() in PositionTry::inset is deferred to v1.x; resolving to 0" - ); - } } } } @@ -3217,14 +3234,17 @@ fn length_to_px(len: Length, axis_basis: f32) -> f32 { // condition value would be a degenerate case (a rule about // a container, sized in units of that same container). Warn // is unnecessary because authors compose with Length::Px. - // Fr is a grid-only unit; degrades to 0 here. + // Fr is a grid-only unit; degrades to 0 here. anchor-size() is + // only meaningful inside anchor inset resolution; in a container- + // query condition value it has no anchor box, so it degrades to 0. Length::Fr(_) | Length::Cqw(_) | Length::Cqh(_) | Length::Cqi(_) | Length::Cqb(_) | Length::Cqmin(_) - | Length::Cqmax(_) => 0.0, + | Length::Cqmax(_) + | Length::AnchorSize(_) => 0.0, } } @@ -5472,6 +5492,44 @@ mod tests { "never skip without last-frame geometry (D3)" ); } + + #[test] + fn try_anchored_position_resolves_anchor_size_height() { + // anchor-size(height): `inset.top` resolves to the anchor's own + // height (40), so the anchored entity is placed at + // anchor.y(0) + anchor.height(40) + resolved_top(40) = 80. + let inset = Inset { + top: Sizing::Length(Length::AnchorSize(AxisDimension::Height)), + ..Default::default() + }; + let pos = try_anchored_position( + Vec2::ZERO, + Vec2::new(80.0, 40.0), + Vec2::new(10.0, 10.0), + &inset, + Vec2::new(800.0, 600.0), + ); + assert_eq!(pos.y, 80.0); + } + + #[test] + fn try_anchored_position_resolves_anchor_size_width_on_left_edge() { + // anchor-size(width) on the `left` edge resolves to the anchor's + // width (80) regardless of axis, so x = + // anchor.x(0) + anchor.width(80) + resolved_left(80) = 160. + let inset = Inset { + left: Sizing::Length(Length::AnchorSize(AxisDimension::Width)), + ..Default::default() + }; + let pos = try_anchored_position( + Vec2::ZERO, + Vec2::new(80.0, 40.0), + Vec2::new(10.0, 10.0), + &inset, + Vec2::new(800.0, 600.0), + ); + assert_eq!(pos.x, 160.0); + } } #[cfg(test)] diff --git a/crates/buiy_core/src/layout/translate.rs b/crates/buiy_core/src/layout/translate.rs index 518d322..38a8513 100644 --- a/crates/buiy_core/src/layout/translate.rs +++ b/crates/buiy_core/src/layout/translate.rs @@ -656,7 +656,10 @@ fn length_to_dim(l: Length) -> taffy::Dimension { | Length::Cqi(_) | Length::Cqb(_) | Length::Cqmin(_) - | Length::Cqmax(_) => taffy::Dimension::length(0.0), + | Length::Cqmax(_) + // anchor-size() is meaningful only inside anchor inset resolution; + // in Taffy dimension translation there is no anchor box → 0. + | Length::AnchorSize(_) => taffy::Dimension::length(0.0), } } @@ -678,7 +681,10 @@ fn length_to_lp(l: Length) -> taffy::LengthPercentage { | Length::Cqi(_) | Length::Cqb(_) | Length::Cqmin(_) - | Length::Cqmax(_) => taffy::LengthPercentage::length(0.0), + | Length::Cqmax(_) + // anchor-size() is meaningful only inside anchor inset resolution; + // in Taffy dimension translation there is no anchor box → 0. + | Length::AnchorSize(_) => taffy::LengthPercentage::length(0.0), } } @@ -698,7 +704,10 @@ fn length_to_lpa(l: Length) -> taffy::LengthPercentageAuto { | Length::Cqi(_) | Length::Cqb(_) | Length::Cqmin(_) - | Length::Cqmax(_) => taffy::LengthPercentageAuto::length(0.0), + | Length::Cqmax(_) + // anchor-size() is meaningful only inside anchor inset resolution; + // in Taffy dimension translation there is no anchor box → 0. + | Length::AnchorSize(_) => taffy::LengthPercentageAuto::length(0.0), } } @@ -822,7 +831,9 @@ fn track_to_sizing(t: &TrackSize) -> TrackSizingFunction { | Length::Cqi(_) | Length::Cqb(_) | Length::Cqmin(_) - | Length::Cqmax(_), + | Length::Cqmax(_) + // anchor-size() has no anchor box in a track template → 0. + | Length::AnchorSize(_), ) => length(0.0), // `MinMax` carries a Vec; spec/Phase3 invariant is // exactly 2 elements [min, max]. Other arities warn-once and @@ -862,7 +873,9 @@ fn track_to_min(t: &TrackSize) -> MinTrackSizingFunction { | Length::Cqi(_) | Length::Cqb(_) | Length::Cqmin(_) - | Length::Cqmax(_), + | Length::Cqmax(_) + // anchor-size() has no anchor box in a track template → 0. + | Length::AnchorSize(_), ) => length(0.0), // CSS forbids these in MinMax's min slot: // - Fr (fr-in-min is grammar-invalid) @@ -899,7 +912,9 @@ fn track_to_max(t: &TrackSize) -> MaxTrackSizingFunction { | Length::Cqi(_) | Length::Cqb(_) | Length::Cqmin(_) - | Length::Cqmax(_), + | Length::Cqmax(_) + // anchor-size() has no anchor box in a track template → 0. + | Length::AnchorSize(_), ) => length(0.0), TrackSize::MinMax(_) | TrackSize::Repeat(_, _) | TrackSize::Subgrid => { warn_once_invalid_track_nesting(); diff --git a/crates/buiy_core/src/layout/types.rs b/crates/buiy_core/src/layout/types.rs index 868169b..956b3ab 100644 --- a/crates/buiy_core/src/layout/types.rs +++ b/crates/buiy_core/src/layout/types.rs @@ -50,6 +50,25 @@ pub enum Length { Cqmin(f32), /// `cqmax` — percentage of `max(cqi, cqb)`. Cqmax(f32), + /// CSS `anchor-size()` — resolves to the anchor target's + /// *resolved* size on the named axis. Meaningful only inside a + /// `PositionTry::inset` term (where the per-try anchor box is known + /// at resolution time); in every non-anchor context it resolves to + /// `0`/`auto`, exactly like `Cq*` outside its query. Carries only the + /// axis selector — the anchor box itself is supplied by the + /// resolution site, so no `AnchorRef` payload is needed. + /// + /// Spec: docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md § 3.4. + AnchorSize(AxisDimension), +} + +/// Which axis of the anchor box an `anchor-size()` term reads. +/// +/// Spec: docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md § 3.4. +#[derive(Reflect, Clone, Copy, Debug, PartialEq, Eq)] +pub enum AxisDimension { + Width, + Height, } impl Length { @@ -958,9 +977,6 @@ pub enum AnchorErrorKind { /// per (name, frame)" only in that the per-entity gate also avoids /// repeat warns if the same entity re-inserts within the same frame. DuplicateName, - /// `anchor-size()` used in a `PositionTry::inset` term. Tier-C - /// deferred to v1.x; the term resolves to zero with a warn. - AnchorSizeUsed, } /// Phase 7 — session-scoped warn-once dedup key. Variants cover the @@ -1044,6 +1060,15 @@ pub enum LayoutWarnOnceKey { /// docs/plans/2026-05-22-buiy-layout-sticky-table-multicol.md. StickyCqDeferred(Entity), + /// Sticky entity uses a `Length::AnchorSize` inset. `anchor-size()` + /// reads the anchor target's box, but a sticky element has no anchor + /// box — the term is meaningless here, so the inset resolves to 0.0. + /// One warn per (entity, session), mirroring `StickyFrUnsupported` / + /// `StickyCqDeferred`. + /// + /// Spec: docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md § 3.4. + StickyAnchorSizeUnsupported(Entity), + /// `Containment.contain` includes `SIZE` / `INLINE_SIZE` and the /// corresponding axis sizing is `Sizing::Auto`. Per spec § 5.1 the /// auto size is treated as `0px`. One warn per (entity, session). diff --git a/crates/buiy_core/tests/layout_anchor_positioning.rs b/crates/buiy_core/tests/layout_anchor_positioning.rs index 2e9a53a..e75f337 100644 --- a/crates/buiy_core/tests/layout_anchor_positioning.rs +++ b/crates/buiy_core/tests/layout_anchor_positioning.rs @@ -5,9 +5,9 @@ use bevy::prelude::*; use buiy_core::components::{Node, ResolvedLayout}; use buiy_core::layout::{ - Anchor, AnchorErrorKind, AnchorName, AnchorNameRegistry, AnchorRef, Display, Inset, - LayoutAnchorBroken, LayoutAnchorWarnedThisFrame, LayoutPlugin, Length, PositionTry, Style, - SyncStylesIterCount, TryCondition, + Anchor, AnchorErrorKind, AnchorName, AnchorNameRegistry, AnchorRef, AxisDimension, Display, + Inset, LayoutAnchorBroken, LayoutAnchorWarnedThisFrame, LayoutPlugin, Length, PositionTry, + Sizing, Style, SyncStylesIterCount, TryCondition, }; fn app() -> App { @@ -117,6 +117,62 @@ fn write_resolved_layout_prefers_anchor_override_over_taffy_position() { assert!(!overrides.by_entity.contains_key(&anchor_e)); // anchor target, not anchored } +#[test] +fn anchor_size_inset_resolves_to_anchor_height() { + use buiy_core::layout::PostTaffyPositionOverrides; + let mut app = app(); + + // Anchor: 80x40 at the default Taffy origin (0,0). + let _anchor = app + .world_mut() + .spawn(( + Node, + Style::default().width_px(80.0).height_px(40.0), + Anchor { + anchor_name: Some(AnchorName::Named("a".into())), + ..default() + }, + )) + .id(); + + // Anchored: 10x10, placed BELOW the anchor by `anchor-size(height)`. + // The `top` inset resolves to the anchor's own height (40), so the + // anchored entity lands at anchor.y(0) + anchor.height(40) + + // resolved_top(40) = 80 — explicitly NOT 0.0 (the old deferred path). + let anchored = app + .world_mut() + .spawn(( + Node, + Style::default().width_px(10.0).height_px(10.0), + Anchor { + position_anchor: Some(AnchorRef::Name("a".into())), + position_try: vec![PositionTry { + inset: Inset { + top: Sizing::Length(Length::AnchorSize(AxisDimension::Height)), + ..default() + }, + conditions: vec![], // no conditions = always passes + }], + ..default() + }, + )) + .id(); + + app.update(); + app.update(); + + let anchored_rl = app.world().get::(anchored).unwrap(); + assert_eq!( + anchored_rl.position.y, 80.0, + "anchor-size(height) must resolve to the anchor's real height (40), not 0.0" + ); + + // Confirm the override channel carried the anchor-size-derived position. + let overrides = app.world().resource::(); + assert!(overrides.by_entity.contains_key(&anchored)); + assert_eq!(overrides.by_entity.get(&anchored).unwrap().y, 80.0); +} + #[test] fn sync_styles_reruns_when_anchor_changes() { use buiy_core::layout::SyncStylesIterCount; diff --git a/docs/plans/follow-ups.md b/docs/plans/follow-ups.md index 47061e2..46ec01b 100644 --- a/docs/plans/follow-ups.md +++ b/docs/plans/follow-ups.md @@ -91,11 +91,29 @@ frame N+1 applies B's" — currently the assertion would never fire because B never updates. -## Anchor positioning — `anchor-size()` term +## Anchor positioning — `anchor-size()` term — LANDED **Originated:** Phase 6 (anchor positioning), spec § 3.4 line 231 ("tier-C feature deferred to v1.x"). +**Status:** **Landed.** `anchor-size()` ships as +`Length::AnchorSize(AxisDimension)` (no `AnchorRef` payload — the +over-specified sketch below is superseded; the per-try anchor box is +already known at the resolution site `try_anchored_position`, so only the +axis selector is needed). The `to_px` closure resolves +`AnchorSize(Width) → anchor_size.x` / `AnchorSize(Height) → anchor_size.y` +against the per-try box; every non-anchor `Length` match site degrades it +to `0`/`auto` alongside the existing `Cq*` defensive arms. Sticky has no +anchor box, so `resolve_sticky_inset` resolves it to `0.0` and warns once +via the new session-scoped `LayoutWarnOnceKey::StickyAnchorSizeUnsupported`. +The previously-unreachable `AnchorErrorKind::AnchorSizeUsed` variant + +warn arm were **deleted** (not repurposed): anchor-size now resolves to a +real value, so an error/warn kind is semantically wrong; the variant had +no Reflect/serialization consumer worth preserving (it never fired). +Spec § 3.4 + README § 5 deferral reversed. Covered by +`try_anchored_position_resolves_anchor_size_{height,width...}` (unit) and +`anchor_size_inset_resolves_to_anchor_height` (integration). + **Symptom:** authors using `anchor-size()` in a `PositionTry::inset` term get a `0.0` resolution and a per-frame warn via `AnchorErrorKind::AnchorSizeUsed`. The variant is shipped in Phase 6 as diff --git a/docs/specs/2026-05-08-buiy-layout-design/README.md b/docs/specs/2026-05-08-buiy-layout-design/README.md index 5f47ca0..db287e1 100644 --- a/docs/specs/2026-05-08-buiy-layout-design/README.md +++ b/docs/specs/2026-05-08-buiy-layout-design/README.md @@ -93,6 +93,7 @@ This spec is target-state. The Phase 0 → target migration (the decomposed-comp - **Stacking-context performance.** The set of triggers is large (positioned + non-`auto` z-index, opacity < 1, transform, filter, will-change, isolation, mix-blend-mode). Whether to detect lazily (during paint) or eagerly (during layout) is open. [stacking-and-top-layer.md](stacking-and-top-layer.md) discusses. - **`writing-mode: sideways-*` Taffy support.** Taffy 0.10 has logical properties but doesn't fully model sideways modes. Whether to ship a Buiy-side rotation pass or wait on Taffy is open. [container-queries-and-writing-modes.md](container-queries-and-writing-modes.md) details. - **Top-layer ordering across windows.** When a Buiy app has multiple windows each with its own modal, modal stacking is per-window. Cross-window top-layer (a modal that visually escapes its window) is out of scope; tracked in `buiy-window-and-surface-design`. +- **`anchor-size()` in `PositionTry::inset` — RESOLVED.** Previously deferred to v1.x with no API surface; now ships as `Length::AnchorSize(AxisDimension)`, resolved against the per-try anchor box at anchor-resolution time (no Taffy re-layout). See [display-and-positioning.md § 3.4](display-and-positioning.md#34-performance-and-ordering). ## References diff --git a/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md b/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md index f4effca..3525ece 100644 --- a/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md +++ b/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md @@ -257,7 +257,7 @@ commands.spawn(( - An anchor target must resolve before its dependent. Sub-pass 6d builds a Kahn topological sort over the (anchored → anchor) DAG: `O(V + E)` where V = anchored entities, E = anchor edges (= V, since each anchored entity has exactly one anchor target). - **Cycle handling.** Edges that would close a cycle are dropped — the dropped edge is `(child anchored, anchor)` for the *most-recently-inserted* anchored entity in the cycle (tracked by the `AnchorNameRegistry` insertion epoch and an analogous epoch on `AnchorRef::Entity`). Both endpoints get `LayoutAnchorBroken` markers; one `warn!` fires per cycle per frame naming the dropped edge. Result: every cycle resolves deterministically; tests can assert exact membership. - Cost: `O(anchored entities × tries + V + E)`. Usually small — most anchored elements have 1-3 fallbacks. -- Anchor resolution does **not** trigger Taffy re-layout. The anchor's *size* is fixed by the time anchor resolution runs; only its position changes. (Anchor *size* affecting layout — `anchor-size()` in CSS — is a tier-C feature **fully deferred to v1.x with no API surface in v1**: there is no `Length`/`Sizing`/`Inset` variant that can express `anchor-size()`, so authors cannot exercise it. The `AnchorErrorKind::AnchorSizeUsed` warn arm exists ([systems.rs](../../../crates/buiy_core/src/layout/systems.rs)) but is currently **unreachable** — nothing pushes that kind into the warn set, because no inset term can request an anchor size. When the API lands in v1.x, the warn arm will fire once per (entity, frame) and the size term resolves to zero. See open questions in [README § 5](README.md#5-open-questions).) +- Anchor resolution does **not** trigger Taffy re-layout. The anchor's *size* is fixed by the time anchor resolution runs; only its position changes. `anchor-size()` exploits exactly this: because the anchor's size is already resolved when anchor resolution runs, an `anchor-size()` term reads that size directly — no Taffy re-layout is needed. It ships as `Length::AnchorSize(AxisDimension)` (the `AxisDimension::{Width, Height}` selector names which axis of the anchor box to read), usable inside `PositionTry::inset` via `Sizing::Length(Length::AnchorSize(_))`. At resolution time `try_anchored_position` ([systems.rs](../../../crates/buiy_core/src/layout/systems.rs)) resolves it against the per-try anchor box: `Width → anchor.size.x`, `Height → anchor.size.y`, independent of which edge the term sits on. In every non-anchor context (track sizing, Taffy dimension translation, container-query condition values, sticky insets) it degrades to `0`/`auto` exactly like `Cq*` outside its query; a sticky inset additionally warns once per (entity, session) via `LayoutWarnOnceKey::StickyAnchorSizeUnsupported` because sticky has no anchor box. The former `AnchorErrorKind::AnchorSizeUsed` warn arm (a v1.x forward-compat hook for the deferred feature) has been **retired** — anchor-size now lands and resolves to a real value, so an error/warn kind would be semantically wrong. ### 3.5 Open question: `position-try` chain depth From 657bc2028a04a0cd8f6444fb8a2c9ba815be7c28 Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 08:11:41 -0700 Subject: [PATCH 4/9] test(layout): AnchorRef::Entity direct-reference end-to-end coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All 11 existing anchor tests use AnchorRef::Name; the direct AnchorRef::Entity(e) branch had zero end-to-end coverage. Adds two headless tests (no production change — the Entity arm + Display::None guard already exist in build_anchor_edge_map's resolve_target closure): - anchor_entity_ref_positions_relative_to_target: plain Node target (no anchor_name/registry) addressed by Entity, anchored 10px below → override + ResolvedLayout y == 60, no LayoutAnchorBroken. - anchor_entity_ref_display_none_target_is_missing: Entity ref at a Display::None target → zero override + LayoutAnchorBroken + (anchored, TargetMissing) warn, exercising the guard via the Entity arm rather than the Name arm. Display::None (not despawn) chosen for the negative: a despawned Entity makes resolve_target return Some(e) (guard does not fire) and hits a different miss path — Display::None is the clean deterministic guard. follow-ups.md flipped to LANDED. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- .../tests/layout_anchor_positioning.rs | 129 ++++++++++++++++++ docs/plans/follow-ups.md | 16 ++- 2 files changed, 144 insertions(+), 1 deletion(-) diff --git a/crates/buiy_core/tests/layout_anchor_positioning.rs b/crates/buiy_core/tests/layout_anchor_positioning.rs index e75f337..1310f36 100644 --- a/crates/buiy_core/tests/layout_anchor_positioning.rs +++ b/crates/buiy_core/tests/layout_anchor_positioning.rs @@ -549,3 +549,132 @@ fn anchor_target_with_display_none_is_treated_as_missing() { // D9 explicit query in anchor_resolution). assert!(app.world().get::(anchored).is_some()); } + +// --------------------------------------------------------------------- +// `AnchorRef::Entity` direct-reference coverage. Every test above uses +// `AnchorRef::Name`; these two exercise the `AnchorRef::Entity(t)` arm +// of `resolve_target` (systems.rs) end-to-end — both the positive +// (target present in the Taffy tree) and negative (Display::None guard) +// paths — with NO name registry involvement on either target. +// --------------------------------------------------------------------- + +#[test] +fn anchor_entity_ref_positions_relative_to_target() { + use buiy_core::layout::PostTaffyPositionOverrides; + let mut app = app(); + + // Target: a plain 100x50 Node at the default Taffy origin (0,0). + // NO Anchor, NO anchor_name, NO registry entry — addressed purely + // by its Entity handle. + let target = app + .world_mut() + .spawn((Node, Style::default().width_px(100.0).height_px(50.0))) + .id(); + + // Anchored: 80x20, placed 10px below the target via a direct + // `AnchorRef::Entity`. `conditions: vec![]` always passes (no + // PrimaryWindow is spawned, so this avoids any viewport ambiguity). + let anchored = app + .world_mut() + .spawn(( + Node, + Style::default().width_px(80.0).height_px(20.0), + Anchor { + position_anchor: Some(AnchorRef::Entity(target)), + position_try: vec![PositionTry { + inset: Inset::below(Length::Px(10.0)), + conditions: vec![], + }], + ..default() + }, + )) + .id(); + + // Frame 1 sizes the boxes via Taffy; frame 2 runs the anchor pass. + app.update(); + app.update(); + + // Override carries the resolved position: target.y(0) + + // target.h(50) + inset(10) = 60. + let overrides = app.world().resource::(); + assert!(overrides.by_entity.contains_key(&anchored)); + assert_eq!(overrides.by_entity.get(&anchored).unwrap().y, 60.0); + // The target itself is not anchored — no override entry. + assert!(!overrides.by_entity.contains_key(&target)); + + // ResolvedLayout reflects the override. + assert_eq!( + app.world() + .get::(anchored) + .unwrap() + .position + .y, + 60.0 + ); + + // Direct ref resolved cleanly — not broken. + assert!(app.world().get::(anchored).is_none()); +} + +#[test] +fn anchor_entity_ref_display_none_target_is_missing() { + use buiy_core::layout::PostTaffyPositionOverrides; + let mut app = app(); + + // Target: a Display::None Node, NO anchor_name — addressed purely + // by Entity. The Display::None guard in `resolve_target` (D9) must + // fire via the Entity arm exactly as it does via the Name arm. + let hidden = app + .world_mut() + .spawn(( + Node, + Style::default() + .width_px(50.0) + .height_px(50.0) + .display(Display::None), + )) + .id(); + + let anchored = app + .world_mut() + .spawn(( + Node, + Style::default().width_px(20.0).height_px(20.0), + Anchor { + position_anchor: Some(AnchorRef::Entity(hidden)), + position_try: vec![PositionTry { + inset: Inset::below(Length::Px(0.0)), + conditions: vec![], + }], + ..default() + }, + )) + .id(); + + app.update(); + app.update(); + + // Display::None → resolve_target returns None → TargetMissing: + // zero override + broken marker + per-frame warn. + assert!(app.world().get::(anchored).is_some()); + + let overrides = app.world().resource::(); + assert_eq!( + overrides.by_entity.get(&anchored).copied(), + Some(Vec2::ZERO) + ); + assert_eq!( + app.world() + .get::(anchored) + .unwrap() + .position, + Vec2::ZERO + ); + + let warned = app.world().resource::(); + assert!( + warned + .set + .contains(&(anchored, AnchorErrorKind::TargetMissing)) + ); +} diff --git a/docs/plans/follow-ups.md b/docs/plans/follow-ups.md index 46ec01b..b92c69c 100644 --- a/docs/plans/follow-ups.md +++ b/docs/plans/follow-ups.md @@ -192,7 +192,7 @@ after 6d). The corrections are invisible to 6d. Option 1 preserves spec order and is the more flexible long-term shape. -## Anchor positioning — `AnchorRef::Entity` integration test gap +## Anchor positioning — `AnchorRef::Entity` integration test gap — LANDED **Originated:** Phase 6 (final whole-branch review). @@ -207,6 +207,20 @@ its `Entity` handle, and uses `AnchorRef::Entity(e)` as the anchor reference (no name registry involved). Assert the anchored entity's position tracks the target. +**Status:** **Landed.** Two end-to-end tests added to +`crates/buiy_core/tests/layout_anchor_positioning.rs` — +`anchor_entity_ref_positions_relative_to_target` (positive: a direct +`AnchorRef::Entity` to a plain-Node target with no registry entry +resolves to the target box edge + inset; override `y == 60`, no +`LayoutAnchorBroken`) and `anchor_entity_ref_display_none_target_is_missing` +(negative: `AnchorRef::Entity` at a `Display::None` target → the D9 guard +in `resolve_target` fires via the Entity arm → zero override + +`LayoutAnchorBroken` + `TargetMissing` warn). Test-only; no production +change — the `AnchorRef::Entity(t) => Some(*t)` arm and the `Display::None` +guard already existed inside `build_anchor_edge_map`'s `resolve_target` +closure. Fills declared coverage of spec §4 'Test surface'; no +target-state change. + ## Anchor positioning — extract `anchor_resolution` into sub-helpers — LANDED **Originated:** Phase 6 (final whole-branch review). From 465eec9040a005297d5730e23d6ff477b616e398 Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 09:57:12 -0700 Subject: [PATCH 5/9] feat(layout): sticky Length::Cq* inset resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sticky Cqw/Cqh/Cqi/Cqb/Cqmin/Cqmax insets now resolve against the sticky entity's own nearest container-query ancestor instead of degrading to 0.0 + StickyCqDeferred warn. resolve_sticky_inset delegates Cq* to the existing translate::resolve_cq_unit_px (cqw/cqh/cqi/cqb/cqmin/cqmax math + writing-mode inline/block axis swap + no-ancestor viewport fallback). Timing fix (plan-review major): the CQ-ancestor size is read CURRENT-frame from Taffy (tree.by_entity + tree.tree.layout), NOT from last-frame ResolvedLayout — sticky_offset runs in PostTaffyOverrides before WriteResolvedLayout, and every other size it consumes (self/parent/scroll) is current-frame Taffy, so a last-frame container size would be internally inconsistent on the establishing/resize frame. sticky_offset builds a current-frame container_index and resolves the CQ frame once per entity via nearest_container_with_size; this also collapses the fixtures to one update(). StickyCqDeferred warn retired (resolve_cq_unit_px owns the no-ancestor warn — delegating makes sticky consistent with every other Cq* site). The L2 Length::AnchorSize sticky arm is preserved. Tests: layout_sticky 18/18 (4 new incl. Cqi/Cqb VerticalRl axis-swap + separate-CQ-ancestor-vs-scroll-container). Spec §2.3/§3.4 + container-queries §1.4 + README §5 updated; follow-ups.md LANDED. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- crates/buiy_core/src/layout/systems.rs | 137 ++++++++--- crates/buiy_core/src/layout/types.rs | 12 +- crates/buiy_core/tests/layout_sticky.rs | 217 ++++++++++++++++-- docs/plans/follow-ups.md | 38 ++- .../2026-05-08-buiy-layout-design/README.md | 1 + .../container-queries-and-writing-modes.md | 2 + .../display-and-positioning.md | 4 +- 7 files changed, 343 insertions(+), 68 deletions(-) diff --git a/crates/buiy_core/src/layout/systems.rs b/crates/buiy_core/src/layout/systems.rs index e453940..1ef74f4 100644 --- a/crates/buiy_core/src/layout/systems.rs +++ b/crates/buiy_core/src/layout/systems.rs @@ -21,7 +21,7 @@ use super::components::{ LayoutAnchorBroken, MultiColumn, Overflow, Position, Rotate, Scale, Scroll, ScrollOffset, Stacking, Translate, UiTransform, WritingMode, WritingModeResolved, }; -use super::translate::{ContainerSnapshot, StyleView, style_to_taffy}; +use super::translate::{ContainerSnapshot, StyleView, resolve_cq_unit_px, style_to_taffy}; use super::tree::LayoutTree; use super::types::{ AnchorErrorKind, AnchorName, AnchorRef, AxisDimension, BreakAfter, BreakBefore, ColumnCount, @@ -409,8 +409,10 @@ fn world_position( /// reference frame, per D3 / D11. /// /// Returns `Some(px)` for "this edge is sticky-active" or `None` for -/// "this edge is not set." Inputs that are deferred (`Cq*`) or -/// semantically invalid (`Fr` — grid-only) return `Some(0.0)` and +/// "this edge is not set." `Cq*` insets resolve against `cq_frame` (the +/// sticky entity's own nearest CQ ancestor) via the shared +/// `resolve_cq_unit_px`. Semantically-invalid inputs (`Fr` — grid-only, +/// `AnchorSize` — sticky has no anchor box) return `Some(0.0)` and /// record one `warn!` per (entity, session) via `warned`. /// /// v2 — `Length` has only `Px / Percent / Fr / Cq*`. `Vh/Vw/Vmin/Vmax/ @@ -420,9 +422,13 @@ fn world_position( /// deliberate decision per future variant. /// /// Phase 7 — sub-pass 6a (`sticky_offset`). +#[allow(clippy::too_many_arguments)] fn resolve_sticky_inset( s: &Sizing, scroll_container_axis_size: f32, + cq_frame: Option, + viewport: Vec2, + wmr: &WritingModeResolved, entity: Entity, warned: &mut LayoutWarnedOnceSession, ) -> Option { @@ -453,31 +459,22 @@ fn resolve_sticky_inset( } 0.0 } - // All Cq* variants — full resolution is deferred to a Phase - // 7.x follow-up (would port Phase 6's `length_inset_to_px`, - // which takes an anchor-box second argument; sticky's - // reference frame is the sticky entity's own cq-ancestor, a - // different shape). v1: warn once per entity, resolve to 0.0. + // All Cq* variants resolve against the sticky entity's OWN + // nearest container-query ancestor (the `cq_frame` snapshot), + // via the shared `resolve_cq_unit_px` resolver — the same path + // sizing, tracks, and edges use. Cqi/Cqb resolve on the + // writing-mode inline/block axes; no-CQ-ancestor (cq_frame == + // None) falls back to viewport inside `resolve_cq_unit_px`, + // identical to every other Cq* site. Length::Cqw(_) | Length::Cqh(_) | Length::Cqi(_) | Length::Cqb(_) | Length::Cqmin(_) - | Length::Cqmax(_) => { - if warned - .set - .insert(LayoutWarnOnceKey::StickyCqDeferred(entity)) - { - warn!( - "Sticky entity {:?} uses Cq* inset; sticky-cq resolution is deferred to a Phase 7.x follow-up. Inset resolves to 0.0.", - entity, - ); - } - 0.0 - } + | Length::Cqmax(_) => resolve_cq_unit_px(*length, cq_frame, viewport, wmr), // `anchor-size()` reads an anchor box; sticky has none. Resolve // to 0.0, warn once per (entity, session) — same channel + dedup - // shape as the Fr / Cq* sticky-deferred arms above. + // shape as the Fr sticky-deferred arm above. Length::AnchorSize(_) => { if warned .set @@ -589,8 +586,13 @@ fn compute_sticky_displacement( /// Sticky behaves as `Relative` when no scroll-container ancestor is /// in scope (D5, silent no-op) — useful for sticky-in-static-context /// placeholder patterns. Percent insets resolve against the scroll -/// container's content-box axis size (D11). `Length::Fr` and -/// `Length::Cq*` insets warn-once-per-session and resolve to 0.0. +/// container's content-box axis size (D11). `Length::Cq*` insets +/// resolve against the sticky entity's own nearest container-query +/// ancestor (size read CURRENT-frame from Taffy) via the shared +/// `resolve_cq_unit_px`; the no-CQ-ancestor case falls back to the +/// viewport like every other Cq* site. `Length::Fr` (grid-only) and +/// `Length::AnchorSize` (sticky has no anchor box) warn-once-per-session +/// and resolve to 0.0. /// /// Spec: docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md § 2.3. #[allow(clippy::too_many_arguments)] @@ -600,6 +602,9 @@ pub(super) fn sticky_offset( overflow_q: Query<&Overflow>, scroll_offset_q: Query<&ScrollOffset>, parent_chain: Query<&ChildOf>, + container_q: Query<(Entity, &Container)>, + wmr_q: Query<&WritingModeResolved>, + primary_window: Query<&bevy::window::Window, With>, mut overrides: ResMut, mut warned: ResMut, ) { @@ -608,6 +613,39 @@ pub(super) fn sticky_offset( // redundant walks. let mut memo: HashMap<(Entity, Entity), Vec2> = HashMap::new(); + // CQ-ancestor frame source for `Length::Cq*` insets. Read + // CURRENT-frame from Taffy (NOT last-frame `&ResolvedLayout`): + // `sticky_offset` runs in `PostTaffyOverrides` (AFTER `TaffyCompute`, + // BEFORE `WriteResolvedLayout`), so a container's `ResolvedLayout` is + // stale here but its Taffy size is fresh — consistent with the + // self/parent/scroll sizes read from Taffy below. Entities Taffy + // hasn't placed yet are skipped (`.ok()`), same skip-this-frame + // semantics as the self/parent/scroll reads. + let container_index: HashMap = container_q + .iter() + .filter(|(_, c)| c.container_type != ContainerType::Normal) + .filter_map(|(entity, c)| { + let node = tree.by_entity.get(&entity)?; + let layout = tree.tree.layout(*node).ok()?; + Some(( + entity, + ContainerSnapshot { + container_type: c.container_type, + size: Vec2::new(layout.size.width, layout.size.height), + }, + )) + }) + .collect(); + + // Viewport fallback for the no-CQ-ancestor case (mirrors + // `sync_styles`). `resolve_cq_unit_px` uses this when an entity has + // no queried ancestor. + let viewport_size = primary_window + .single() + .ok() + .map(|w| Vec2::new(w.resolution.width(), w.resolution.height())) + .unwrap_or(Vec2::ZERO); + for (e, pos, display) in sticky_query.iter() { // D14 — filter in Rust (no Bevy `Without` exists, // `Or<>` slots are scarce). D10 — skip `Display::None`. @@ -669,14 +707,55 @@ pub(super) fn sticky_offset( .copied() .unwrap_or_default(); + // CQ frame + writing mode for any `Length::Cq*` inset — the + // sticky entity's OWN nearest CQ ancestor (walks `ChildOf` from + // the sticky entity, distinct from anchor's per-try anchor-box + // frame). Resolved once per entity (not per edge) to keep the + // `resolve_sticky_inset` signature L6-clean. + let cq_frame = nearest_container_with_size(e, &container_index, &parent_chain); + let wmr = wmr_q.get(e).copied().unwrap_or_default(); + // D3 / D11 — per-axis inset resolution. The caller passes the // correct scroll-container axis size (height for top/bottom, - // width for left/right); `resolve_sticky_inset` does not need - // an axis-tag parameter. - let top = resolve_sticky_inset(&pos.inset.top, s_size.y, e, &mut warned); - let bottom = resolve_sticky_inset(&pos.inset.bottom, s_size.y, e, &mut warned); - let left = resolve_sticky_inset(&pos.inset.left, s_size.x, e, &mut warned); - let right = resolve_sticky_inset(&pos.inset.right, s_size.x, e, &mut warned); + // width for left/right) for `Percent`; `resolve_sticky_inset` + // does not need an axis-tag parameter. `Cq*` resolves against + // the CQ frame + writing mode (not the scroll axis). + let top = resolve_sticky_inset( + &pos.inset.top, + s_size.y, + cq_frame, + viewport_size, + &wmr, + e, + &mut warned, + ); + let bottom = resolve_sticky_inset( + &pos.inset.bottom, + s_size.y, + cq_frame, + viewport_size, + &wmr, + e, + &mut warned, + ); + let left = resolve_sticky_inset( + &pos.inset.left, + s_size.x, + cq_frame, + viewport_size, + &wmr, + e, + &mut warned, + ); + let right = resolve_sticky_inset( + &pos.inset.right, + s_size.x, + cq_frame, + viewport_size, + &wmr, + e, + &mut warned, + ); let displacement = compute_sticky_displacement( e_in_s, diff --git a/crates/buiy_core/src/layout/types.rs b/crates/buiy_core/src/layout/types.rs index 956b3ab..a5ea332 100644 --- a/crates/buiy_core/src/layout/types.rs +++ b/crates/buiy_core/src/layout/types.rs @@ -1051,20 +1051,10 @@ pub enum LayoutWarnOnceKey { /// docs/plans/2026-05-22-buiy-layout-sticky-table-multicol.md. StickyFrUnsupported(Entity), - /// Sticky entity uses a `Length::Cq*` inset (container query - /// unit). Full cq-context resolution for sticky is deferred to - /// a Phase 7.x follow-up (port from Phase 6 `length_inset_to_px`). - /// v1 resolves to 0.0. One warn per (entity, session). - /// - /// Spec: plan decision D3 in - /// docs/plans/2026-05-22-buiy-layout-sticky-table-multicol.md. - StickyCqDeferred(Entity), - /// Sticky entity uses a `Length::AnchorSize` inset. `anchor-size()` /// reads the anchor target's box, but a sticky element has no anchor /// box — the term is meaningless here, so the inset resolves to 0.0. - /// One warn per (entity, session), mirroring `StickyFrUnsupported` / - /// `StickyCqDeferred`. + /// One warn per (entity, session), mirroring `StickyFrUnsupported`. /// /// Spec: docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md § 3.4. StickyAnchorSizeUnsupported(Entity), diff --git a/crates/buiy_core/tests/layout_sticky.rs b/crates/buiy_core/tests/layout_sticky.rs index 54265cd..643a73a 100644 --- a/crates/buiy_core/tests/layout_sticky.rs +++ b/crates/buiy_core/tests/layout_sticky.rs @@ -13,7 +13,7 @@ use bevy::prelude::*; use buiy_core::layout::{ Anchor, AnchorRef, Display, Inset, LayoutPlugin, LayoutWarnOnceKey, LayoutWarnedOnceSession, Length, OverflowMode, Position, PositionKind, PositionTry, PostTaffyPositionOverrides, - ScrollOffset, Sizing, Style, + ScrollOffset, Sizing, Style, WritingModeKind, }; use buiy_core::{Node, ResolvedLayout}; @@ -497,12 +497,29 @@ fn sticky_percent_inset_against_scroll_container() { // ===================================================================== #[test] -fn sticky_cq_inset_deferred_resolves_to_zero_with_warn() { - // Cqw inset on sticky → resolves to 0 and warns. With inset 0, - // top-pin threshold = visible_top + 0 = scroll_offset. natural - // y_in_block = 50, scroll_offset = 0 → no displacement. +fn sticky_cqw_inset_resolves_against_nearest_cq_ancestor() { + // The scroll container is ALSO a size-container (the sticky's own + // nearest CQ ancestor). 300-wide → Cqw(10) top inset = 10% of 300 = + // 30px. With scroll_offset.y = 100, visible_top = 100, threshold = + // 100 + 30 = 130. natural y_in_block = 50. max(50, 130) = 130, + // clamped by the tall parent (no clamp). Override carries (0, 130). + // + // SINGLE update: the CQ-ancestor size is read CURRENT-frame from + // Taffy (the same source as sticky's self/parent/scroll reads), so + // no two-frame establishment is needed. let mut app = app(); - let scroll = scroll_container(&mut app, 0.0); + let scroll = app + .world_mut() + .spawn(( + Node, + Style::default() + .width_px(300.0) + .height_px(500.0) + .overflow_y(OverflowMode::Scroll) + .container_size(), + ScrollOffset { x: 0.0, y: 100.0 }, + )) + .id(); let block = content_block(&mut app, scroll, 1000.0); let _spacer = content_block(&mut app, block, 50.0); let sticky = sticky_child( @@ -510,27 +527,187 @@ fn sticky_cq_inset_deferred_resolves_to_zero_with_warn() { block, 100.0, 30.0, - sticky_top_sizing(Sizing::Length(Length::Cqw(20.0))), + sticky_top_sizing(Sizing::Length(Length::Cqw(10.0))), ); app.update(); - // Inset resolves to 0; with scroll_offset=0 there's no - // displacement, so the override map should NOT contain the sticky - // entry. (Test ensures the no-op-write invariant holds in this - // path.) The warn is the focus assertion. let overrides = app.world().resource::(); - assert!( - !overrides.by_entity.contains_key(&sticky), - "Cq inset → 0; scroll_offset=0 → no displacement → no override entry", + let pos = overrides + .by_entity + .get(&sticky) + .copied() + .unwrap_or_else(|| panic!("expected sticky in overrides for Cqw-inset case")); + assert_eq!( + pos.y, 130.0, + "Cqw(10) of 300px CQ-ancestor width = 30px inset → threshold 130 \ + at scroll_y=100; got {:?}", + pos, ); +} - let warned = app.world().resource::(); - assert!( - warned - .set - .contains(&LayoutWarnOnceKey::StickyCqDeferred(sticky)), - "Cq inset should record a StickyCqDeferred warn for the sticky entity", +#[test] +fn sticky_cqi_inset_resolves_on_inline_axis_under_vertical_writing_mode() { + // Scroll container is a size-container 400-wide x 600-tall, with + // width != height so the axis swap is observable. The sticky child + // is in VerticalRl: inline axis = container HEIGHT (600), block axis + // = container WIDTH (400). Cqi(10) top inset = 10% of inline (600) = + // 60px (NOT 40px = 10% of width). scroll_y=100 → threshold 160. + // natural y_in_block = 50 → override.y = 160. + let mut app = app(); + let scroll = app + .world_mut() + .spawn(( + Node, + Style::default() + .width_px(400.0) + .height_px(600.0) + .overflow_y(OverflowMode::Scroll) + .container_size(), + ScrollOffset { x: 0.0, y: 100.0 }, + )) + .id(); + let block = content_block(&mut app, scroll, 1000.0); + let _spacer = content_block(&mut app, block, 50.0); + let sticky = app + .world_mut() + .spawn((Node, { + let mut s = Style::default() + .width_px(100.0) + .height_px(30.0) + .writing_mode_kind(WritingModeKind::VerticalRl); + s.position = sticky_top_sizing(Sizing::Length(Length::Cqi(10.0))); + s + })) + .id(); + app.world_mut().entity_mut(block).add_children(&[sticky]); + + app.update(); + + let overrides = app.world().resource::(); + let pos = overrides + .by_entity + .get(&sticky) + .copied() + .unwrap_or_else(|| panic!("expected sticky in overrides for Cqi-inset case")); + assert_eq!( + pos.y, 160.0, + "Cqi(10) under VerticalRl = 10%% of inline=HEIGHT(600) = 60px → \ + threshold 160 at scroll_y=100 (NOT 140 = 10%% of width); got {:?}", + pos, + ); +} + +#[test] +fn sticky_cqb_inset_resolves_on_block_axis_under_vertical_writing_mode() { + // Same fixture/writing mode as the Cqi test. Cqb resolves on the + // BLOCK axis = container WIDTH (400) under VerticalRl. Cqb(10) = + // 10% of 400 = 40px. scroll_y=100 → threshold 140. natural y = 50 → + // override.y = 140. A full size-container (not inline-size) is used + // so Cqb resolves on the real block axis (inline-size containers + // can't answer the block axis — they viewport-fall-back). + let mut app = app(); + let scroll = app + .world_mut() + .spawn(( + Node, + Style::default() + .width_px(400.0) + .height_px(600.0) + .overflow_y(OverflowMode::Scroll) + .container_size(), + ScrollOffset { x: 0.0, y: 100.0 }, + )) + .id(); + let block = content_block(&mut app, scroll, 1000.0); + let _spacer = content_block(&mut app, block, 50.0); + let sticky = app + .world_mut() + .spawn((Node, { + let mut s = Style::default() + .width_px(100.0) + .height_px(30.0) + .writing_mode_kind(WritingModeKind::VerticalRl); + s.position = sticky_top_sizing(Sizing::Length(Length::Cqb(10.0))); + s + })) + .id(); + app.world_mut().entity_mut(block).add_children(&[sticky]); + + app.update(); + + let overrides = app.world().resource::(); + let pos = overrides + .by_entity + .get(&sticky) + .copied() + .unwrap_or_else(|| panic!("expected sticky in overrides for Cqb-inset case")); + assert_eq!( + pos.y, 140.0, + "Cqb(10) under VerticalRl = 10%% of block=WIDTH(400) = 40px → \ + threshold 140 at scroll_y=100 (NOT 160 = 10%% of height); got {:?}", + pos, + ); +} + +#[test] +fn sticky_cqw_resolves_against_inner_cq_ancestor_not_scroll_container() { + // Three-level fixture proving the nearest-CQ walk is decoupled from + // the scroll-container walk: + // scroll container (overflow, NOT a size-container) 300-wide + // → middle size-container 500-wide (the nearest CQ ancestor) + // → sticky child, Cqw(10) top inset + // Cqw must resolve against the MIDDLE's width (500 → 50px), NOT the + // scroll container's width (300 → would be 30px). scroll_y=100 → + // threshold 150. natural y inside middle = 50 (50px spacer) → + // override.y = 150 (in middle's frame, which equals scroll frame + // here since middle starts at y=0). + let mut app = app(); + let scroll = app + .world_mut() + .spawn(( + Node, + Style::default() + .width_px(300.0) + .height_px(500.0) + .overflow_y(OverflowMode::Scroll), + ScrollOffset { x: 0.0, y: 100.0 }, + )) + .id(); + // Middle: a size-container 500-wide, tall enough to scroll within. + let middle = app + .world_mut() + .spawn(( + Node, + Style::default() + .width_px(500.0) + .height_px(1000.0) + .container_size(), + )) + .id(); + app.world_mut().entity_mut(scroll).add_children(&[middle]); + let _spacer = content_block(&mut app, middle, 50.0); + let sticky = sticky_child( + &mut app, + middle, + 100.0, + 30.0, + sticky_top_sizing(Sizing::Length(Length::Cqw(10.0))), + ); + + app.update(); + + let overrides = app.world().resource::(); + let pos = overrides + .by_entity + .get(&sticky) + .copied() + .unwrap_or_else(|| panic!("expected sticky in overrides for inner-CQ-ancestor case")); + assert_eq!( + pos.y, 150.0, + "Cqw(10) resolves against the MIDDLE CQ ancestor width (500 → 50px), \ + NOT the scroll container (300 → 30px) → threshold 150; got {:?}", + pos, ); } diff --git a/docs/plans/follow-ups.md b/docs/plans/follow-ups.md index b92c69c..57dd5af 100644 --- a/docs/plans/follow-ups.md +++ b/docs/plans/follow-ups.md @@ -324,21 +324,45 @@ metrics deferred). **Spec touchpoint:** `flex-and-grid.md § 3` (multi-column). -## Layout — sticky `Length::Cq*` inset resolution +## Layout — sticky `Length::Cq*` inset resolution — LANDED **Originated:** Phase 7 (D3 deferral). +**Status:** **Landed.** Sticky `Length::Cqw/Cqh/Cqi/Cqb/Cqmin/Cqmax` +insets now resolve via the shared `translate.rs::resolve_cq_unit_px` +against the sticky entity's own nearest container-query ancestor +(`Container { container_type != Normal }`, found by walking `ChildOf` +from the sticky entity — distinct from anchor's per-try "anchor target +box" frame). The CQ-ancestor size is read CURRENT-frame from Taffy +(`tree.by_entity` + `tree.tree.layout`), NOT from last-frame +`&ResolvedLayout`: `sticky_offset` runs in `PostTaffyOverrides` (after +`TaffyCompute`, before `WriteResolvedLayout`), so a container's +`ResolvedLayout` is stale there but its Taffy size is fresh — this keeps +sticky `Cq*` same-frame-consistent with the self/parent/scroll sizes +`sticky_offset` already reads from Taffy, and collapses the tests to a +single `app.update()`. Cqi/Cqb resolve on the writing-mode inline/block +axes (per-entity `WritingModeResolved`); the no-CQ-ancestor case rides +`resolve_cq_unit_px`'s existing viewport fallback, identical to every +other Cq* site. The `StickyCqDeferred` warn variant was **retired** +(delegating to `resolve_cq_unit_px` makes it dead; keeping it would +double-warn). Covered by `sticky_cqw_inset_resolves_against_nearest_cq_ancestor`, +`sticky_cqi_inset_resolves_on_inline_axis_under_vertical_writing_mode`, +`sticky_cqb_inset_resolves_on_block_axis_under_vertical_writing_mode`, +and `sticky_cqw_resolves_against_inner_cq_ancestor_not_scroll_container` +in `tests/layout_sticky.rs`. + **Symptom:** Sticky entity with `Length::Cqw/Cqh/Cqi/Cqb/Cqmin/Cqmax` -inset emits `LayoutWarnOnceKey::StickyCqDeferred(Entity)` and resolves to +inset emitted `LayoutWarnOnceKey::StickyCqDeferred(Entity)` and resolved to 0.0. -**Implementation sketch:** port Phase 6's `length_inset_to_px` cq-context -resolver. Sticky's reference frame is the sticky entity's own nearest CQ -ancestor (distinct from anchor's "anchor target box" frame). Multi-axis -fixture needed (Cqi/Cqb resolve against writing-mode inline/block axes). +**Implementation sketch (as landed):** reuse `resolve_cq_unit_px` (the +same resolver sizing/tracks/edges use) instead of porting Phase 6's +anchor-box-shaped `length_inset_to_px`. Sticky's reference frame is the +sticky entity's own nearest CQ ancestor. Multi-axis fixtures pin the +Cqi/Cqb writing-mode axis swap. **Spec touchpoint:** `display-and-positioning.md § 2.3`, -`container-queries-and-writing-modes.md § 1`. +`container-queries-and-writing-modes.md § 1.4`. ## Layout — sticky em/rem/Vh/Vw/Vmin/Vmax inset support diff --git a/docs/specs/2026-05-08-buiy-layout-design/README.md b/docs/specs/2026-05-08-buiy-layout-design/README.md index db287e1..b8d99ca 100644 --- a/docs/specs/2026-05-08-buiy-layout-design/README.md +++ b/docs/specs/2026-05-08-buiy-layout-design/README.md @@ -94,6 +94,7 @@ This spec is target-state. The Phase 0 → target migration (the decomposed-comp - **`writing-mode: sideways-*` Taffy support.** Taffy 0.10 has logical properties but doesn't fully model sideways modes. Whether to ship a Buiy-side rotation pass or wait on Taffy is open. [container-queries-and-writing-modes.md](container-queries-and-writing-modes.md) details. - **Top-layer ordering across windows.** When a Buiy app has multiple windows each with its own modal, modal stacking is per-window. Cross-window top-layer (a modal that visually escapes its window) is out of scope; tracked in `buiy-window-and-surface-design`. - **`anchor-size()` in `PositionTry::inset` — RESOLVED.** Previously deferred to v1.x with no API surface; now ships as `Length::AnchorSize(AxisDimension)`, resolved against the per-try anchor box at anchor-resolution time (no Taffy re-layout). See [display-and-positioning.md § 3.4](display-and-positioning.md#34-performance-and-ordering). +- **Sticky `Length::Cq*` inset — RESOLVED.** Previously deferred (`StickyCqDeferred` warn-once, resolved to 0); now resolves via the shared `resolve_cq_unit_px` against the sticky entity's own nearest container-query ancestor (size read current-frame from Taffy), `Cqi`/`Cqb` on the writing-mode inline/block axes, viewport fallback when there is no CQ ancestor. See [display-and-positioning.md § 2.3](display-and-positioning.md#23-sticky-positioning). ## References diff --git a/docs/specs/2026-05-08-buiy-layout-design/container-queries-and-writing-modes.md b/docs/specs/2026-05-08-buiy-layout-design/container-queries-and-writing-modes.md index a673ac0..f261b36 100644 --- a/docs/specs/2026-05-08-buiy-layout-design/container-queries-and-writing-modes.md +++ b/docs/specs/2026-05-08-buiy-layout-design/container-queries-and-writing-modes.md @@ -98,6 +98,8 @@ If no queried ancestor exists, container units fall back to viewport units (`cqw `cqi` / `cqb` against a container with `ContainerType::InlineSize` resolve only on the inline axis; querying the block axis falls back to the same warn-and-degrade path. (See [README § 5 — open questions](README.md#5-open-questions) for nested-container subtleties.) +**Consumers.** The shared `resolve_cq_unit_px` resolver is used by sizing, grid tracks, edge translation, *and* sticky positioning insets — a sticky `Length::Cq*` inset resolves against the sticky entity's own nearest queried ancestor (size read current-frame from Taffy). See [display-and-positioning.md § 2.3](display-and-positioning.md). + ### 1.5 Test surface - **Activation flip** — fixture with one `@container (min-width: 600px)` rule; resize container from 500 → 700 in two frames; assert this frame's `ResolvedLayout` reflects the activated rule. diff --git a/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md b/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md index 3525ece..db5d5e9 100644 --- a/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md +++ b/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md @@ -159,6 +159,8 @@ A sticky element behaves as `Relative` until its scroll container's scroll offse Sticky offsets *do not* invalidate Taffy. The element's contribution to its parent's flow is computed as `Relative`; the sticky displacement is a render-time visual offset baked into `ResolvedLayout.position` after Taffy. +Inset units resolve as follows. `Px` is literal; `Percent` resolves against the scroll container's content-box axis size (height for top/bottom, width for left/right). `Length::Cq*` insets resolve against the **sticky entity's own nearest container-query ancestor** (`Container { container_type != Normal }`, found by walking `ChildOf` from the sticky entity) via the shared `resolve_cq_unit_px` resolver — the same path sizing, tracks, and edges use. The CQ-ancestor size is read **current-frame from Taffy** (consistent with sticky's self/parent/scroll-container reads, since sub-pass 6a runs after Taffy compute but before `ResolvedLayout` write-back). `Cqi`/`Cqb` resolve on the writing-mode inline/block axes; when there is no CQ ancestor the unit falls back to the viewport like every other `Cq*` site. This frame is the sticky entity's *own* ancestor — distinct from anchor positioning's per-try "anchor target box" frame (§ 3.4). `Length::Fr` (grid-only) and `Length::AnchorSize` (sticky has no anchor box) are meaningless as sticky insets: they resolve to `0` and warn once per (entity, session). See [container-queries-and-writing-modes.md § 1.4](container-queries-and-writing-modes.md). + ## 3. Anchor positioning Tier-C. Buiy-owned, post-Taffy. CSS Anchor Positioning Module Level 1. @@ -257,7 +259,7 @@ commands.spawn(( - An anchor target must resolve before its dependent. Sub-pass 6d builds a Kahn topological sort over the (anchored → anchor) DAG: `O(V + E)` where V = anchored entities, E = anchor edges (= V, since each anchored entity has exactly one anchor target). - **Cycle handling.** Edges that would close a cycle are dropped — the dropped edge is `(child anchored, anchor)` for the *most-recently-inserted* anchored entity in the cycle (tracked by the `AnchorNameRegistry` insertion epoch and an analogous epoch on `AnchorRef::Entity`). Both endpoints get `LayoutAnchorBroken` markers; one `warn!` fires per cycle per frame naming the dropped edge. Result: every cycle resolves deterministically; tests can assert exact membership. - Cost: `O(anchored entities × tries + V + E)`. Usually small — most anchored elements have 1-3 fallbacks. -- Anchor resolution does **not** trigger Taffy re-layout. The anchor's *size* is fixed by the time anchor resolution runs; only its position changes. `anchor-size()` exploits exactly this: because the anchor's size is already resolved when anchor resolution runs, an `anchor-size()` term reads that size directly — no Taffy re-layout is needed. It ships as `Length::AnchorSize(AxisDimension)` (the `AxisDimension::{Width, Height}` selector names which axis of the anchor box to read), usable inside `PositionTry::inset` via `Sizing::Length(Length::AnchorSize(_))`. At resolution time `try_anchored_position` ([systems.rs](../../../crates/buiy_core/src/layout/systems.rs)) resolves it against the per-try anchor box: `Width → anchor.size.x`, `Height → anchor.size.y`, independent of which edge the term sits on. In every non-anchor context (track sizing, Taffy dimension translation, container-query condition values, sticky insets) it degrades to `0`/`auto` exactly like `Cq*` outside its query; a sticky inset additionally warns once per (entity, session) via `LayoutWarnOnceKey::StickyAnchorSizeUnsupported` because sticky has no anchor box. The former `AnchorErrorKind::AnchorSizeUsed` warn arm (a v1.x forward-compat hook for the deferred feature) has been **retired** — anchor-size now lands and resolves to a real value, so an error/warn kind would be semantically wrong. +- Anchor resolution does **not** trigger Taffy re-layout. The anchor's *size* is fixed by the time anchor resolution runs; only its position changes. `anchor-size()` exploits exactly this: because the anchor's size is already resolved when anchor resolution runs, an `anchor-size()` term reads that size directly — no Taffy re-layout is needed. It ships as `Length::AnchorSize(AxisDimension)` (the `AxisDimension::{Width, Height}` selector names which axis of the anchor box to read), usable inside `PositionTry::inset` via `Sizing::Length(Length::AnchorSize(_))`. At resolution time `try_anchored_position` ([systems.rs](../../../crates/buiy_core/src/layout/systems.rs)) resolves it against the per-try anchor box: `Width → anchor.size.x`, `Height → anchor.size.y`, independent of which edge the term sits on. In every non-anchor context (track sizing, Taffy dimension translation, container-query condition values) it degrades to `0`/`auto` exactly like `Cq*` outside its query; on a sticky inset it resolves to `0` and warns once per (entity, session) via `LayoutWarnOnceKey::StickyAnchorSizeUnsupported` because sticky has no anchor box. (Sticky `Cq*` insets, unlike `anchor-size()`, *do* resolve — against the sticky entity's own CQ ancestor, see § 2.3 — so they are not part of this degrade-to-0 list.) The former `AnchorErrorKind::AnchorSizeUsed` warn arm (a v1.x forward-compat hook for the deferred feature) has been **retired** — anchor-size now lands and resolves to a real value, so an error/warn kind would be semantically wrong. ### 3.5 Open question: `position-try` chain depth From 782728e4acae4c73e6ed2bc49e279affb962e66a Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 10:50:38 -0700 Subject: [PATCH 6/9] feat(layout): sticky both-top-and-bottom dual clamp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A sticky element with BOTH inset_top and inset_bottom set now honors both edges (CSS positioned-layout §6.3 dual-clamp) instead of the v1 'top wins' deviation. compute_sticky_displacement applies both thresholds per axis: pin to the top line (max U) when scroll pushes the natural position above it, pin to the bottom line (min L) when below it, natural in between. The U>L degenerate case (box taller than the sticky band) re-applies top-precedence after the bottom clamp, so the box never escapes upward. Band-clamp keeps the explicit .max(lo).min(hi) ordering (panic-free when box taller than parent). Single-edge cases unchanged (verified at runtime, not just by reasoning). Flipped both regression tests: the pure unit sticky_both_top_and_bottom_* (top_wins -> dual_clamp_bottom_honored) and the integration sticky_both_top_and_bottom_inset_top_wins -> _bottom_honored_near_scroll_end; added a dedicated U>L conflict test (anti-vacuous: deleting the top-precedence branch fails it) and a both-extremes integration fixture. Tests: layout_sticky 19/19, lib layout 226/226. Supersedes Phase 7 D4 in CHANGELOG; spec §2.3 documents dual-clamp; follow-ups.md LANDED. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- CHANGELOG.md | 27 +++- crates/buiy_core/src/layout/systems.rs | 152 ++++++++++++------ crates/buiy_core/tests/layout_sticky.rs | 127 ++++++++++----- docs/plans/follow-ups.md | 40 +++-- .../display-and-positioning.md | 2 + 5 files changed, 247 insertions(+), 101 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d99f810..e758212 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -582,12 +582,27 @@ tagged release. (viewport units) or a font-rendering phase adds them, the sticky inset resolver gains new arms (currently closed-match — compiler forces the addition). -- **Both-top-and-bottom-inset sticky — "top wins" (v1 deviation).** Per - D4. CSS spec § 6.3 implies a dual-clamp ("sticks to whichever edge - the scroll position is currently closer to"); Phase 7's simpler "top - wins" matches WebKit/Blink in the common case. Documented test - `sticky_both_top_and_bottom_inset_top_wins` is the regression test; - flipping it documents the algorithm upgrade. Tracked in follow-ups. +- **Both-top-and-bottom-inset sticky — dual-clamp (CSS § 6.3).** + Superseded the Phase 7 D4 "top wins" deviation. + `compute_sticky_displacement` now applies both the top line + `U = visible_top + top_px` + and the bottom line `L = (visible_bottom - bottom_px) - size` + simultaneously per axis: the box pins to `U` when scroll pushes its + natural position above it, pins to `L` when scroll pushes it below, + and sits at natural flow in between — honoring the bottom inset that + the v1 single-edge `if-top else-if-bottom` chain left unreachable. In + the degenerate case where the band is shorter than the box (`U > L`) + the top edge takes precedence (`.max(U)` re-applied after the bottom + `.min(L)`). Each axis still reduces to the single-edge formula when + only one inset is set. The v1 regression test + `sticky_both_top_and_bottom_inset_top_wins` was flipped to + `sticky_both_top_and_bottom_bottom_honored_near_scroll_end` + (integration) and the pure unit test to + `sticky_both_top_and_bottom_dual_clamp_bottom_honored`; the new + `sticky_both_top_and_bottom_conflict_top_precedence` unit fixture + locks the `U > L` top-precedence branch and + `sticky_both_insets_clamp_at_both_extremes` proves both edges clamp at + their respective scroll extremes. - **Sticky inside sticky — inner uses outer's natural (un-displaced) position.** `world_position` walks Taffy positions; an inner sticky inside an outer sticky resolves its threshold against the outer's diff --git a/crates/buiy_core/src/layout/systems.rs b/crates/buiy_core/src/layout/systems.rs index 1ef74f4..2b98000 100644 --- a/crates/buiy_core/src/layout/systems.rs +++ b/crates/buiy_core/src/layout/systems.rs @@ -500,12 +500,21 @@ fn resolve_sticky_inset( /// natural-relative-to-parent position to get the final /// position-in-parent-frame. /// -/// **v1 deviation: when both `inset_top` and `inset_bottom` are set, -/// top wins.** A future correct dual-clamp implementation will replace -/// this if-else with an "upper-stuck vs lower-stuck, smallest -/// perturbation from natural wins" rule (CSS spec § 6.3). Documented in -/// CHANGELOG; the `sticky_both_top_and_bottom_active_top_wins` test -/// pins the current behavior. +/// **Dual-clamp (CSS positioned-layout § 6.3): when both `inset_top` +/// and `inset_bottom` are set, both clamps apply simultaneously.** The +/// box pins to the top line `U = visible_top + top_px` when scroll +/// pushes its natural position above `U`, pins to the bottom line +/// `L = (visible_bottom - bottom_px) - size` when scroll pushes it below +/// `L`, and sits at its natural flow position in between. When the box +/// is shorter than the sticky band the two clamps don't conflict and it +/// follows whichever edge the scroll has reached. In the degenerate case +/// where the band is shorter than the box (`U > L`, thresholds collide), +/// CSS gives the TOP edge precedence — re-applied with `.max(U)` *after* +/// the bottom `.min(L)` so the bottom clamp cannot win. Each axis +/// degenerates to the single-edge formula when only one inset is set. +/// The `sticky_both_top_and_bottom_dual_clamp_bottom_honored` and +/// `sticky_both_top_and_bottom_conflict_top_precedence` unit tests pin +/// this behavior (the latter locks the `U > L` re-max branch). /// /// Pure function — no Bevy queries, no Taffy reads. Easy to unit test. /// @@ -531,38 +540,44 @@ fn compute_sticky_displacement( let parent_bottom = parent_in_s.y + parent_size.y; let parent_right = parent_in_s.x + parent_size.x; - let desired_y = if let Some(top_px) = inset_top { - let threshold = visible_top + top_px; - e_natural_in_s - .y - .max(threshold) - .min(parent_bottom - e_size.y) - .max(parent_in_s.y) - } else if let Some(bottom_px) = inset_bottom { - let threshold = visible_bottom - bottom_px; - (threshold - e_size.y) - .min(e_natural_in_s.y) - .max(parent_in_s.y) - .min(parent_bottom - e_size.y) - } else { - e_natural_in_s.y - }; - let desired_x = if let Some(left_px) = inset_left { - let threshold = visible_left + left_px; - e_natural_in_s - .x - .max(threshold) - .min(parent_right - e_size.x) - .max(parent_in_s.x) - } else if let Some(right_px) = inset_right { - let threshold = visible_right - right_px; - (threshold - e_size.x) - .min(e_natural_in_s.x) - .max(parent_in_s.x) - .min(parent_right - e_size.x) - } else { - e_natural_in_s.x - }; + // Dual-clamp per CSS § 6.3 (see doc comment). Apply the top line U + // (.max), then the bottom line L (.min); when both are set and the + // band is shorter than the box (U > L) re-apply .max(U) so the top + // edge takes precedence. Then clamp into the parent band with an + // explicit .max(lo).min(hi) — NOT f32::clamp, which panics when + // lo > hi (box taller than the parent band; the + // sticky_clamped_by_parent_* tests depend on graceful degradation). + let mut desired_y = e_natural_in_s.y; + if let Some(top_px) = inset_top { + desired_y = desired_y.max(visible_top + top_px); + } + if let Some(bottom_px) = inset_bottom { + desired_y = desired_y.min((visible_bottom - bottom_px) - e_size.y); + } + if let (Some(top_px), Some(bottom_px)) = (inset_top, inset_bottom) { + let u = visible_top + top_px; + let l = (visible_bottom - bottom_px) - e_size.y; + if u > l { + desired_y = desired_y.max(u); + } + } + let desired_y = desired_y.max(parent_in_s.y).min(parent_bottom - e_size.y); + + let mut desired_x = e_natural_in_s.x; + if let Some(left_px) = inset_left { + desired_x = desired_x.max(visible_left + left_px); + } + if let Some(right_px) = inset_right { + desired_x = desired_x.min((visible_right - right_px) - e_size.x); + } + if let (Some(left_px), Some(right_px)) = (inset_left, inset_right) { + let u = visible_left + left_px; + let l = (visible_right - right_px) - e_size.x; + if u > l { + desired_x = desired_x.max(u); + } + } + let desired_x = desired_x.max(parent_in_s.x).min(parent_right - e_size.x); Vec2::new(desired_x - e_natural_in_s.x, desired_y - e_natural_in_s.y) } @@ -5394,29 +5409,68 @@ mod tests { assert_eq!(d, Vec2::new(0.0, -180.0)); } - // ---- v2 — both-top-and-bottom-active behavior (test-reviewer BLOCKER B2) ---- + // ---- dual-clamp — both-top-and-bottom (CSS § 6.3) ---- #[test] - fn sticky_both_top_and_bottom_active_top_wins() { - // v1 deviation: when both insets are set, top wins. This test - // documents the behavior — a future correct dual-clamp impl - // will fail this test and that's the signal to flip it. + fn sticky_both_top_and_bottom_dual_clamp_bottom_honored() { + // both insets set → bottom honored when scroll near end + // (dual-clamp § 6.3, supersedes v1 top-wins). Scroll near the + // bottom so the TOP clamp is inactive and the BOTTOM clamp fires: + // e_natural_in_s.y=900, scroll_offset.y=400. + // visible_top=400 → U=410; 900 > 410 so the top-clamp does not + // move the box (desired stays 900 after .max(U)). + // visible_bottom=900 → L=900-10-30=860; .min(L) pulls to 860. + // U(410) <= L(860): no conflict re-max. + // band [0, 1000-30=970] keeps 860. + // displacement = 860 - 900 = -40. + // Under the v1 "top wins" helper the bottom branch was unreachable + // and the top branch returned displacement 0 (Vec2::ZERO), so this + // assertion is RED against the pre-dual-clamp code. let d = compute_sticky_displacement( - Vec2::new(0.0, 50.0), + Vec2::new(0.0, 900.0), Vec2::new(100.0, 30.0), Vec2::new(0.0, 0.0), Vec2::new(300.0, 1000.0), Vec2::new(300.0, 500.0), - Vec2::new(0.0, 100.0), + Vec2::new(0.0, 400.0), Some(10.0), Some(10.0), None, None, // both insets set ); - // Top-pin branch fires: visible_top=100, threshold=110, - // max(50, 110)=110. Clamped by parent_bottom - e_h = 970 → 110. - // Displacement = 60. Bottom inset is ignored. - assert_eq!(d, Vec2::new(0.0, 60.0)); + assert_eq!(d, Vec2::new(0.0, -40.0)); + } + + #[test] + fn sticky_both_top_and_bottom_conflict_top_precedence() { + // U>L conflict (band shorter than box): top-precedence pins to U, + // NOT the naive bottom clamp. This is the ONLY test that exercises + // the `if U>L { desired = desired.max(U) }` re-max branch — its + // job is to LOCK that branch (the anti-vacuous check: deleting the + // branch must break this test). Its RED signal is the + // branch-deletion check, not the legacy helper (the old top-wins + // single-edge branch happens to return the same value here). + // + // Geometry: scroll_container_size.y=40 (band only 40px tall), + // e_size=(100,30), both insets 10, scroll_offset ZERO. + // visible_top=0 → U=10; visible_bottom=40 → L=40-10-30=0. + // U(10) > L(0) → conflict. + // e_natural_in_s.y=5: .max(U=10)=10, .min(L=0)=0 (naive would + // stop here at 0), re-.max(U=10)=10 (top-precedence wins). + // band [0, 1000-30=970] keeps 10. displacement = 10 - 5 = 5. + let d = compute_sticky_displacement( + Vec2::new(0.0, 5.0), + Vec2::new(100.0, 30.0), + Vec2::new(0.0, 0.0), + Vec2::new(300.0, 1000.0), + Vec2::new(300.0, 40.0), + Vec2::ZERO, + Some(10.0), + Some(10.0), + None, + None, + ); + assert_eq!(d, Vec2::new(0.0, 5.0)); } // ----------------------------------------------------------------- diff --git a/crates/buiy_core/tests/layout_sticky.rs b/crates/buiy_core/tests/layout_sticky.rs index 643a73a..b98b6d6 100644 --- a/crates/buiy_core/tests/layout_sticky.rs +++ b/crates/buiy_core/tests/layout_sticky.rs @@ -348,44 +348,38 @@ fn sticky_bottom_clamped_by_parent_top() { } // ===================================================================== -// Both-active conflict +// Both-active dual-clamp (CSS § 6.3) // ===================================================================== +/// Construct a sticky `Position` with BOTH top and bottom px insets. +fn sticky_both_insets(top_px: f32, bottom_px: f32) -> Position { + Position { + kind: PositionKind::Sticky, + inset: Inset { + top: Sizing::Length(Length::Px(top_px)), + bottom: Sizing::Length(Length::Px(bottom_px)), + ..Default::default() + }, + } +} + #[test] -fn sticky_both_top_and_bottom_inset_top_wins() { - // v1 deviation per CHANGELOG: when both insets are set, the - // top-pin branch fires first and the bottom inset is ignored. - // Future correct dual-clamp implementation will replace this test - // with one that asserts an "upper-stuck vs lower-stuck, smallest - // perturbation wins" rule. - // - // Setup: sticky at natural y_in_S = 50, top inset 10, bottom inset - // 10, scroll_offset.y = 100. Top-pin branch: visible_top=100, - // threshold=110, max(50, 110)=110. Clamped by parent_bottom-size - // = 1000-30 = 970 → 110. displacement = 60. - // - // If bottom-pin were applied first (wrong), it'd push the element - // up — different displacement. The test pins the top-wins choice. +fn sticky_both_top_and_bottom_bottom_honored_near_scroll_end() { + // both insets set → bottom honored when scroll near end (dual-clamp, + // supersedes v1 top-wins). Mirrors `sticky_bottom_pins_when_scroll_ + // near_bottom`: 300x500 scroll container, scroll_offset.y=400, + // 1000-tall block, 900-px spacer → sticky natural y_in_S=900. + // visible_top=400 → U=410; 900 > 410, top-clamp inactive. + // visible_bottom=900 → L=900-10-30=860; .min(L)=860. + // U(410) <= L(860): no re-max. band [0,970] keeps 860. + // override.y = 900 + (860-900) = 860. + // Under the v1 "top-wins" helper the top branch returned displacement + // 0 → no override entry → the .get().unwrap_or_else panics (valid RED). let mut app = app(); - let scroll = scroll_container(&mut app, 100.0); + let scroll = scroll_container(&mut app, 400.0); let block = content_block(&mut app, scroll, 1000.0); - let _spacer = content_block(&mut app, block, 50.0); - let sticky = app - .world_mut() - .spawn((Node, { - let mut s = Style::default().width_px(100.0).height_px(30.0); - s.position = Position { - kind: PositionKind::Sticky, - inset: Inset { - top: Sizing::Length(Length::Px(10.0)), - bottom: Sizing::Length(Length::Px(10.0)), - ..Default::default() - }, - }; - s - })) - .id(); - app.world_mut().entity_mut(block).add_children(&[sticky]); + let _spacer = content_block(&mut app, block, 900.0); + let sticky = sticky_child(&mut app, block, 100.0, 30.0, sticky_both_insets(10.0, 10.0)); app.update(); @@ -394,15 +388,74 @@ fn sticky_both_top_and_bottom_inset_top_wins() { .by_entity .get(&sticky) .copied() - .unwrap_or_else(|| panic!("expected sticky entry for both-active case")); - // Top branch wins: y_in_block = 50 + 60 = 110. + .unwrap_or_else(|| panic!("expected sticky entry for both-active dual-clamp case")); assert_eq!( - pos.y, 110.0, - "both insets set → top wins (v1 deviation); got {:?}", + pos.y, 860.0, + "both insets set → bottom honored when scroll near end (dual-clamp § 6.3, supersedes v1 top-wins); got {:?}", pos, ); } +#[test] +fn sticky_both_insets_clamp_at_both_extremes() { + // Symmetry fixture: the same both-insets sticky element pins to the + // TOP line U at a top-extreme scroll and to the BOTTOM line L at a + // bottom-extreme scroll. Drives two independent apps so the suite + // documents top-extreme and bottom-extreme distinctly. + // + // Case (a) — top-extreme: scroll_offset.y=300, 50-px spacer. + // natural y_in_S=50; visible_top=300 → U=310; .max(U)=310. + // visible_bottom=800 → L=800-10-30=760; .min(L)=310. + // U(310) <= L(760): no re-max. band [0,970] keeps 310. + // override.y = 50 + (310-50) = 310. (Old top-wins code also yields + // 310 here — case (a) is the symmetry counterpart, not the RED.) + { + let mut app = app(); + let scroll = scroll_container(&mut app, 300.0); + let block = content_block(&mut app, scroll, 1000.0); + let _spacer = content_block(&mut app, block, 50.0); + let sticky = sticky_child(&mut app, block, 100.0, 30.0, sticky_both_insets(10.0, 10.0)); + + app.update(); + + let overrides = app.world().resource::(); + let pos = overrides + .by_entity + .get(&sticky) + .copied() + .unwrap_or_else(|| panic!("expected sticky entry for top-extreme dual-clamp case")); + assert_eq!( + pos.y, 310.0, + "top-extreme: both-insets sticky pins to the top line U=310; got {:?}", + pos, + ); + } + + // Case (b) — bottom-extreme: scroll_offset.y=400, 900-px spacer. + // natural y_in_S=900; visible_bottom=900 → L=860; pins to 860. + { + let mut app = app(); + let scroll = scroll_container(&mut app, 400.0); + let block = content_block(&mut app, scroll, 1000.0); + let _spacer = content_block(&mut app, block, 900.0); + let sticky = sticky_child(&mut app, block, 100.0, 30.0, sticky_both_insets(10.0, 10.0)); + + app.update(); + + let overrides = app.world().resource::(); + let pos = overrides + .by_entity + .get(&sticky) + .copied() + .unwrap_or_else(|| panic!("expected sticky entry for bottom-extreme dual-clamp case")); + assert_eq!( + pos.y, 860.0, + "bottom-extreme: both-insets sticky pins to the bottom line L=860; got {:?}", + pos, + ); + } +} + // ===================================================================== // No-scroll-container — silent no-op // ===================================================================== diff --git a/docs/plans/follow-ups.md b/docs/plans/follow-ups.md index 57dd5af..b5fb023 100644 --- a/docs/plans/follow-ups.md +++ b/docs/plans/follow-ups.md @@ -379,23 +379,45 @@ these `Length` variants, extend `resolve_sticky_inset` with new arms **Spec touchpoint:** Phase 10 — viewport units; future font-rendering spec — em/rem. -## Layout — sticky both-top-and-bottom dual clamp +## Layout — sticky both-top-and-bottom dual clamp — LANDED **Originated:** Phase 7 (D4 — v1 "top wins" deviation). +**Status:** **Landed.** `compute_sticky_displacement` now honors the +bottom inset: when both `inset_top` and `inset_bottom` (or both +`inset_left`/`inset_right`) are set, both clamps apply simultaneously per +CSS § 6.3. Each axis applies the top line `U = visible_top + top_px` +(`.max(U)`), then the bottom line `L = (visible_bottom − bottom_px) − +size` (`.min(L)`); when the band is shorter than the box (`U > L`) it +re-applies `.max(U)` so the top edge takes precedence. The band clamp +stays an explicit `.max(parent_lo).min(parent_hi)` (NOT `f32::clamp`, +which panics when `lo > hi`). Each axis reduces to the prior single-edge +formula when only one inset is set (the single-edge regression tests stay +green). The v1 "top wins" tests were flipped: the integration regression +test `sticky_both_top_and_bottom_inset_top_wins` → +`sticky_both_top_and_bottom_bottom_honored_near_scroll_end` and the pure +unit test `sticky_both_top_and_bottom_active_top_wins` → +`sticky_both_top_and_bottom_dual_clamp_bottom_honored` (both now assert +the bottom inset wins near the scroll end). New fixtures: the +`sticky_both_top_and_bottom_conflict_top_precedence` unit test locks the +`U > L` top-precedence re-max branch (verified anti-vacuous: deleting the +branch breaks it), and `sticky_both_insets_clamp_at_both_extremes` +(integration) proves both edges clamp at their respective scroll +extremes. + **Symptom:** Sticky element with both `inset_top` and `inset_bottom` set -ignores the bottom inset (top wins). CSS spec § 6.3 implies dual-clamp +ignored the bottom inset (top wins). CSS spec § 6.3 implies dual-clamp behavior where the element sticks to whichever edge the scroll position is closer to. -**Implementation sketch:** implement dual-clamp in -`compute_sticky_displacement` — likely requires storing both upper and -lower sticky thresholds and computing midpoint logic. The v2 test -`sticky_both_top_and_bottom_inset_top_wins` (in `tests/layout_sticky.rs`) -is the regression test for the v1 "top wins" behavior — flipping it -documents the algorithm upgrade. +**Implementation sketch (as landed):** replace each axis's +`if-top else-if-bottom` chain (which left the bottom branch unreachable +when both were set) with a single dual-clamp expression applying both +thresholds, plus the `U > L` top-precedence re-max for the degenerate +band-shorter-than-box case. The 10-arg signature is unchanged. -**Spec touchpoint:** CSS spec § 6.3 (positioned layout). +**Spec touchpoint:** `display-and-positioning.md § 2.3`; CSS spec § 6.3 +(positioned layout). ## Layout — sticky inside sticky diff --git a/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md b/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md index db5d5e9..a57cb36 100644 --- a/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md +++ b/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md @@ -159,6 +159,8 @@ A sticky element behaves as `Relative` until its scroll container's scroll offse Sticky offsets *do not* invalidate Taffy. The element's contribution to its parent's flow is computed as `Relative`; the sticky displacement is a render-time visual offset baked into `ResolvedLayout.position` after Taffy. +When **both** the top and bottom insets (or both left and right) are set on the same axis, the box is dual-clamped per CSS positioned-layout § 6.3: both clamps apply simultaneously. On the block axis the box pins to the top line `U = visible_top + top_inset` when scroll pushes its natural position above `U`, pins to the bottom line `L = (visible_bottom − bottom_inset) − box_height` when scroll pushes it below `L`, and sits at its natural flow position in between — following whichever edge the scroll has reached (the inline axis mirrors this with left/right). When the box is shorter than the sticky band the two clamps never conflict; in the degenerate case where the band is shorter than the box (`U > L`, the thresholds collide) the **top edge takes precedence**. Each axis reduces to the single-edge behavior when only one inset is set. Implemented in the pure `compute_sticky_displacement` helper ([systems.rs](../../../crates/buiy_core/src/layout/systems.rs)). + Inset units resolve as follows. `Px` is literal; `Percent` resolves against the scroll container's content-box axis size (height for top/bottom, width for left/right). `Length::Cq*` insets resolve against the **sticky entity's own nearest container-query ancestor** (`Container { container_type != Normal }`, found by walking `ChildOf` from the sticky entity) via the shared `resolve_cq_unit_px` resolver — the same path sizing, tracks, and edges use. The CQ-ancestor size is read **current-frame from Taffy** (consistent with sticky's self/parent/scroll-container reads, since sub-pass 6a runs after Taffy compute but before `ResolvedLayout` write-back). `Cqi`/`Cqb` resolve on the writing-mode inline/block axes; when there is no CQ ancestor the unit falls back to the viewport like every other `Cq*` site. This frame is the sticky entity's *own* ancestor — distinct from anchor positioning's per-try "anchor target box" frame (§ 3.4). `Length::Fr` (grid-only) and `Length::AnchorSize` (sticky has no anchor box) are meaningless as sticky insets: they resolve to `0` and warn once per (entity, session). See [container-queries-and-writing-modes.md § 1.4](container-queries-and-writing-modes.md). ## 3. Anchor positioning From 434e9afc755da14accec911b1a3132b42c6a5e87 Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 11:13:43 -0700 Subject: [PATCH 7/9] feat(layout): sticky inside sticky tracks the displaced ancestor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A sticky child B of a sticky-displaced ancestor A previously computed its thresholds from A's NATURAL Taffy position, ignoring A's displacement. Fix, both inside sub-pass 6a (sticky_offset): - depth-sort the sticky entities by ChildOf-chain depth ascending, so an outer (shallower) sticky resolves and inserts its PostTaffyPositionOverrides entry BEFORE an inner (deeper) one reads it — same-frame eventual consistency via topological-by-depth ordering, not two-frame. - world_position (only called from sticky_offset) consults the just-written overrides map per ancestor segment: the stored override is already natural_rel + displacement (the correct displaced rel-to-parent), so the inner sticky's threshold geometry sees the displaced outer position. The inner override value is correct by construction (the consult only feeds threshold COMPUTATION, never the e_natural_rel written to the map — no double-counting; pinned by a dedicated test). per-entity memo reset keeps the override-aware walk from returning pre-override values. L4/L5 surfaces untouched. sort_by_cached_key avoids re-walking the chain per comparison. Tests: layout_sticky 21/21 (2 new: inner-tracks-displaced-outer + no-double-count), lib layout 226/226. Spec §2.3 nested-sticky limitation removed; follow-ups.md LANDED. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- crates/buiy_core/src/layout/systems.rs | 104 +++++++++++--- crates/buiy_core/tests/layout_sticky.rs | 128 ++++++++++++++++++ docs/plans/follow-ups.md | 50 +++++-- .../display-and-positioning.md | 2 + 4 files changed, 260 insertions(+), 24 deletions(-) diff --git a/crates/buiy_core/src/layout/systems.rs b/crates/buiy_core/src/layout/systems.rs index 2b98000..df32c94 100644 --- a/crates/buiy_core/src/layout/systems.rs +++ b/crates/buiy_core/src/layout/systems.rs @@ -368,25 +368,54 @@ fn nearest_scroll_container( } } +/// Number of `ChildOf` steps from `entity` up to the root (root = 0). +/// L6 uses this to depth-sort the sticky set so an outer (shallower) +/// sticky resolves and writes its override before an inner (deeper) +/// one reads it via `world_position`. Mirrors `nearest_scroll_container`'s +/// `.parent()` walk. +fn child_of_depth(entity: Entity, parent_chain: &Query<&ChildOf>) -> usize { + let mut depth = 0usize; + let mut current = entity; + while let Ok(co) = parent_chain.get(current) { + depth += 1; + current = co.parent(); + } + depth +} + /// Compute `entity`'s position in `ancestor`'s content-box coordinate /// system by walking `ChildOf` from `entity` up to (but not including) -/// `ancestor`, summing the Taffy `.location` of each step. +/// `ancestor`, summing each step's rel-to-parent location. +/// +/// L6 (sticky-inside-sticky): for each step the location is the +/// just-written `overrides.get(&step)` when present, else the Taffy +/// `.location`. A sticky ancestor's override stores `natural_rel + +/// displacement` (its DISPLACED position in its-parent's frame, the +/// exact per-segment location this walk needs), so an inner sticky +/// sees its ancestor's displacement same-frame. The depth-sorted 6a +/// loop guarantees a shallower (outer) sticky's override is inserted +/// before a deeper (inner) one reads it here. Non-sticky ancestors and +/// undisplaced sticky ancestors have no override entry and contribute +/// their plain Taffy `.location` (correct). /// /// Uses the provided `memo` cache to avoid re-walking shared subpaths /// (mirrors `resolve_writing_mode`'s memoization pattern). Memoization /// key is `(entity, ancestor)` to handle multiple scroll-container -/// frames in the same call. +/// frames in the same call. The 6a loop MUST clear `memo` between +/// sticky entities so a value cached before an outer override existed +/// is never returned stale. /// /// Returns `None` if (a) `entity` has no `LayoutTree` mapping, (b) the /// walk leaves `ancestor`'s subtree without finding `ancestor`, or /// (c) a `tree.tree.layout()` read fails. /// -/// Phase 7 — sub-pass 6a (`sticky_offset`). +/// Phase 7 — sub-pass 6a (`sticky_offset`). L6 — override-consult. fn world_position( entity: Entity, ancestor: Entity, tree: &LayoutTree, parent_chain: &Query<&ChildOf>, + overrides: &HashMap, memo: &mut HashMap<(Entity, Entity), Vec2>, ) -> Option { if entity == ancestor { @@ -397,10 +426,16 @@ fn world_position( } // ChildOf accessor is `.parent()` in Bevy 0.18. let parent = parent_chain.get(entity).ok()?.parent(); - let parent_position = world_position(parent, ancestor, tree, parent_chain, memo)?; + let parent_position = world_position(parent, ancestor, tree, parent_chain, overrides, memo)?; let node_id = tree.by_entity.get(&entity)?; let layout = tree.tree.layout(*node_id).ok()?; - let position = parent_position + Vec2::new(layout.location.x, layout.location.y); + // L6: a same-frame override for this segment IS its displaced + // rel-to-parent location; fall back to Taffy `.location` otherwise. + let seg = overrides + .get(&entity) + .copied() + .unwrap_or_else(|| Vec2::new(layout.location.x, layout.location.y)); + let position = parent_position + seg; memo.insert((entity, ancestor), position); Some(position) } @@ -661,12 +696,32 @@ pub(super) fn sticky_offset( .map(|w| Vec2::new(w.resolution.width(), w.resolution.height())) .unwrap_or(Vec2::ZERO); - for (e, pos, display) in sticky_query.iter() { - // D14 — filter in Rust (no Bevy `Without` exists, - // `Or<>` slots are scarce). D10 — skip `Display::None`. - if !matches!(pos.kind, PositionKind::Sticky) || matches!(display, Display::None) { - continue; - } + // L6 — depth-sort the qualifying sticky entities so an outer + // (shallower) sticky resolves and inserts its override BEFORE a + // deeper (inner) sticky reads that displaced position via + // `world_position`. Same-frame eventual consistency via depth + // ordering (NOT two-frame). Siblings at equal depth in unrelated + // subtrees have no ordering dependency (one can't be the other's + // sticky ancestor), so an unstable sort by depth alone suffices — + // no full topological sort needed. Filter D14 / D10 here so the + // depth walk only runs on real candidates. + let mut sticky_sorted: Vec<(Entity, &Position)> = sticky_query + .iter() + .filter(|(_, pos, display)| { + matches!(pos.kind, PositionKind::Sticky) && !matches!(display, Display::None) + }) + .map(|(e, pos, _)| (e, pos)) + .collect(); + sticky_sorted.sort_by_cached_key(|(e, _)| child_of_depth(*e, &parent_chain)); + + for (e, pos) in sticky_sorted { + // L6 — reset the `world_position` memo per sticky entity. Its + // only purpose is reuse between the `e` and `parent` walks of + // the SAME entity; sharing it across the depth-ordered loop + // would return values cached before an outer override was + // inserted (stale, pre-displacement), making the fix + // order-dependent and flaky. + memo.clear(); // D5 — no scroll container, silent no-op. let Some(scroll_container) = nearest_scroll_container(e, &parent_chain, &overflow_q) else { continue; @@ -705,13 +760,30 @@ pub(super) fn sticky_offset( }; let s_size = Vec2::new(s_layout.size.width, s_layout.size.height); - let Some(e_in_s) = world_position(e, scroll_container, &tree, &parent_chain, &mut memo) - else { + // L6 — pass the override map (shared `&HashMap`) so the + // ancestor walk sees any already-resolved outer sticky's + // displaced position. `e`'s own override is written only at the + // END of this iteration (after both reads), so the immutable + // borrow of `overrides.by_entity` has ended before the `&mut` + // insert below — no borrow conflict. + let Some(e_in_s) = world_position( + e, + scroll_container, + &tree, + &parent_chain, + &overrides.by_entity, + &mut memo, + ) else { continue; }; - let Some(parent_in_s) = - world_position(parent, scroll_container, &tree, &parent_chain, &mut memo) - else { + let Some(parent_in_s) = world_position( + parent, + scroll_container, + &tree, + &parent_chain, + &overrides.by_entity, + &mut memo, + ) else { continue; }; diff --git a/crates/buiy_core/tests/layout_sticky.rs b/crates/buiy_core/tests/layout_sticky.rs index b98b6d6..99760bf 100644 --- a/crates/buiy_core/tests/layout_sticky.rs +++ b/crates/buiy_core/tests/layout_sticky.rs @@ -848,6 +848,134 @@ fn sticky_in_nested_scroll_containers_uses_innermost() { ); } +// ===================================================================== +// Sticky inside sticky (L6) — an inner sticky tracks its sticky +// ancestor's DISPLACED position, not the ancestor's natural Taffy +// position. Sub-pass 6a depth-sorts the sticky set (outer resolves +// before inner) and `world_position` consults the same-frame override +// map when walking the ancestor chain. +// ===================================================================== + +#[test] +fn sticky_inside_sticky_inner_tracks_displaced_outer() { + // Fixture: S(300x500, scroll_y=100) > block(1000) > [50px spacer, A]. + // A is sticky_top(0), 100x200. Inside A: [20px spacer, B], B is + // sticky_top(0), 100x30. + // + // A: natural-in-S = 50, visible_top = 100, threshold = 100. + // desired = max(50, 100) = 100; parent band block [0, 800] → 100. + // displacement_A = (0, 50); override[A].y = 50 + 50 = 100. + // => A's DISPLACED position-in-S = 100. + // + // B (FIXED — uses A's displaced position): + // B natural-in-S = 100 (A displaced) + 20 (spacer in A) = 120. + // threshold = 100; desired = max(120, 100) = 120; + // parent band A [100, 100+200-30=270] → 120. + // displacement_B = 0 → NO override entry for B. + // + // B (BUGGY — uses A's natural position): + // B natural-in-S = 50 (A natural) + 20 = 70. + // threshold = 100; desired = max(70, 100) = 100; + // parent band A-natural [50, 50+200-30=220] → 100. + // displacement_B = 30 → override[B] PRESENT (y = 20 + 30 = 50). + // + // The two paths give DIFFERENT B outcomes: fixed → absent, buggy → + // present. Assert ABSENCE (the fixed expectation). + let mut app = app(); + let scroll = scroll_container(&mut app, 100.0); + let block = content_block(&mut app, scroll, 1000.0); + let _spacer = content_block(&mut app, block, 50.0); + let a = sticky_child(&mut app, block, 100.0, 200.0, sticky_top(0.0)); + let _inner_spacer = content_block(&mut app, a, 20.0); + let b = sticky_child(&mut app, a, 100.0, 30.0, sticky_top(0.0)); + + app.update(); + + let overrides = app.world().resource::(); + // A displaces. + let a_pos = overrides + .by_entity + .get(&a) + .copied() + .unwrap_or_else(|| panic!("outer sticky A should displace; got none")); + assert_eq!( + a_pos.y, 100.0, + "outer sticky A pins to y=100; got {:?}", + a_pos + ); + // B does NOT displace once it tracks A's DISPLACED position. Under + // the buggy natural-A path it would over-displace and get an entry. + assert!( + !overrides.by_entity.contains_key(&b), + "inner sticky B should track A's DISPLACED position (no displacement); \ + a present override (y≈50) means B used A's NATURAL position. got {:?}", + overrides.by_entity.get(&b), + ); +} + +#[test] +fn sticky_inside_sticky_override_value_is_not_double_counted() { + // Fixture where the inner sticky B DOES displace, so we can pin its + // exact stored override value and prove A's displacement is NOT + // baked into it (A's displacement reaches B via normal parent→child + // ResolvedLayout composition, applied to A's own override). + // + // S(300x500, scroll_y=100) > block(1000) > [50px spacer, A]. + // A is sticky_top(0), 100x600. Inside A: [500px spacer, B], B is + // sticky_bottom(0), 100x30. + // + // A: natural-in-S = 50, visible_top = 100, threshold = 100. + // desired = max(50, 100) = 100; parent band block [0, 1000-600=400] + // → 100. displacement_A = 50; override[A].y = 50 + 50 = 100. + // => A's DISPLACED position-in-S = 100; A bottom = 700. + // + // B (FIXED — uses A's displaced position): + // B natural-rel-to-A = 500; B natural-in-S = 100 + 500 = 600. + // sticky_bottom(0): visible_bottom = 100 + 500 = 600. + // desired = min(600, 600 - 0 - 30 = 570) = 570; + // parent band A [100, 100+600-30=670] → 570. + // displacement_B = 570 - 600 = -30. + // override[B].y = B_natural_rel_to_A(500) + (-30) = 470. + // + // B (BUGGY — uses A's natural position): + // B natural-in-S = 50 + 500 = 550. desired = min(550, 570) = 550; + // parent band A-natural [50, 50+600-30=620] → 550. + // displacement_B = 0 → NO override (buggy path would be absent). + // + // Assert override[B].y == 470 exactly. The value is B's natural-rel + // -to-A plus B's OWN displacement only — A's +50 is NOT added in. + let mut app = app(); + let scroll = scroll_container(&mut app, 100.0); + let block = content_block(&mut app, scroll, 1000.0); + let _spacer = content_block(&mut app, block, 50.0); + let a = sticky_child(&mut app, block, 100.0, 600.0, sticky_top(0.0)); + let _inner_spacer = content_block(&mut app, a, 500.0); + let b = sticky_child(&mut app, a, 100.0, 30.0, sticky_bottom(0.0)); + + app.update(); + + let overrides = app.world().resource::(); + let a_pos = overrides + .by_entity + .get(&a) + .copied() + .unwrap_or_else(|| panic!("outer sticky A should displace; got none")); + assert_eq!(a_pos.y, 100.0, "outer A pins to y=100; got {:?}", a_pos); + + let b_pos = overrides + .by_entity + .get(&b) + .copied() + .unwrap_or_else(|| panic!("inner sticky B should displace; got none")); + assert_eq!( + b_pos.y, 470.0, + "B's override = natural-rel-to-A (500) + B's own displacement (-30) = 470; \ + a value inflated by A's +50 (e.g. 520) means A's displacement was \ + double-counted into B's stored override. got {:?}", + b_pos, + ); +} + // ===================================================================== // Display::None skip (D10) // ===================================================================== diff --git a/docs/plans/follow-ups.md b/docs/plans/follow-ups.md index b5fb023..44d842b 100644 --- a/docs/plans/follow-ups.md +++ b/docs/plans/follow-ups.md @@ -419,23 +419,57 @@ band-shorter-than-box case. The 10-arg signature is unchanged. **Spec touchpoint:** `display-and-positioning.md § 2.3`; CSS spec § 6.3 (positioned layout). -## Layout — sticky inside sticky +## Layout — sticky inside sticky — LANDED **Originated:** Phase 7 (documented v1 limitation). +**Status:** **Landed.** A sticky element nested inside a +sticky-displaced ancestor now tracks the ancestor's DISPLACED position, +same-frame. Two coordinated changes inside sub-pass 6a (`sticky_offset`): +(1) the qualifying sticky entities are DEPTH-SORTED by `ChildOf`-chain +depth ascending (`child_of_depth`) so a shallower (outer) sticky resolves +and inserts its `PostTaffyPositionOverrides` entry BEFORE a deeper (inner) +sticky reads it — same-frame eventual consistency via depth ordering, NOT +two-frame; (2) `world_position` takes an `&HashMap` override +map and, per ancestor-walk segment, uses the just-written override +(`natural_rel + displacement` = that entity's displaced rel-to-parent +location) when present, else the Taffy `.location`. The per-call `memo` +is CLEARED at the top of each sticky-entity iteration (its only purpose is +reuse between the `e` and `parent` walks of the SAME entity; sharing it +across the depth-ordered loop would return values cached before an outer +override existed). The override map is passed as `&overrides.by_entity` +(shared borrow); the current entity's own `.insert` happens at the END of +its loop body after both reads, so no borrow conflict and no per-frame +clone. A's displacement reaches B's final render position through the +normal parent→child `ResolvedLayout`/`Transform` composition (applied via +A's own override), NOT by baking it into B's stored value — so it is not +double-counted. Siblings at equal depth in unrelated subtrees have no +ordering dependency, so an unstable sort by depth alone suffices (no full +toposort). Covered by `sticky_inside_sticky_inner_tracks_displaced_outer` +(inner tracks displaced outer → no over-displacement) and +`sticky_inside_sticky_override_value_is_not_double_counted` (pins the +exact inner override value to prove the outer displacement is not +re-added) in `tests/layout_sticky.rs`; the single-level +`sticky_pins_to_top_during_scroll` and nested-scroll +`sticky_in_nested_scroll_containers_uses_innermost` tests stay green +(no regression — single-level depth-sort is trivial, no ancestor override +means `world_position` falls back to Taffy `.location` exactly as before). + **Symptom:** When entity A is sticky-displaced and entity B is a sticky child of A, B's `world_position` walks Taffy positions (un-displaced); B's threshold computation uses A's *natural* position, not displaced. Rare authoring case. -**Implementation sketch:** consult `PostTaffyPositionOverrides` -(just-written by 6a) when walking the ancestor chain in -`world_position`, so inner sticky sees displaced outer. Requires careful -ordering (inner sticky must run after outer; topological pre-pass or -two-frame eventual-consistency are both options). +**Implementation sketch (as landed):** depth-sort the sticky set in +`sticky_offset` (outer resolves first), and consult the just-written +`PostTaffyPositionOverrides` per segment when walking the ancestor chain +in `world_position`, clearing the `world_position` memo between sticky +entities. L4 (the widened `resolve_sticky_inset` Cq/viewport/wmr params, +the `sticky_offset` container_q/wmr_q/primary_window plumbing, the +current-frame `container_index`) and L5 (the dual-clamp form of +`compute_sticky_displacement`) are untouched. -**Spec touchpoint:** `display-and-positioning.md § 2.3` (does not -explicitly address nested-sticky). +**Spec touchpoint:** `display-and-positioning.md § 2.3`. ## Layout — `clear_warned_once_on_exit` lifecycle wire-up diff --git a/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md b/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md index a57cb36..c2bfa1e 100644 --- a/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md +++ b/docs/specs/2026-05-08-buiy-layout-design/display-and-positioning.md @@ -163,6 +163,8 @@ When **both** the top and bottom insets (or both left and right) are set on the Inset units resolve as follows. `Px` is literal; `Percent` resolves against the scroll container's content-box axis size (height for top/bottom, width for left/right). `Length::Cq*` insets resolve against the **sticky entity's own nearest container-query ancestor** (`Container { container_type != Normal }`, found by walking `ChildOf` from the sticky entity) via the shared `resolve_cq_unit_px` resolver — the same path sizing, tracks, and edges use. The CQ-ancestor size is read **current-frame from Taffy** (consistent with sticky's self/parent/scroll-container reads, since sub-pass 6a runs after Taffy compute but before `ResolvedLayout` write-back). `Cqi`/`Cqb` resolve on the writing-mode inline/block axes; when there is no CQ ancestor the unit falls back to the viewport like every other `Cq*` site. This frame is the sticky entity's *own* ancestor — distinct from anchor positioning's per-try "anchor target box" frame (§ 3.4). `Length::Fr` (grid-only) and `Length::AnchorSize` (sticky has no anchor box) are meaningless as sticky insets: they resolve to `0` and warn once per (entity, session). See [container-queries-and-writing-modes.md § 1.4](container-queries-and-writing-modes.md). +A sticky element nested inside a sticky-displaced ancestor tracks the **ancestor's displaced position**, not its natural Taffy position, computed same-frame. Sub-pass 6a depth-sorts the sticky entities (an outer, shallower sticky resolves and writes its override before an inner, deeper one), and the ancestor-chain walk that computes the inner element's thresholds consults the same-frame `PostTaffyPositionOverrides` — substituting an already-resolved ancestor's displaced rel-to-parent position for its natural Taffy `.location`. So the inner sticky's thresholds are computed against the displaced outer box (same-frame eventual consistency via depth ordering, not two-frame). The ancestor's displacement reaches the inner element's final position through the normal parent→child `ResolvedLayout`/`Transform` composition (applied via the ancestor's own override), so it is not double-counted into the inner element's stored offset. + ## 3. Anchor positioning Tier-C. Buiy-owned, post-Taffy. CSS Anchor Positioning Module Level 1. From 90348f14480cbdc366f7609cb08ab0618b402a0a Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 11:27:32 -0700 Subject: [PATCH 8/9] feat(layout): will-change stacking-context trigger former MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit forms_stacking_context now forms an SC when Containment.will_change names an SC-forming property (CSS will-change trigger 5: a property named in will-change forms an SC iff it would at a non-initial value). New WillChangeProperty::forms_stacking_context() names the SC-forming subset once: Transform / Opacity / Filter form one; ZIndex (needs positioning) and ScrollPosition do not. Trigger-5b clause reads the already-present containment param — the 6f sub-pass closure already passes containment_q.get(e).ok(), so the predicate change lights up end-to-end with no signature/call-site change. SC-trigger half only — the render layer-promotion hint stays deferred (no composition-layer/RenderLayers concept exists in render/). Both will-change follow-up entries flip the SC-former half to LANDED; the Phase-8 entry keeps layer-promotion open. Tests: lib layout 229 (+3 unit: subset + predicate-forms + layout-only-doesnt), layout_stacking 16 (+2 e2e: will-change Transform forms SC, ZIndex does not). Spec stacking-and-top-layer §2 trigger 5 + §7 + transforms-and-containment §5.3. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- crates/buiy_core/src/layout/systems.rs | 80 +++++++++++++++++-- crates/buiy_core/src/layout/types.rs | 32 ++++++++ crates/buiy_core/tests/layout_stacking.rs | 65 ++++++++++++++- docs/plans/follow-ups.md | 61 +++++++------- .../stacking-and-top-layer.md | 17 +++- .../transforms-and-containment.md | 8 +- 6 files changed, 222 insertions(+), 41 deletions(-) diff --git a/crates/buiy_core/src/layout/systems.rs b/crates/buiy_core/src/layout/systems.rs index df32c94..dd59d2a 100644 --- a/crates/buiy_core/src/layout/systems.rs +++ b/crates/buiy_core/src/layout/systems.rs @@ -27,7 +27,7 @@ use super::types::{ AnchorErrorKind, AnchorName, AnchorRef, AxisDimension, BreakAfter, BreakBefore, ColumnCount, ColumnFill, ContainFlags, ContainerType, ContentVisibility, GridAreas, Inset, Isolation, LayoutWarnOnceKey, Length, PositionKind, QueryCondition, Sizing, TopLayer, TransformMatrix, - TryCondition, WritingModeKind, ZIndex, + TryCondition, WillChange, WritingModeKind, ZIndex, }; use crate::components::{Node, ResolvedLayout, ResolvedTransform}; use crate::render::components::{Filter, MixBlendMode, Opacity}; @@ -4073,11 +4073,15 @@ pub fn top_layer_paint_rank(t: TopLayer) -> u8 { /// (3) non-identity transform, (4) `Containment.contain ⊇ PAINT/STRICT`, /// (5) the render-side formers (`Opacity < 1`, non-empty `Filter`, /// `MixBlendMode != Normal` — read from the render-owned components, same -/// crate), (6) root. Trigger 5's `will-change` former is still deferred -/// with the rest of `will-change` layer promotion (spec § 7); +/// crate), (5b) `Containment.will_change` naming an SC-forming property +/// (CSS: a property named in `will-change` forms a stacking context iff +/// it would at a non-initial value — the subset is +/// `WillChangeProperty::forms_stacking_context` = Transform/Opacity/Filter; +/// `ZIndex`/`ScrollPosition` are excluded), (6) root. /// `BackdropFilter` is deliberately NOT a trigger — it forms an /// `EffectGroup` but never a stacking context (render component-model.md -/// § 8). +/// § 8). The `will-change` *layer-promotion* hint stays deferred (spec +/// § 7) — only the SC-trigger half is realized here. /// /// Driven by the `stacking_context` sub-pass (6f). #[allow(clippy::too_many_arguments)] @@ -4124,6 +4128,17 @@ pub(super) fn forms_stacking_context( if forms_render_stacking_context(opacity, filter, blend) { return true; } + // Trigger 5b — `will-change` naming an SC-forming property. CSS treats + // a property named in `will-change` as already forming a stacking + // context if it would at a non-initial value; the SC-forming subset is + // named once in `WillChangeProperty::forms_stacking_context` + // (Transform/Opacity/Filter — z-index and scroll-position excluded). + if let Some(c) = containment + && let WillChange::Properties(props) = &c.will_change + && props.iter().any(|p| p.forms_stacking_context()) + { + return true; + } false } @@ -4483,7 +4498,9 @@ mod cq_tests { mod tests { use super::*; use crate::layout::components::{Containment, Position, Stacking}; - use crate::layout::types::{ContainFlags, Isolation, PositionKind, TopLayer, ZIndex}; + use crate::layout::types::{ + ContainFlags, Isolation, PositionKind, TopLayer, WillChange, WillChangeProperty, ZIndex, + }; use crate::layout::components::Display; @@ -4927,6 +4944,59 @@ mod tests { assert!(!forms_via_render(None, None, Some(&MixBlendMode::Normal))); } + // ---- trigger 5b — will-change SC former (spec § 2 trigger 5; + // transforms-and-containment.md § 5.3). A `will-change` naming an + // SC-forming property is treated as already forming a stacking + // context (CSS: a property that would form one at a non-initial value + // forms one when named in will-change). ---- + + /// `forms_stacking_context` with no other trigger but a `Containment` + /// whose `will_change` names the given properties — DRYs the + /// trigger-5b boundary tests below. + fn forms_via_will_change(props: Vec) -> bool { + let c = Containment { + will_change: WillChange::Properties(props), + ..Default::default() + }; + forms_stacking_context( + None, + PositionKind::Static, + false, + Some(&c), + None, + None, + None, + false, + ) + } + + #[test] + fn will_change_transform_forms_context() { + assert!(forms_via_will_change(vec![WillChangeProperty::Transform])); + assert!(forms_via_will_change(vec![WillChangeProperty::Opacity])); + assert!(forms_via_will_change(vec![WillChangeProperty::Filter])); + } + + #[test] + fn will_change_layout_only_does_not_form_context() { + // z-index / scroll-position are not SC-forming on their own. + assert!(!forms_via_will_change(vec![ + WillChangeProperty::ZIndex, + WillChangeProperty::ScrollPosition, + ])); + // `will-change: auto` (the Containment default) forms nothing. + assert!(!forms_stacking_context( + None, + PositionKind::Static, + false, + Some(&Containment::default()), + None, + None, + None, + false, + )); + } + #[test] fn top_layer_activation_default_is_empty() { assert!(TopLayerActivation::default().order.is_empty()); diff --git a/crates/buiy_core/src/layout/types.rs b/crates/buiy_core/src/layout/types.rs index a5ea332..4ad0b39 100644 --- a/crates/buiy_core/src/layout/types.rs +++ b/crates/buiy_core/src/layout/types.rs @@ -1242,6 +1242,25 @@ pub enum WillChangeProperty { ScrollPosition, } +impl WillChangeProperty { + /// The SC-forming subset of `will-change` properties. + /// + /// CSS rule: a property named in `will-change` forms a stacking + /// context iff that property *would* form one at a non-initial value. + /// So `Transform` (trigger 3), `Opacity` (`< 1`), and `Filter` + /// (`!= none`) qualify. `ZIndex` does NOT — `z-index` only forms an + /// SC on a positioned element, so `will-change: z-index` alone never + /// creates one. `ScrollPosition` is not SC-forming either. Extend the + /// match here (the single source of truth) when new SC-forming + /// variants are added. + /// + /// Spec: docs/specs/2026-05-08-buiy-layout-design/stacking-and-top-layer.md § 2 (trigger 5); + /// transforms-and-containment.md § 5.3. + pub(crate) fn forms_stacking_context(self) -> bool { + matches!(self, Self::Transform | Self::Opacity | Self::Filter) + } +} + // ============================================================ // Phase 9 — stacking value types (stacking-and-top-layer.md § 1) // ============================================================ @@ -1366,6 +1385,19 @@ mod tests { assert_eq!(WillChange::default(), WillChange::Auto); } + #[test] + fn will_change_property_sc_forming_subset() { + // SC-forming: the properties that would form a stacking context at + // a non-initial value (transform / opacity<1 / filter!=none). + assert!(WillChangeProperty::Transform.forms_stacking_context()); + assert!(WillChangeProperty::Opacity.forms_stacking_context()); + assert!(WillChangeProperty::Filter.forms_stacking_context()); + // Not SC-forming: z-index needs positioning to form an SC, and + // scroll-position never forms one. + assert!(!WillChangeProperty::ZIndex.forms_stacking_context()); + assert!(!WillChangeProperty::ScrollPosition.forms_stacking_context()); + } + #[test] fn edges_helpers_produce_uniform_and_axis_values() { let all = Edges::all(8.0); diff --git a/crates/buiy_core/tests/layout_stacking.rs b/crates/buiy_core/tests/layout_stacking.rs index 34945ea..218ddbc 100644 --- a/crates/buiy_core/tests/layout_stacking.rs +++ b/crates/buiy_core/tests/layout_stacking.rs @@ -5,8 +5,8 @@ use bevy::prelude::*; use buiy_core::components::StackingContext; use buiy_core::layout::{ - ContainFlags, Isolation, LayoutPlugin, Length, PositionKind, Style, TopLayer, - TopLayerActivation, ZIndex, + ContainFlags, Containment, Isolation, LayoutPlugin, Length, PositionKind, Style, TopLayer, + TopLayerActivation, WillChange, WillChangeProperty, ZIndex, }; use buiy_core::render::components::{Filter, FilterFn, MixBlendMode, Opacity}; use buiy_core::{CorePlugin, Node}; @@ -107,6 +107,67 @@ fn paint_containment_forms_stacking_context_end_to_end() { ); } +#[test] +fn will_change_forms_stacking_context_end_to_end() { + // Trigger 5b via the real `containment_q.get(e)` path in 6f: a + // `will-change` naming an SC-forming property (Transform) forms a + // stacking context even though no transform is actually applied. + let mut app = app(); + let child = app + .world_mut() + .spawn(( + Node, + Style::default().containment(Containment { + will_change: WillChange::Properties(vec![WillChangeProperty::Transform]), + ..default() + }), + )) + .id(); + let root = app + .world_mut() + .spawn((Node, Style::default())) + .add_child(child) + .id(); + app.update(); + assert!( + app.world().get::(child).is_some(), + "will-change: transform forms a stacking context (trigger 5b)" + ); + let root_sc = app.world().get::(root).unwrap(); + assert!( + root_sc.painters_z.contains(&child), + "the will-change child is an atomic painter in the root context" + ); +} + +#[test] +fn will_change_layout_only_forms_no_stacking_context() { + // Trigger 5b negative: `will-change: z-index` is layout-only and not + // SC-forming (z-index needs positioning to create one), so no + // stacking context is formed. + let mut app = app(); + let child = app + .world_mut() + .spawn(( + Node, + Style::default().containment(Containment { + will_change: WillChange::Properties(vec![WillChangeProperty::ZIndex]), + ..default() + }), + )) + .id(); + let _root = app + .world_mut() + .spawn((Node, Style::default())) + .add_child(child) + .id(); + app.update(); + assert!( + app.world().get::(child).is_none(), + "will-change: z-index is layout-only and forms no stacking context" + ); +} + #[test] fn parentless_top_layer_does_not_self_reference() { // Regression (review finding B1): a top-layer entity that is itself a diff --git a/docs/plans/follow-ups.md b/docs/plans/follow-ups.md index 44d842b..66ec469 100644 --- a/docs/plans/follow-ups.md +++ b/docs/plans/follow-ups.md @@ -569,19 +569,26 @@ nodes are kept alive. Hidden never warns (fully implemented). Mirrored in **Spec touchpoint:** `transforms-and-containment.md § 5.2`. -## Layout / render — `will-change` layer promotion + SC trigger +## Layout / render — `will-change` layer promotion + SC trigger — SC TRIGGER LANDED; layer promotion DEFERRED **Originated:** Phase 8 (D7 — tier-E, stored-only). -**Symptom:** `WillChange` is stored on `Containment` but no layer -promotion or stacking-context trigger behavior is produced. +**Status — SC trigger (landed):** the stacking-context half is **landed**. +`forms_stacking_context` (layout/systems.rs) now reads `Containment.will_change` +and forms a `StackingContext` when the list names an SC-forming property. The +SC-forming subset is encoded once in `WillChangeProperty::forms_stacking_context` +(Transform / Opacity / Filter; `ZIndex` / `ScrollPosition` excluded — `will-change: +z-index` does not form an SC, matching CSS). Unit tests sit beside the other +trigger tests (layout/systems.rs); end-to-end coverage in tests/layout_stacking.rs. +See the "Phase 9 `will-change` stacking-context former" entry below. -**Implementation sketch:** honor `WillChange::Properties` as a render -layer-promotion hint and a stacking-context trigger when the list mentions -an SC-forming property (`WillChangeProperty::Transform` etc.) — coordinates -with Phase 9 stacking. +**Status — layer promotion (deferred):** the **render layer-promotion hint** +remains deferred. There is no composition-layer / `RenderLayers` concept in +render/ to hang a promotion hint on, so this half is not yet actionable — it +stays open until such a mechanism exists. -**Spec touchpoint:** `transforms-and-containment.md § 5.3`. +**Spec touchpoint:** `transforms-and-containment.md § 5.3`; +`stacking-and-top-layer.md § 2` trigger 5, § 7. ## Render — `UiTransform` paint + `Containment` PAINT clip rect + perspective / backface @@ -737,7 +744,7 @@ the composed `ResolvedTransform` (trigger 3). It implements the spec § 2 SC trigger union (positioned + z-index, isolation, transform, paint/strict containment, root), the § 2.1 five-tier z-index paint-order sort, and the § 4 top-layer escape. The render-side trigger-5 formers have since landed -(next entry); the will-change SC trigger remains deferred (separate +(next entry); the will-change SC trigger has since landed too (separate follow-up below). **Spec touchpoint:** `transforms-and-containment.md § 3`, § 6; @@ -757,33 +764,33 @@ forms a `StackingContext`. The clause delegates to `render::effect::forms_render_stacking_context`, which derives from the canonical effect-group former predicate (`effect_reason_for`) — ONE source of truth for the shared terms, so the SC trigger and the group former can -never drift apart. `will-change` stays deferred (separate follow-up below); -`BackdropFilter` deliberately forms an `EffectGroup` but never an SC +never drift apart. The `will-change` SC trigger has since landed (separate +follow-up below); `BackdropFilter` deliberately forms an `EffectGroup` but never an SC (render component-model.md § 8). Unit tests sit beside the other trigger tests (layout/systems.rs); end-to-end coverage in tests/layout_stacking.rs. **Spec touchpoint:** `stacking-and-top-layer.md § 2` trigger 5, § 7. -## Layout — Phase 9 `will-change` stacking-context former +## Layout — Phase 9 `will-change` stacking-context former — LANDED **Originated:** Phase 9 (D1) — coordinates with the Phase-8 "will-change layer promotion + SC trigger" follow-up (above). -**Symptom:** a `WillChange` value naming an SC-forming property (e.g. -`WillChangeProperty::Transform`, `Opacity`) should form a stacking context, -but sub-pass 6f does not treat `will-change` as a trigger. `WillChange` is -Phase-8 tier-E, stored-only with no behavior. - -**Cause:** Phase 8 stores `WillChange` on `Containment` but ships no behavior -(D7); Phase 9 deliberately did not wire it as an SC trigger to keep the -deferral consistent. The two concerns (render layer promotion + SC trigger) -are the same underlying feature and should land together. - -**Implementation sketch:** when honoring `WillChange`, extend -`forms_stacking_context` to return `true` when the `Containment.will_change` -list names an SC-forming property, in the same change that adds the render -layer-promotion hint. Cross-links the existing Phase-8 "will-change layer -promotion + SC trigger" follow-up. +**Status:** **Landed.** `forms_stacking_context` (layout/systems.rs) gained a +trigger-5b clause: when `Containment.will_change` is `WillChange::Properties` +and names an SC-forming property, the entity forms a `StackingContext`. The +SC-forming subset is encoded once as `WillChangeProperty::forms_stacking_context` +(types.rs) = Transform / Opacity / Filter; `ZIndex` and `ScrollPosition` are +excluded (CSS: `will-change: z-index` does not create an SC — z-index needs +positioning). No signature change was needed: the 6f `forms` closure already +passed `containment_q.get(e).ok()`, so the unit-level predicate and the +end-to-end 6f path lit up together. Unit tests sit beside the other trigger +tests (layout/systems.rs) and the subset helper (types.rs); end-to-end +coverage (positive + layout-only negative) in tests/layout_stacking.rs. + +Only the SC-trigger half landed. The `will-change` **render layer-promotion +hint** stays deferred (see the combined Phase-8 entry above) — there is no +composition-layer / `RenderLayers` concept in render/ to honor it. **Spec touchpoint:** `transforms-and-containment.md § 5.3`; `stacking-and-top-layer.md § 2` trigger 5, § 7. diff --git a/docs/specs/2026-05-08-buiy-layout-design/stacking-and-top-layer.md b/docs/specs/2026-05-08-buiy-layout-design/stacking-and-top-layer.md index b4d6bcb..821ad7e 100644 --- a/docs/specs/2026-05-08-buiy-layout-design/stacking-and-top-layer.md +++ b/docs/specs/2026-05-08-buiy-layout-design/stacking-and-top-layer.md @@ -50,7 +50,7 @@ An entity forms a *stacking context* — a sub-tree painted as one unit, ordered 2. `Stacking::isolation = Isolate`. 3. `Transform` is non-identity. (Detailed in [transforms-and-containment.md § 3](transforms-and-containment.md#3-stacking-context-formation).) 4. `Containment::contain` includes `Paint` or `Strict`. (Detailed in [transforms-and-containment.md § 5](transforms-and-containment.md#5-containment).) -5. Render-side properties form one too: `opacity < 1.0`, `filter != none`, `mix_blend_mode != normal`, `will_change` mentions an SC-forming property (will-change portion: tier-E, deferred — see [transforms-and-containment.md § 5.3](transforms-and-containment.md#53-will-change)). These live on render-side components but are *checked* during this spec's stacking-context detection so layout can hand a correct list to render. *Status:* the `opacity` / `filter` / `mix_blend_mode` formers are **realized** in 6f (§ 7) via one predicate shared with the render effect-group former (`render::effect`); `backdrop-filter` is deliberately NOT an SC former — it forms only an `EffectGroup` (render component-model.md § 8). +5. Render-side properties form one too: `opacity < 1.0`, `filter != none`, `mix_blend_mode != normal`, `will_change` mentions an SC-forming property (see [transforms-and-containment.md § 5.3](transforms-and-containment.md#53-will-change)). These live on render-side components but are *checked* during this spec's stacking-context detection so layout can hand a correct list to render. *Status:* the `opacity` / `filter` / `mix_blend_mode` formers are **realized** in 6f (§ 7) via one predicate shared with the render effect-group former (`render::effect`); the `will_change` SC former is **realized** too — `forms_stacking_context` reads `Containment.will_change` and forms an SC when it names an SC-forming property (the `WillChangeProperty::forms_stacking_context` subset = Transform/Opacity/Filter; `ZIndex`/`ScrollPosition` excluded, matching CSS). `backdrop-filter` is deliberately NOT an SC former — it forms only an `EffectGroup` (render component-model.md § 8). 6. The root entity always forms one. The rule set is deliberately union — any single trigger is sufficient. The CSS spec is the source of truth; the foundation visuals.md § 3.2 enumeration anchors the trigger list. @@ -183,6 +183,14 @@ each is tracked in [`../../plans/follow-ups.md`](../../plans/follow-ups.md). `effect_reason_for`) so the SC trigger and the `EffectGroup` former share one source of truth — the effect-compositor's group-contiguity invariant (`render/buckets.rs`) holds by construction. +- **Trigger 5 — `will-change` SC former** (landed post-Phase-9 follow-up): + `forms_stacking_context` reads `Containment.will_change` and forms an SC when + it names an SC-forming property. The SC-forming subset is named once in + `WillChangeProperty::forms_stacking_context` (Transform/Opacity/Filter); + `ZIndex`/`ScrollPosition` are excluded (CSS: `will-change: z-index` does not + form an SC — z-index needs positioning). The `will-change` **layer-promotion + hint** stays deferred (no composition-layer concept exists in render/ yet) — + see transforms-and-containment.md § 5.3. - `StackingContext.painters_z` paint-order sort (§ 2.1, all five tiers; floats always empty). - `z_index` sibling ordering within a context (§ 3). @@ -192,9 +200,10 @@ each is tracked in [`../../plans/follow-ups.md`](../../plans/follow-ups.md). **Deferred (target stands; not in Phase 9):** -- **Trigger 5 — `will-change` SC former.** `WillChange` is stored by Phase 8 - (tier-E, no behavior); its SC-forming behavior is deferred with the rest of - `will-change` layer promotion. +- **`will-change` layer-promotion hint.** The SC-trigger half of trigger 5's + `will-change` former is now realized (see above); the *layer-promotion* hint + (`will-change` as a compositing/render-layer optimization) stays deferred — + there is no composition-layer / `RenderLayers` concept in render/ to honor it. - **§ 4.4 per-window scope.** `buiy_core` has a single global `LayoutTree` and uses the primary window only (no per-window layout segregation). Phase 9 ships one global top layer; per-window top layers depend on diff --git a/docs/specs/2026-05-08-buiy-layout-design/transforms-and-containment.md b/docs/specs/2026-05-08-buiy-layout-design/transforms-and-containment.md index 5bad283..2b2d622 100644 --- a/docs/specs/2026-05-08-buiy-layout-design/transforms-and-containment.md +++ b/docs/specs/2026-05-08-buiy-layout-design/transforms-and-containment.md @@ -228,9 +228,9 @@ When the Auto skip fires, the entity's own Taffy node receives the `ContainIntri ### 5.3 `will-change` -`will-change` is foundation tier-E ([visuals.md § 3.2](../2026-05-07-buiy-foundation/visuals.md#32-layout)) — v1 ships the `WillChange` API surface for forward compatibility, but the layer-promotion hint and the SC-forming behavior below are deferred until user demand. Prioritization waits on user demand. +`will-change` is foundation tier-E ([visuals.md § 3.2](../2026-05-07-buiy-foundation/visuals.md#32-layout)) — v1 ships the `WillChange` API surface for forward compatibility. The **SC-forming behavior is realized** (layout reads `will-change` as a stacking-context trigger, below); the **layer-promotion hint remains deferred** until a composition-layer concept exists in render/. -Hint to the optimizer. Render uses it to promote the entity to its own composition layer. Layout uses it as a stacking-context trigger when its property list mentions an SC-forming property (e.g. `WillChangeProperty::Transform`). +Hint to the optimizer. Layout uses it as a stacking-context trigger when its property list mentions an SC-forming property (e.g. `WillChangeProperty::Transform`): `forms_stacking_context` forms an SC when `Containment.will_change` names a property in the SC-forming subset. (The render-side use — promoting the entity to its own composition layer — is the deferred half; there is no `RenderLayers`/composition-layer mechanism yet.) ```rust pub enum WillChangeProperty { @@ -238,6 +238,8 @@ pub enum WillChangeProperty { } ``` +The SC-forming subset (CSS: a property named in `will-change` forms an SC iff it would at a non-initial value) is `Transform` / `Opacity` / `Filter`, encoded once in `WillChangeProperty::forms_stacking_context`. `ZIndex` and `ScrollPosition` are excluded (`will-change: z-index` does not form an SC — z-index needs positioning). + Authors should use sparingly — `will-change` consumes memory by promoting layers eagerly. ## 6. Stacking-context formation triggers (full list) @@ -248,7 +250,7 @@ Consolidating from this file and [stacking-and-top-layer.md § 2](stacking-and-t 2. `Stacking::isolation = Isolate`. 3. `UiTransform` non-identity (this file). 4. `Containment::contain` includes `ContainFlags::PAINT` or `ContainFlags::STRICT` (this file). -5. `Containment::will_change` lists an SC-forming property (this file) — tier-E, deferred (see [§ 5.3](#53-will-change)). +5. `Containment::will_change` lists an SC-forming property (this file) — realized (see [§ 5.3](#53-will-change); subset = Transform/Opacity/Filter). Only the layer-promotion hint stays deferred. 6. `TopLayer != None` (handled separately — top layer escapes the stacking system entirely). 7. Render-side triggers (`opacity < 1`, `filter != none`, `mix_blend_mode != normal`) — checked here for completeness; the components live in render-spec. 8. The root entity always forms a stacking context (matches [stacking-and-top-layer.md § 2](stacking-and-top-layer.md#2-stacking-context-formation) trigger 6). From 9681bafb7835b778980c4f0a8f7ecc8acc80fd82 Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 18 Jun 2026 12:36:47 -0700 Subject: [PATCH 9/9] feat(layout): percent translate units in compose_transform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit translateX(p%) / translateY(p%) now resolve against the entity's OWN current-frame border box (x% of width, y% of height) per CSS Transforms, instead of contributing 0.0. The single 0.0 gap was length_px, the resolver for both the Translate longhand (T factor of M=T·R·S·M_transform) and TransformMatrix::Translate (M_transform); replaced with axis-aware translate_length_px(l, axis_px): Px=>p, Percent=>p*0.01*axis_px, else 0.0. Box-size source is the current-frame Taffy tree (tree.by_entity -> tree.tree.layout), NOT ResolvedLayout — sub-pass 6e transform_composition runs in PostTaffyOverrides BEFORE WriteResolvedLayout, so ResolvedLayout.size is last-frame there; mirrors anchor_resolution's same-frame Taffy read. compose_transform / transform_matrix_to_mat4 gained a box_size param (pure fns; the buiy_verify Tier-3 transform_roundtrips invariant + unit tests pass Vec2::ZERO since Px translates are box-independent). Cq*/Fr translate stays a documented 0.0 residual (needs the nearest CQ-ancestor frame, like sticky L4) behind a warn-once; z-percent is invalid -> 0.0. Tests: layout_transforms +4 (percent x against width, percent y against height, mixed percent+px, Cq* residual), Px-only unchanged. Spec transforms-and- containment §1/§1.1; follow-ups.md PERCENT LANDED (Cq* residual noted). Final layout-track slice. Whole track (L1-L8) green under the full workspace gate (fmt + clippy --workspace + doc --workspace + cargo test --workspace 175/0). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DHcf8nQ9PTT3m5E7u3Q6XV --- crates/buiy_core/src/layout/style.rs | 14 ++- crates/buiy_core/src/layout/systems.rs | 114 ++++++++++++++---- crates/buiy_core/tests/layout_transforms.rs | 103 +++++++++++++++- .../buiy_verify/src/invariant/predicates.rs | 19 ++- docs/plans/follow-ups.md | 32 ++++- .../transforms-and-containment.md | 4 +- 6 files changed, 246 insertions(+), 40 deletions(-) diff --git a/crates/buiy_core/src/layout/style.rs b/crates/buiy_core/src/layout/style.rs index 3eeef9c..2091b79 100644 --- a/crates/buiy_core/src/layout/style.rs +++ b/crates/buiy_core/src/layout/style.rs @@ -487,13 +487,19 @@ impl Style { self } - /// Ergonomic setter — 2D translate in logical pixels (z = 0). - pub fn translate_px(mut self, x: f32, y: f32) -> Self { - self.ui_transform.matrix = - TransformMatrix::Translate(Length::px(x), Length::px(y), Length::ZERO); + /// Ergonomic setter — 2D translate from arbitrary [`Length`]s (z = 0). + /// `Percent` resolves against the entity's own border box at compose time + /// (x = width, y = height) per CSS Transforms. + pub fn translate(mut self, x: Length, y: Length) -> Self { + self.ui_transform.matrix = TransformMatrix::Translate(x, y, Length::ZERO); self } + /// Ergonomic setter — 2D translate in logical pixels (z = 0). + pub fn translate_px(self, x: f32, y: f32) -> Self { + self.translate(Length::px(x), Length::px(y)) + } + /// Ergonomic setter — rotate about the z axis (radians). pub fn rotate_z(mut self, radians: f32) -> Self { self.ui_transform.matrix = TransformMatrix::Rotate(Quat::from_rotation_z(radians)); diff --git a/crates/buiy_core/src/layout/systems.rs b/crates/buiy_core/src/layout/systems.rs index dd59d2a..2a0754d 100644 --- a/crates/buiy_core/src/layout/systems.rs +++ b/crates/buiy_core/src/layout/systems.rs @@ -3951,19 +3951,22 @@ pub(super) fn cq_flip_rerun( /// `Compose([A, B, …])` folds to the matrix product `A · B · …` /// (outermost first; rightmost transforms a child point first). /// -/// `Length`s in `Translate` resolve as px today (percent/cq transform -/// translates resolve against the entity's own box — deferred to the -/// render/animation phase; px is the only meaningful unit at compose -/// time for Phase 8). Non-px `Length` variants resolve to their px -/// magnitude via `Length::Px` only; other variants contribute 0.0. +/// `Length`s in `Translate` resolve via [`translate_length_px`]: `Px` as-is, +/// `Percent` against the entity's own current-frame border `box_size` +/// (x = width, y = height; z-percent is invalid → 0), `Cq*` as a 0.0 residual +/// (warned once), `Fr`/`AnchorSize` as 0.0. The other arms ignore `box_size`, +/// but `Compose` threads it through so a nested `Translate` percent still +/// resolves against the same own box. /// /// Spec: docs/specs/2026-05-08-buiy-layout-design/transforms-and-containment.md § 1. -fn transform_matrix_to_mat4(m: &TransformMatrix) -> Mat4 { +fn transform_matrix_to_mat4(m: &TransformMatrix, box_size: Vec2) -> Mat4 { match m { TransformMatrix::None => Mat4::IDENTITY, - TransformMatrix::Translate(x, y, z) => { - Mat4::from_translation(Vec3::new(length_px(x), length_px(y), length_px(z))) - } + TransformMatrix::Translate(x, y, z) => Mat4::from_translation(Vec3::new( + translate_length_px(x, box_size.x), + translate_length_px(y, box_size.y), + translate_length_px(z, 0.0), + )), TransformMatrix::Rotate(q) => Mat4::from_quat(*q), TransformMatrix::Scale(x, y, z) => Mat4::from_scale(Vec3::new(*x, *y, *z)), TransformMatrix::Skew(ax, ay) => { @@ -3975,19 +3978,53 @@ fn transform_matrix_to_mat4(m: &TransformMatrix) -> Mat4 { } TransformMatrix::Matrix(mat) => *mat, TransformMatrix::Compose(list) => list.iter().fold(Mat4::IDENTITY, |acc, item| { - acc * transform_matrix_to_mat4(item) + acc * transform_matrix_to_mat4(item, box_size) }), } } -/// Resolve a `Length` to px for transform translation. Only `Px` is -/// meaningful at compose time in Phase 8; other units (percent / -/// cq) resolve against the entity's own box and are deferred to the -/// render/animation phase — they contribute 0.0 here. -fn length_px(l: &Length) -> f32 { +/// Warn-once for a `Cq*` transform-translate term. `Cq*` translate needs +/// the entity's nearest CQ-ancestor frame (sticky L4 territory) which sub-pass +/// 6e does not gather, so it stays a documented residual that contributes 0.0. +/// Warn once rather than per-frame so authors learn it is unresolved without +/// log spam (mirrors `translate::warn_once_cq_no_ancestor`). +fn warn_once_cq_translate_residual() { + static WARNED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); + if !WARNED.swap(true, std::sync::atomic::Ordering::Relaxed) { + warn!( + "buiy: container unit (cqw/cqh/cqi/cqb/cqmin/cqmax) used in a \ + transform translate — unresolved (needs the nearest CQ-ancestor \ + frame, a residual deferral) and contributes 0.0 (warned once)" + ); + } +} + +/// Resolve a `Length` to px for transform translation along ONE axis. +/// +/// `Px(p)` resolves to `p`. `Percent(p)` resolves against the entity's own +/// current-frame border box on the relevant axis per CSS Transforms — +/// `translateX(p%) = p% of width`, `translateY(p%) = p% of height` — so the +/// caller passes the matching `axis_px` (`box.x` for x, `box.y` for y). The +/// z axis passes `axis_px = 0.0`: CSS makes `translateZ` percentages invalid, +/// so a z-percent resolves to 0.0. +/// +/// `Cq*` translate is a RESIDUAL deferral (needs the nearest CQ-ancestor +/// frame, like sticky L4) — it contributes 0.0 and fires a one-shot warn. +/// `Fr` / `AnchorSize` are meaningless on a transform and likewise yield 0.0. +fn translate_length_px(l: &Length, axis_px: f32) -> f32 { match l { Length::Px(p) => *p, - _ => 0.0, + Length::Percent(p) => p * 0.01 * axis_px, + Length::Cqw(_) + | Length::Cqh(_) + | Length::Cqi(_) + | Length::Cqb(_) + | Length::Cqmin(_) + | Length::Cqmax(_) => { + warn_once_cq_translate_residual(); + 0.0 + } + Length::Fr(_) | Length::AnchorSize(_) => 0.0, } } @@ -4019,17 +4056,27 @@ pub(super) fn multicol_length_px(l: Option, fallback: f32) -> f32 { /// `translate∘-translate ≈ I`, `rotate(2π) ≈ I`, `scale(k)` checks assert on /// THIS composed matrix, never a re-implementation), hence `pub`. /// +/// `box_size` is the entity's own current-frame border box — the percent +/// basis for translate terms (x = width, y = height; z-percent → 0) per CSS +/// Transforms. The caller (sub-pass 6e) reads it from the current-frame Taffy +/// tree; unit tests pass an explicit box (`Vec2::ZERO` when no percent is in +/// play, so px composition is unchanged). `Cq*` translate is a 0.0 residual +/// (needs the nearest CQ-ancestor frame, like sticky L4). +/// /// Spec: docs/specs/2026-05-08-buiy-layout-design/transforms-and-containment.md § 1, § 1.1. pub fn compose_transform( ui: &UiTransform, t: Option<&Translate>, r: Option<&Rotate>, s: Option<&Scale>, + box_size: Vec2, ) -> Mat4 { let t_mat = match t { - Some(Translate(x, y, z)) => { - Mat4::from_translation(Vec3::new(length_px(x), length_px(y), length_px(z))) - } + Some(Translate(x, y, z)) => Mat4::from_translation(Vec3::new( + translate_length_px(x, box_size.x), + translate_length_px(y, box_size.y), + translate_length_px(z, 0.0), + )), None => Mat4::IDENTITY, }; let r_mat = match r { @@ -4040,7 +4087,7 @@ pub fn compose_transform( Some(Scale(x, y, z)) => Mat4::from_scale(Vec3::new(*x, *y, *z)), None => Mat4::IDENTITY, }; - let m_transform = transform_matrix_to_mat4(&ui.matrix); + let m_transform = transform_matrix_to_mat4(&ui.matrix, box_size); t_mat * r_mat * s_mat * m_transform } @@ -4197,6 +4244,7 @@ pub(super) fn paint_key(stacking: Option<&Stacking>, position_kind: PositionKind #[allow(clippy::type_complexity)] pub(super) fn transform_composition( mut commands: Commands, + tree: NonSend, query: Query< ( Entity, @@ -4214,7 +4262,21 @@ pub(super) fn transform_composition( if matches!(display, Display::None) { continue; } - let m = compose_transform(ui, t, r, s); + // Percent translate resolves against the entity's OWN current-frame + // border box. 6e runs in PostTaffyOverrides — BEFORE + // WriteResolvedLayout — so `ResolvedLayout.size` is last-frame here; + // read the current-frame box straight from the Taffy tree, mirroring + // `anchor_resolution` (the preceding 6d sub-pass). An entity with no + // Taffy node (just-spawned) falls back to ZERO → percent yields 0 this + // frame, self-healing next frame (anchor_resolution's precedent). + let box_size = tree + .by_entity + .get(&e) + .copied() + .and_then(|id| tree.tree.layout(id).ok()) + .map(|l| Vec2::new(l.size.width, l.size.height)) + .unwrap_or(Vec2::ZERO); + let m = compose_transform(ui, t, r, s, box_size); if m == Mat4::IDENTITY { // Identity → no ResolvedTransform; remove a stale one. if existing.is_some() { @@ -5135,7 +5197,7 @@ mod tests { #[test] fn compose_identity_is_identity() { let ui = UiTransform::default(); - let m = compose_transform(&ui, None, None, None); + let m = compose_transform(&ui, None, None, None, Vec2::ZERO); assert_eq!(m, Mat4::IDENTITY); } @@ -5145,7 +5207,7 @@ mod tests { matrix: TransformMatrix::Translate(Length::px(10.0), Length::px(20.0), Length::ZERO), ..Default::default() }; - let m = compose_transform(&ui, None, None, None); + let m = compose_transform(&ui, None, None, None, Vec2::ZERO); assert_eq!(m, Mat4::from_translation(Vec3::new(10.0, 20.0, 0.0))); } @@ -5155,7 +5217,7 @@ mod tests { matrix: TransformMatrix::Scale(2.0, 3.0, 1.0), ..Default::default() }; - let m = compose_transform(&ui, None, None, None); + let m = compose_transform(&ui, None, None, None, Vec2::ZERO); assert_eq!(m, Mat4::from_scale(Vec3::new(2.0, 3.0, 1.0))); } @@ -5172,7 +5234,7 @@ mod tests { }; let t = Translate(Length::px(10.0), Length::ZERO, Length::ZERO); let s = Scale(2.0, 2.0, 1.0); - let m = compose_transform(&ui, Some(&t), None, Some(&s)); + let m = compose_transform(&ui, Some(&t), None, Some(&s), Vec2::ZERO); let expected = Mat4::from_translation(Vec3::new(10.0, 0.0, 0.0)) * Mat4::from_scale(Vec3::new(2.0, 2.0, 1.0)) * Mat4::from_quat(Quat::from_rotation_z(std::f32::consts::FRAC_PI_2)); @@ -5188,7 +5250,7 @@ mod tests { matrix: TransformMatrix::Compose(vec![a, b]), ..Default::default() }; - let m = compose_transform(&ui, None, None, None); + let m = compose_transform(&ui, None, None, None, Vec2::ZERO); let expected = Mat4::from_translation(Vec3::new(5.0, 0.0, 0.0)) * Mat4::from_scale(Vec3::new(2.0, 1.0, 1.0)); assert_eq!(m, expected); diff --git a/crates/buiy_core/tests/layout_transforms.rs b/crates/buiy_core/tests/layout_transforms.rs index 673b17f..8e8fa1b 100644 --- a/crates/buiy_core/tests/layout_transforms.rs +++ b/crates/buiy_core/tests/layout_transforms.rs @@ -5,9 +5,27 @@ use bevy::prelude::*; use buiy_core::{ CorePlugin, Node, ResolvedLayout, ResolvedTransform, - layout::{Display, FlexAxis, LayoutPlugin, Length, Sizing, Style}, + layout::{ + Display, FlexAxis, LayoutPlugin, Length, Sizing, Style, TransformMatrix, UiTransform, + }, }; +/// Spawn a single sized `Node` with the given `style` and return the composed +/// `ResolvedTransform.matrix` after one update (or identity if none was +/// inserted — sub-pass 6e omits `ResolvedTransform` for an identity matrix). +fn resolved_matrix(width: Length, height: Length, style: Style) -> Mat4 { + let mut app = app(); + let mut s = style; + s.box_model.width = Sizing::Length(width); + s.box_model.height = Sizing::Length(height); + let e = app.world_mut().spawn((Node, s)).id(); + app.update(); + app.world() + .get::(e) + .map(|rt| rt.matrix) + .unwrap_or(Mat4::IDENTITY) +} + fn app() -> App { let mut app = App::new(); app.add_plugins(MinimalPlugins); @@ -117,6 +135,89 @@ fn transform_does_not_change_sibling_positions() { } } +#[test] +fn translate_percent_x_resolves_against_own_width() { + // translateX(50%) on a 100px-wide box → 50px x (CSS: x% of border-box width). + let m = resolved_matrix( + Length::px(100.0), + Length::px(40.0), + Style::default().translate(Length::percent(50.0), Length::px(0.0)), + ); + assert_eq!(m, Mat4::from_translation(Vec3::new(50.0, 0.0, 0.0))); +} + +#[test] +fn translate_percent_y_resolves_against_own_height() { + // translateY(25%) on an 80px-tall box → 20px y (y% is of HEIGHT, not width). + let m = resolved_matrix( + Length::px(40.0), + Length::px(80.0), + Style::default().translate(Length::px(0.0), Length::percent(25.0)), + ); + assert_eq!(m, Mat4::from_translation(Vec3::new(0.0, 20.0, 0.0))); +} + +#[test] +fn translate_mixed_percent_and_px() { + // 200x100 box; translate (10% , 15px) → x = 0.1*200 = 20, y = 15. + // (`p * 0.01 * axis` carries a tiny float error, so compare within EPS.) + fn assert_translation(m: Mat4, expected: Vec3) { + let got = m.w_axis.truncate(); + assert!( + got.abs_diff_eq(expected, 1e-4), + "translation {got:?} != expected {expected:?}", + ); + // The linear part stays identity (no rotation/scale leaked in). + assert_eq!(m.x_axis, Vec4::new(1.0, 0.0, 0.0, 0.0)); + assert_eq!(m.y_axis, Vec4::new(0.0, 1.0, 0.0, 0.0)); + assert_eq!(m.z_axis, Vec4::new(0.0, 0.0, 1.0, 0.0)); + } + + let m = resolved_matrix( + Length::px(200.0), + Length::px(100.0), + Style::default().translate(Length::percent(10.0), Length::px(15.0)), + ); + assert_translation(m, Vec3::new(20.0, 15.0, 0.0)); + + // Same percent resolution on the UiTransform.matrix path (not just the + // Translate longhand) — TransformMatrix::Translate with a Percent term. + let m_matrix = resolved_matrix( + Length::px(200.0), + Length::px(100.0), + Style::default().ui_transform(UiTransform { + matrix: TransformMatrix::Translate( + Length::percent(10.0), + Length::px(15.0), + Length::ZERO, + ), + ..Default::default() + }), + ); + assert_translation(m_matrix, Vec3::new(20.0, 15.0, 0.0)); +} + +#[test] +fn cq_translate_is_residual_zero() { + // Cqw translate is a RESIDUAL deferral (needs the nearest CQ-ancestor + // frame, like sticky L4) — it resolves to 0.0 here, NOT to a cqw value. + // With every other term identity, the matrix collapses to identity, so + // sub-pass 6e inserts no ResolvedTransform. This documents scope discipline. + let m = resolved_matrix( + Length::px(100.0), + Length::px(100.0), + Style::default().ui_transform(UiTransform { + matrix: TransformMatrix::Translate(Length::Cqw(50.0), Length::ZERO, Length::ZERO), + ..Default::default() + }), + ); + assert_eq!( + m, + Mat4::IDENTITY, + "Cq* translate is unresolved (0.0 residual) → identity matrix" + ); +} + #[test] fn display_none_transformed_entity_gets_no_resolved_transform() { let mut app = app(); diff --git a/crates/buiy_verify/src/invariant/predicates.rs b/crates/buiy_verify/src/invariant/predicates.rs index 26aa9bd..8d57ac1 100644 --- a/crates/buiy_verify/src/invariant/predicates.rs +++ b/crates/buiy_verify/src/invariant/predicates.rs @@ -107,8 +107,21 @@ pub fn paint_order_is_total(nodes: &ExtractedNodes) -> Result<(), Violation> { pub fn transform_roundtrips(t: &GenTransform) -> Result<(), Violation> { // (a) translate(d) · translate(-d) ≈ I. let d = Vec3::from_array(t.translate); - let fwd = compose_transform(&UiTransform::default(), Some(&translate_of(d)), None, None); - let back = compose_transform(&UiTransform::default(), Some(&translate_of(-d)), None, None); + // Px translates are box-independent → ZERO box is fine. + let fwd = compose_transform( + &UiTransform::default(), + Some(&translate_of(d)), + None, + None, + Vec2::ZERO, + ); + let back = compose_transform( + &UiTransform::default(), + Some(&translate_of(-d)), + None, + None, + Vec2::ZERO, + ); mat4_is_identity("transform_roundtrips/translate", fwd * back)?; // (b) rotate(2π) ≈ I. A full turn about the generated axis. @@ -124,6 +137,7 @@ pub fn transform_roundtrips(t: &GenTransform) -> Result<(), Violation> { None, Some(&Rotate(full_turn)), None, + Vec2::ZERO, ); mat4_is_identity("transform_roundtrips/rotate2pi", rot)?; @@ -134,6 +148,7 @@ pub fn transform_roundtrips(t: &GenTransform) -> Result<(), Violation> { None, None, Some(&Scale(k[0], k[1], k[2])), + Vec2::ZERO, ); mat4_is_pure_scale("transform_roundtrips/scale", s, k)?; Ok(()) diff --git a/docs/plans/follow-ups.md b/docs/plans/follow-ups.md index 66ec469..5e75857 100644 --- a/docs/plans/follow-ups.md +++ b/docs/plans/follow-ups.md @@ -819,18 +819,38 @@ window's root context instead of `roots.first()`. **Spec touchpoint:** `stacking-and-top-layer.md § 4.4`, § 7. -## Layout — non-px translate units in `compose_transform` +## Layout — non-px translate units in `compose_transform` — PERCENT LANDED (Cq* residual) **Originated:** Phase 8 (CHANGELOG deferral note). **Symptom:** `compose_transform` resolves only `Length::Px` for translate; percent / `Cq*` translate contributes `0.0`. -**Implementation sketch:** resolve percent / `Cq*` translate against the -entity's own resolved box (currently `0.0`); coordinate with the animation -phase. - -**Spec touchpoint:** `transforms-and-containment.md § 1`, § 1.1. +**Status — PERCENT landed (2026-06-18).** As landed: `compose_transform` and +`transform_matrix_to_mat4` now take the entity's own current-frame border box +and resolve a `Percent` translate term against it per CSS Transforms — +`translateX(p%)` = `p%` of border-box **width**, `translateY(p%)` = `p%` of +**height** (each axis against its own dimension), `translateZ` percent (invalid +in CSS) → `0`. Sub-pass 6e (`transform_composition`) reads the box straight from +the **current-frame** Taffy tree (`tree.tree.layout(node).size`, mirroring +`anchor_resolution` (6d)) — *not* `ResolvedLayout`, which is still last-frame at +6e time. The `Length::Px` translate path is byte-for-byte unchanged (regression +guarded by `translate_transform_composes_to_resolved_transform`). Tests: +`translate_percent_x_resolves_against_own_width`, +`translate_percent_y_resolves_against_own_height`, +`translate_mixed_percent_and_px`, `cq_translate_is_residual_zero` +(`crates/buiy_core/tests/layout_transforms.rs`); the `Style::translate(Length, +Length)` builder was added to express percent translate. + +**RESIDUAL — `Cq*` translate still deferred.** `Cq*` translate +(`cqw/cqh/cqi/cqb/cqmin/cqmax`) needs the entity's nearest CQ-ancestor container +frame (the sticky-L4 / `resolve_cq_unit_px` machinery), which sub-pass 6e does +not gather. It resolves to `0.0` and fires a one-shot warn +(`warn_once_cq_translate_residual`). Resolving it requires threading the nearest +CQ-ancestor `ContainerSnapshot` into 6e — held out for scope discipline. + +**Spec touchpoint:** `transforms-and-containment.md § 1` ("Translate length +units"), § 1.1. ## Render — effect-compositor depends on the opacity stacking-context trigger (contiguity) — LANDED diff --git a/docs/specs/2026-05-08-buiy-layout-design/transforms-and-containment.md b/docs/specs/2026-05-08-buiy-layout-design/transforms-and-containment.md index 2b2d622..e0d0ad8 100644 --- a/docs/specs/2026-05-08-buiy-layout-design/transforms-and-containment.md +++ b/docs/specs/2026-05-08-buiy-layout-design/transforms-and-containment.md @@ -73,6 +73,8 @@ M = T_translate · R_rotate · S_scale · M_transform a matrix product written left-to-right in outermost-to-innermost order: `T_translate` is the outermost factor and `M_transform` the innermost, so a child point `p` is transformed as `M · p` and therefore feels the *rightmost* (innermost) factor first. For `TransformMatrix::Compose([A, B, …])` the list is written outermost-first and applied as the matrix product `A · B · …` — same rule: the rightmost/innermost entry transforms a child point first. This is exactly the convention CSS uses for a `transform:` list written left-to-right. Phase 8 implements this product; there is no separate "right-to-left" rule to reconcile — "rightmost-applied-first" is just what reading the matrix product `M · p` means. +**Translate length units.** A translate term carries a [`Length`](container-queries-and-writing-modes.md) per axis. `Px` resolves to its magnitude. **`Percent` resolves against the entity's OWN border box** per CSS Transforms — `translateX(p%)` = `p%` of border-box **width**, `translateY(p%)` = `p%` of **height** (each axis against its own dimension); `translateZ` percentages are invalid in CSS and resolve to **0**. This resolution happens in sub-pass **6e** (`transform_composition`), which reads the entity's **current-frame** Taffy border box (`tree.tree.layout(node).size`, the same box `anchor_resolution` (6d) reads — *not* `ResolvedLayout`, which is still last-frame at 6e time since `WriteResolvedLayout` runs later). `Cq*` translate (`cqw/cqh/cqi/cqb/cqmin/cqmax`) is a **residual deferral**: it needs the entity's nearest CQ-ancestor container frame (the sticky-L4 / `resolve_cq_unit_px` machinery), which 6e does not gather, so it resolves to **0.0** and fires a one-shot warn. `Fr`/`anchor-size()` are meaningless on a transform and likewise resolve to 0. + ### 1.1 Longhand components CSS exposes `translate`, `rotate`, `scale` as separate properties applied independently of `transform`. Buiy mirrors: @@ -98,7 +100,7 @@ impl Default for Scale { } ``` -When present, these compose with `UiTransform.matrix` per the single convention in [§ 1](#1-transform): `M = T_translate · R_rotate · S_scale · M_transform` (the written order `translate → rotate → scale → transform.matrix` is outermost-to-innermost, so `M_transform` transforms a child point first). The composition runs as `PostTaffyOverrides` sub-pass **6e** (`Phase 8` — extending the shipped sub-pass chain: 6a sticky, 6b table, 6c multicol, 6d anchor; per [architecture.md § 3](architecture.md#3-system-pipeline)); the composed matrix is written to a private `ResolvedTransform` component for render and consumed by step 7 (`WriteResolvedLayout`). Transform-triggered stacking-context detection (`Phase 9`, [stacking-and-top-layer.md](stacking-and-top-layer.md)) runs as sub-pass **6f**, *after* 6e, since it reads the composed matrix produced by 6e. +When present, these compose with `UiTransform.matrix` per the single convention in [§ 1](#1-transform): `M = T_translate · R_rotate · S_scale · M_transform` (the written order `translate → rotate → scale → transform.matrix` is outermost-to-innermost, so `M_transform` transforms a child point first). The composition runs as `PostTaffyOverrides` sub-pass **6e** (`Phase 8` — extending the shipped sub-pass chain: 6a sticky, 6b table, 6c multicol, 6d anchor; per [architecture.md § 3](architecture.md#3-system-pipeline)); the composed matrix is written to a private `ResolvedTransform` component for render and consumed by step 7 (`WriteResolvedLayout`). Translate percent terms resolve against the entity's own current-frame border box at this sub-pass (see [§ 1](#1-transform), "Translate length units"); `Cq*` translate is a residual `0.0` deferral. Transform-triggered stacking-context detection (`Phase 9`, [stacking-and-top-layer.md](stacking-and-top-layer.md)) runs as sub-pass **6f**, *after* 6e, since it reads the composed matrix produced by 6e. ### 1.2 Layout impact