fix(background): halo zones can be wide ellipses, not just circles - #351
Merged
Merged
Conversation
) A reference video needs a thin, wide light band along the top edge, plus rotation — a single `radius` can only ever draw a circle. HaloZone gains optional radius_x/radius_y (falling back to radius when absent) and a rotation in degrees. Circularity is decided structurally, not just by whether the new fields are present: a zone is circular whenever effective_radius_x() equals effective_radius_y(), regardless of rotation (rotating a circle is a no-op, so it's not even attempted). That keeps every legacy radius-only scenario on the exact original draw_circle call — same operation order, same floating point values — rather than trusting that Skia's drawOval happens to match drawCircle bit-for-bit. Explicitly setting radius_x == radius_y == radius takes the same fast path for the same reason, which is what the new byte-identity test exercises directly. The blur mask sizes off the minor axis (min(radius_x, radius_y)), so a very flat ellipse doesn't drown in a blur scaled to its long axis. The transition-interpolation path (lerp_zones) now carries radius_x, radius_y and rotation across a halo-to-halo scene transition, so an ellipse shape change no longer snaps at the cut. Note: crates/rustmotion/skills/SKILL.md's rules index was intentionally left untouched (out of scope for this change) — rules/halo-shapes.md still needs a line added there to be discoverable from the skill.
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
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.
Refs #344 — its point 2.
HaloZonehad a singleradius, so thehaloanimated-background preset could only draw circles. The reference video #344 was rebuilt against needs a thin, wide elliptical light along the top edge, and a rotated one. Neither could be expressed.What changed
HaloZonegainsradius_x,radius_y(bothOption<f32>, falling back toradius) androtationin degrees, pluseffective_radius_x(),effective_radius_y()andis_circular().Backward compatibility is the point, so it is tested byte-for-byte
Every scenario using plain
radiusmust render identically, and the implementation earns that rather than assuming it: a circular zone takes the exact original code path — same expression order, samedraw_circlecall — instead of being routed throughdraw_ovaland trusting Skia to matchdrawCirclebit-for-bit on anti-aliased edges.radius_only_zone_renders_byte_identically_to_explicit_equal_radius_x_radius_ypins it.is_circular()deliberately ignoresrotation: rotating a true circle is a no-op, and going through a rotation matrix for zero visual gain would perturb anti-aliased edge pixels.Two details worth knowing
Blur follows the short axis. A non-circular zone sizes its blur off
min(radius_x, radius_y), so a thin wide band does not get a blur scaled to its length and wash out.The transition interpolation was fixed too.
lerp_zones— thea_zones/b_zonespath used byinterpolate_animated_bg— now interpolates the three new fields, so a halo-to-halo scene transition will not snap the shape. That path is currently#[allow(dead_code)]and not yet wired to a caller; it was fixed anyway because it is the obvious future wiring point and a half-updated interpolator is the kind of thing that is found much later, from a render.Verification
14 tests: 7 on the schema helpers (defaults, fallback, the
is_circular()truth table including the rotation case), 5 on the painter, 2 on the transition interpolation.Proven to catch the absence — reverting
draw_bg_haloto the circle-only body, with the schema fields present but unused:cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace(1540) all clean.Note
The three new fields are snake_case (
radius_x,radius_y), unlike their kebab-case neighboursworld-positionandanimated-background. That inconsistency is the schema's, not a typo —rules/halo-shapes.mdsays so explicitly so a generator copies the casing rather than guessing it.Written comment-free, per the codebase-wide rule from #345.