fix(blur): ghosts stop stealing flex slots, and blur gains an axis - #401
Merged
Merged
Conversation
…e their own clock (#359, #360) build_ghosts (box_builder.rs) had three defects reported as one issue, all traceable to the same root: a ghost was never given its own position, its own children, or its own paint-time clock. - Regression of #66: base_css_for_ghost only set `position: absolute` when the principal itself already was, so a ghost of a flow child kept the principal's flow-participating CSS and became an extra full-size flex item -- `samples` copies pushing every later sibling out of place. A ghost is now unconditionally `position: absolute`; one already-absolute gets its exact left/top, a flow one falls back to Taffy's justify/align resolution for an inset-less absolute child, which never consumes space. - A ghost of a container was built with `children: Vec::new()`, so only the container's own background/border ever traveled with the trail -- a card's label, a mockup's screenshot, never did. build_one_ghost now calls the existing `container_children` at the ghost's own sampled time (via a `(scale, shift)` remap identical in shape to the principal's, just re-anchored so `ghost_time` plays the role `actx.time` normally does), so nested content is rebuilt, not copied. A ghost of a *measured* leaf (text, counter, badge, table, rich_text, ...) also picks up its own `component_intrinsic`, which the old ghost never set -- without it the box measured to zero and nothing painted, the "shape trails, text doesn't" half of the same issue. - A `pointer` moved by `path` reads its own position from `ctx.time` inside `paint_content`, not from `style.transform` -- so re-evaluating its CSS at a ghost's sampled time did nothing, since dispatch recomputes `ctx.time` itself from `time_params` at paint time. Every ghost now gets its own `time_params` entry, shifted by exactly its own sample offset, so `LegacyPaintDispatcher::dispatch`'s `local_time` -- and therefore `ctx.time` handed to `paint_content` -- lands on the ghost's own instant for *any* effect that reads live time at dispatch, not just transforms baked in at build time. Each of the three has a test named for its fault in box_builder.rs's tests module (ghost_does_not_steal_a_flex_slot, ghost_of_a_container_carries_its_nested_child + ghost_of_a_measured_leaf_keeps_its_intrinsic_size, ghost_of_a_path_driven_pointer_samples_a_different_waypoint_offset), each reverted against the pre-fix build_ghosts and confirmed red before being restored. #360's directional blur rides the same fix, since a smear kernel needed motion_blur's internals anyway: - `FilterFn::Blur` gains optional `radius-x`/`radius-y` (falling back to `radius` per-axis when absent -- fully backward compatible), and a new `FilterFn::DirectionalBlur { angle, radius }` for the diagonal case, rendered in paint_pass.rs by rotating the sampled content around the box's own center, blurring on one axis, and rotating back. - `blur_x`/`blur_y` join `KNOWN_MOTION_PROPERTIES` and `AnimatedProperties` as animatable keyframe properties, surfaced as the same `Blur` filter via a new box_builder.rs helper (apply_animated_props in css/animation.rs isn't in this workstream's owned files, so the isotropic `blur` path there is untouched and the new axes are composed alongside it instead). - `motion_blur mode: "smear"` skips the ghost sampler entirely: it measures the node's own displacement over the `shutter / fps` window and applies that as a `Blur { radius-x, radius-y }` filter on the principal -- the "real kernel" the issue preferred over stacking more visible copies. Touched crates/rustmotion-core/src/engine/paint_pass.rs (shared): the two `filters_to_image_filter` call sites now pass `box_layout` (needed for `DirectionalBlur`'s rotation pivot), the `Blur`/`DirectionalBlur` match arms in `filter_bleed`/`filters_to_image_filter`/`color_matrix_for`, a new `resolve_blur_radii` + `directional_blur_image_filter` helper, and three existing `FilterFn::Blur { radius }` test literals updated for the new fields. Also one mechanical line in css/animation.rs's `apply_animated_props` (outside owned files) to add the two new `Blur` fields to its existing isotropic-blur construction site, unavoidable once `FilterFn::Blur` gained fields. Not implemented, and named as such in the new rule docs: a `scope` toggle to ghost only a container's own box without its children (every container ghost now always carries its subtree; no scenario needed the opt-out), `samples: "auto"` densification (superseded by `mode: "smear"`), and per-character `blur_axis`/`stretch` on the char_* presets (lives in text.rs/renderer/text.rs, outside this workstream's owned files -- wiring the schema field without a consumer would be exactly the inert-field pattern this codebase avoids elsewhere). Two new French rule docs land under skills/rules/ (motion-blur-and-trail.md, directional-blur.md); SKILL.md is deliberately left unlinked since the orchestrator owns that index.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #359. Closes #360. Refs #388.
#359 — three faults, all in ghost construction
Ghosts stole flex slots — a regression of #66.
base_css_for_ghostonly setposition: absolutewhen the principal was already absolute, so a flow child's ghost kept flow-participating CSS and became an extra full-size flex item. Ghosts are now unconditionally absolute; an inset-less absolute child is resolved by justify/align, so it consumes no slot.That last fact was verified empirically against Taffy 0.10.1's flexbox source before the fix was committed to — it is real but undocumented behaviour, and worth knowing for anyone who touches ghost positioning again.
Ghosts dropped their children and lost intrinsic size — they were built with
children: Vec::new()andintrinsic: None. A card's label never trailed, and a measured leaf with no explicit width collapsed to 0×0 and painted nothing. Each ghost now builds its own children at its own sampled time and carries its own intrinsic.pointer.pathwas never sampled.Pointer::paint_contentreadsctx.timeat dispatch, not the baked transform, and ghosts reused the principal'stime_paramsentry — so re-evaluating CSS at the ghost's instant during build changed nothing. Each ghost now gets its own entry. Pre-fix, all four ghosts sampled the identical offset(1020.37036, 560.1852).#360 — blur gains an axis
radius-x/radius-yon the existing blur filter (falling back toradiusper axis), a newdirectional-blurfilter that rotates about the box centre and blurs on one axis, animatableblur_x/blur_y, andmotion_blur mode: "smear"which skips ghosts entirely and applies the node's own displacement over the shutter window as a directional blur.Three parts deliberately skipped, and named in the rule docs rather than silently dropped:
scope: "self"|"subtree"(no repro needed it),samples: "auto"(superseded bysmear, which the issue itself preferred), and per-characterblur_axis/stretchonchar_*presets — that last one lives in the text components, and adding the schema field without a consumer would be exactly the inert-field anti-pattern #339, #349 and #364 were all about.A latent bug found on the way, not fixed here
FilterFn's#[serde(tag = "fn", rename_all = "kebab-case")]renames variant tags only, not struct-variant field names in this serde version. It surfaced becauseradius-xsilently deserialised toNoneuntil an explicit#[serde(rename)]was added.That means
FilterFn::DropShadow'soffset_x/offset_yare almost certainly unreachable asoffset-x/offset-yin JSON today — no test or skill doc exercises them either way. Untouched here because it is unrelated to these two issues, but it deserves its own audit.Verification
Six new tests plus three JSON round-trips, each proven red by reverting.
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace(1668) all clean.paint_pass.rsis shared and was touched minimally: bothfilters_to_image_filtercall sites now passbox_layout(the rotation pivot), three match arms for the new fields and variant, and two private helpers.css/animation.rsneeded two mechanical lines onceFilterFn::Blurgained fields — flagged by the agent rather than buried.Written comment-free, per the codebase-wide rule from #345.