Skip to content

Comments

Add very basic Animation support#34

Merged
NoahR02 merged 18 commits intomainfrom
feature/animations
Jul 10, 2025
Merged

Add very basic Animation support#34
NoahR02 merged 18 commits intomainfrom
feature/animations

Conversation

@NoahR02
Copy link
Collaborator

@NoahR02 NoahR02 commented Jul 8, 2025

This adds basic animation support and adds an example of how to use the api.

Currently font properties won't animate because the layout logic is being done in update_state rather than compute_layout.

@AustinMReppert AustinMReppert requested a review from Copilot July 8, 2025 15:00

This comment was marked as outdated.

NoahR02 and others added 7 commits July 8, 2025 11:33
It also shows how long a forced layout will take

2025-07-08T15:38:04.517153Z  INFO animate_tree:layout: craft_core::app: close time.busy=80.1µs time.idle=2.10µs
2025-07-08T15:38:04.517246Z  INFO animate_tree: craft_core::app: close time.busy=221µs time.idle=2.60µs
@AustinMReppert AustinMReppert requested a review from Copilot July 9, 2025 23:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds foundational animation support across the core UI library and a new example demonstrating how to use it.

  • Introduce Animation, KeyFrame, LoopAmount, and TimingFunction types for defining animations.
  • Extend Style, Element, and the rendering loop (App/CraftWinitState) to drive animations each frame.
  • Add examples/animations as a working demo of the new API.

Reviewed Changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
website/src/examples.rs Register the new "Animations" example in the sidebar
examples/animations/main.rs Provide a basic animation demonstration
examples/animations/Cargo.toml Setup dependencies for the animations example
crates/craft_core/src/style/styles.rs Add animations field and accessor/mutator methods
crates/craft_core/src/reactive/reactive_tree.rs Store previous animation flags in ReactiveTree
crates/craft_core/src/lib.rs Expose the animations module and init flags/time
crates/craft_core/src/elements/element_styles.rs Add push_animation builder method
crates/craft_core/src/elements/element_states.rs Derive Hash for ElementState enum
crates/craft_core/src/elements/element.rs Implement on_animation_frame and reset_layout_item
crates/craft_core/src/elements/base_element_state.rs Track per-element active animations
crates/craft_core/src/craft_winit_state.rs Switch event loop to Poll when animations active
crates/craft_core/src/app.rs Integrate animate_tree into the main update/render loop
crates/craft_core/src/animations/mod.rs New animations module exports
crates/craft_core/src/animations/animation.rs Define animation primitives and interpolation logic
Cargo.toml Add examples/animations to the workspace
Comments suppressed due to low confidence (1)

crates/craft_core/src/style/styles.rs:466

  • This public method lacks documentation. Please add a doc comment explaining its behavior—e.g., that it retrieves a named Animation if set in the style.
    pub fn animation(&self, animation: &str) -> Option<&Animation> {

}

let mut merged = old.clone();
merged.animations = None;
Copy link

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearing animations on every merge will drop scheduled animations as soon as you interpolate once. Consider preserving or merging the existing animations list instead of resetting it.

Suggested change
merged.animations = None;
merged.animations = match (&old.animations, &new.animations) {
(Some(old_animations), Some(new_animations)) => {
Some(old_animations.iter().chain(new_animations.iter()).cloned().collect())
}
(Some(old_animations), None) => Some(old_animations.clone()),
(None, Some(new_animations)) => Some(new_animations.clone()),
(None, None) => None,
};

Copilot uses AI. Check for mistakes.
Comment on lines +199 to +201
let mut sorted = animation.key_frames.iter().collect::<Vec<_>>();
sorted.sort_by(|a, b| a.offset_percentage.total_cmp(&b.offset_percentage));
for window in sorted.windows(2) {
Copy link

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Allocating and sorting a Vec of keyframes on every frame may be inefficient. You could keep keyframes pre-sorted in the Animation struct or use a fixed-size buffer to avoid repeated heap allocations.

Suggested change
let mut sorted = animation.key_frames.iter().collect::<Vec<_>>();
sorted.sort_by(|a, b| a.offset_percentage.total_cmp(&b.offset_percentage));
for window in sorted.windows(2) {
for window in animation.key_frames.windows(2) {

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@AustinMReppert AustinMReppert self-requested a review July 9, 2025 23:36
@NoahR02 NoahR02 merged commit ae6f658 into main Jul 10, 2025
6 checks passed
@AustinMReppert AustinMReppert deleted the feature/animations branch July 10, 2025 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants