fix(background): scroll only the presets that are periodic under it - #414
Merged
Merged
Conversation
compute_scroll_offset translated every preset before dispatch. Three of the seven are not periodic under translation and already animate themselves from speed: - gradient_shift uses direction for the rotation sense, and paints its shader over the frame rect with no margin, so any translation left an uncovered band. - concentric_circles computes its own offset = (time * speed) % spacing. The outer translation was a second animation on top, and translating a radial pattern moves its centre. - halo animates its zones internally. PR #154 bounded the offset to one tile period, which turned an unbounded drift into a bounded periodic jump. It is now zero for those three: declaring a direction on one of them is pixel-inert, verified frame by frame against the same scenario with no direction at all. This is a visible rendering change for an existing scenario that declares one. The motion being removed is the motion that dragged the background off the frame. The four that keep it -- grid_dots, grid_lines, pixel_grid, heropattern -- are tiled patterns with no motion of their own, drawn with a whole period of margin on each side. pixel_grid was the exception: its cell loops started at index 0, so scrolling right uncovered a band on the left. They start at -1 now, which is what the other three already did. pixel_grid's own `motion` field does not translate anything, so it composes with the scroll rather than doubling it, and its default (`none`) leaves it exactly in grid_dots's position: a still texture the outer scroll is the only thing that can move.
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.
Closes #155.
A scenario that declares
directionongradient_shift,concentric_circlesorhalorenders differently after this. The motion being removed is the motion thatdragged the background off the frame, so removing it is the fix — but it is worth
saying out loud rather than letting it arrive inside a batch.
What was wrong
compute_scroll_offsettranslated every preset before dispatch. Three of theseven are not periodic under translation, and already animate themselves from
speed:gradient_shiftdirectionmeanscw/ccwhere)Rect::from_wh(width, height)with no margin, so any offset leaves an uncovered bandconcentric_circlesoffset = (time * speed) % spacinghaloPR #154 bounded the offset to one tile period, turning an unbounded drift into a
bounded periodic jump. It is now zero for those three.
The four that keep it
grid_dots,grid_lines,pixel_gridandheropatternare tiled patterns withno motion of their own, drawn with a whole period of margin on each side. For them
the outer scroll is the only thing that can move anything.
pixel_gridwas the exception: its cell loops started at index0, so scrollingright uncovered a band on the left. They start at
-1now — what the other threealready did.
pixel_grid's ownmotionfield (twinkle/sweep) does not translate anything,so it composes with the scroll rather than doubling it, and its default (
none)leaves it exactly where
grid_dotsis: a still texture.Tests
The issue suggested comparing the
avg_lumaof a trailing band over time. Thatturned out to measure the wrong thing —
gradient_shiftandhalovary that bandlegitimately as they animate themselves, so the test failed on a correct
implementation. Two sharper statements replaced it:
direction_is_inert_on_a_preset_that_cannot_be_translateddirection: rightare byte-identical to frames with no direction at alldirection_still_moves_a_preset_that_has_no_motion_of_its_owna_scrolled_tile_never_uncovers_the_band_it_is_dragged_away_froma_preset_that_is_not_periodic_under_translation_is_never_translatedheropatternandconcentric_circlesare deliberately not in the coverage test:both leave transparent gaps by design, so the scene colour showing through is not
a defect there.
Both halves of the fix bite on revert:
Docs
rules/continuous-presets.mdgains the table of which presets acceptdirectionand why the other three now ignore it.
Gate
cargo fmt --all --checkclean ·cargo clippy --workspace --all-targets --features rustmotion/studio -D warningsclean ·cargo test --workspace1782 passed.