fix(workspace): Enable animation for theme background images - #14618
fix(workspace): Enable animation for theme background images#14618niyasrad wants to merge 2 commits into
Conversation
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Niyas Hameed.
|
|
Every PR must be linked to a same-repo issue before Oz can review it. Next step: open or find a same-repo issue describing this change, then link it to this PR by adding See the contribution guidelines for the full readiness model. Powered by Oz |
github-actions
Bot
added
the
external-contributor
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Every PR must be linked to a same-repo issue before Oz can review it.
Next step: open or find a same-repo issue describing this change, then link it to this PR by adding Closes #123 to the PR description (or using the "Development" sidebar on GitHub). A maintainer will mark the issue ready-to-implement when it is ready. Once it is marked, comment /oz-review to re-trigger review.
See the contribution guidelines for the full readiness model.
Powered by Oz
Add enable_animation_with_start_time() call to the workspace background rendering path. Without this, animated GIF backgrounds freeze on their first frame instead of playing. This follows the established pattern used elsewhere in the codebase (e.g. changelog_section.rs:153).
932afa0 to
0507341
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @niyasrad on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
Add regression coverage for the animated image frame advancement fix. The original bug was that paint_animated_image() would ignore the started_at field and always recompute Instant::now(), making elapsed_time ~0 and freezing animations on the first frame. The fix adds enable_animation_with_start_time() to set started_at, which paint_animated_image() then uses to compute elapsed time correctly. This commit includes: 1. Six solid tests directly verifying AnimatedImage::get_current_frame() frame advancement/wrapping over elapsed time with real GIF/WebP fixtures 2. Four regression tests for the paint_animated_image fix: - Tests that verify enable_animation_with_start_time() sets started_at - Tests that verify started_at is actually used for frame selection 3. Refactor: Extract elapsed-time computation into a pure helper method compute_elapsed_time_ms(&self, now: Instant) that can be unit-tested directly. This method respects self.started_at and would fail if someone reintroduces the bug (e.g., ignoring self.started_at or recomputing Instant::now() fresh). The new regression tests directly verify that compute_elapsed_time_ms uses self.started_at correctly, and would catch the regression if someone changed the logic to ignore started_at.
1 participant
Description
Adds the missing
enable_animation_with_start_time()call to the workspace background rendering path (app/src/workspace/view.rs). Animated GIF theme backgrounds currently freeze on their first frame because the animation start time is never initialized at this call site. Follows the same working pattern already used atchangelog_section.rs:153.Related to #5178 (
.gifbackground support) — see comment there for context. This doesn't add.gifpicker support in the theme UI (a separate change), but fixes the animation-freeze bug that would otherwise affect any.gifbackground, since the file-path deserialization doesn't restrict extensions today.Linked Issue
ready-to-specorready-to-implement.Note: #5178 is currently labeled
enhancement/triaged, not yetready-to-spec/ready-to-implement— flagging this openly rather than checking it off. Happy to file a dedicated issue if that's preferred.Testing
./script/runcargo check -p warppasses. I was not able to complete a full localcargo build+ visual/screenshot verification — my dev machine has 7.7GB RAM, well under what's needed to comfortably build this project (Warp's CI runners use 64GB). I'd appreciate help verifying the visual animation behavior before merge.Screenshots / Videos
None yet — see testing note above.
Agent Mode