From a01563b0684a48252f6b26ee86af431d742a730a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 24 Jan 2023 16:29:13 +0000 Subject: [PATCH 01/10] Add mechanism to keep track of scheduled animations. --- src/textual/_animator.py | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/textual/_animator.py b/src/textual/_animator.py index fb2b4015b..2f7f5a89f 100644 --- a/src/textual/_animator.py +++ b/src/textual/_animator.py @@ -22,6 +22,8 @@ if TYPE_CHECKING: from textual.app import App EasingFunction = Callable[[float], float] +AnimationKey = tuple[int, str] +"""Animation keys are the id of the object and the attribute being animated.""" class AnimationError(Exception): @@ -166,10 +168,19 @@ class BoundAnimator: class Animator: - """An object to manage updates to a given attribute over a period of time.""" + """An object to manage updates to a given attribute over a period of time. + + Attrs: + _animations: Dictionary that maps animation keys to the corresponding animation + instances. + _scheduled: Keys corresponding to animations that have been scheduled but not yet + started. + app: The app that owns the animator object. + """ def __init__(self, app: App, frames_per_second: int = 60) -> None: - self._animations: dict[tuple[object, str], Animation] = {} + self._animations: dict[AnimationKey, Animation] = {} + self._scheduled: set[AnimationKey] = set() self.app = app self._timer = Timer( app, @@ -196,7 +207,7 @@ class Animator: self._idle_event.set() def bind(self, obj: object) -> BoundAnimator: - """Bind the animator to a given objects.""" + """Bind the animator to a given object.""" return BoundAnimator(self, obj) def animate( @@ -237,6 +248,7 @@ class Animator: on_complete=on_complete, ) if delay: + self._scheduled.add((id(obj), attribute)) self.app.set_timer(delay, animate_callback) else: animate_callback() @@ -273,13 +285,14 @@ class Animator: duration is None and speed is not None ), "An Animation should have a duration OR a speed" + animation_key = (id(obj), attribute) + self._scheduled.discard(animation_key) + if final_value is ...: final_value = value start_time = self._get_time() - animation_key = (id(obj), attribute) - easing_function = EASING[easing] if isinstance(easing, str) else easing animation: Animation | None = None From 1d18ac35c513375aca75185fa2b3f22f8605b404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 24 Jan 2023 16:29:27 +0000 Subject: [PATCH 02/10] Add method to check if something is being animated. --- src/textual/_animator.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/textual/_animator.py b/src/textual/_animator.py index 2f7f5a89f..3874a07e1 100644 --- a/src/textual/_animator.py +++ b/src/textual/_animator.py @@ -210,6 +210,11 @@ class Animator: """Bind the animator to a given object.""" return BoundAnimator(self, obj) + def is_being_animated(self, obj: object, attribute: str) -> bool: + """Does the object/attribute pair have an ongoing or scheduled animation?""" + key = (id(obj), attribute) + return key in self._animations or key in self._scheduled + def animate( self, obj: object, From 34a08b7fe58ebab2eec7c43d185da459bbdecc5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 24 Jan 2023 16:29:49 +0000 Subject: [PATCH 03/10] Check if a rule is being animated when updating it. This is the fix for #1372 because the styles that were sticking had nothing to do with `:hover` per se. The issue was in the fact that we were trying to start a second animation (back to the default background color) while the first animation hadn't started yet, so we would skip the creation of the new animation but the old one would still run to completion. This issue also affects animations that start with a delay. If we set an animation A -> B with a delay of 10s and 1s later we set an animation B -> A, then the animation A -> B will run after 9s but the animation B -> A will not run because it was not created in the first place. --- src/textual/css/stylesheet.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/textual/css/stylesheet.py b/src/textual/css/stylesheet.py index 9b00add69..0b676776d 100644 --- a/src/textual/css/stylesheet.py +++ b/src/textual/css/stylesheet.py @@ -449,6 +449,8 @@ class Stylesheet: get_new_render_rule = new_render_rules.get if animate: + animator = node.app.animator + base = node.styles.base for key in modified_rule_keys: # Get old and new render rules old_render_value = get_current_render_rule(key) @@ -456,13 +458,18 @@ class Stylesheet: # Get new rule value (may be None) new_value = rules.get(key) - # Check if this can / should be animated - if is_animatable(key) and new_render_value != old_render_value: + # Check if this can / should be animated. It doesn't suffice to check + # if the current and target values are different because a previous + # animation may have been scheduled but may have not started yet. + if is_animatable(key) and ( + new_render_value != old_render_value + or animator.is_being_animated(base, key) + ): transition = new_styles._get_transition(key) if transition is not None: duration, easing, delay = transition - node.app.animator.animate( - node.styles.base, + animator.animate( + base, key, new_render_value, final_value=new_value, From 15c95db9608e7be28c530cc5a2ba8aa0785567b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 24 Jan 2023 18:12:25 +0000 Subject: [PATCH 04/10] Add mechanism to wait for current and scheduled animations. --- src/textual/_animator.py | 13 +++++++++++++ src/textual/pilot.py | 6 +++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/textual/_animator.py b/src/textual/_animator.py index 3874a07e1..85ad72e87 100644 --- a/src/textual/_animator.py +++ b/src/textual/_animator.py @@ -190,11 +190,15 @@ class Animator: callback=self, pause=True, ) + # Flag if no animations are currently taking place. self._idle_event = asyncio.Event() + # Flag if no animations are currently taking place and none are scheduled. + self._fully_idle_event = asyncio.Event() async def start(self) -> None: """Start the animator task.""" self._idle_event.set() + self._fully_idle_event.set() self._timer.start() async def stop(self) -> None: @@ -205,6 +209,7 @@ class Animator: pass finally: self._idle_event.set() + self._fully_idle_event.set() def bind(self, obj: object) -> BoundAnimator: """Bind the animator to a given object.""" @@ -254,6 +259,7 @@ class Animator: ) if delay: self._scheduled.add((id(obj), attribute)) + self._fully_idle_event.clear() self.app.set_timer(delay, animate_callback) else: animate_callback() @@ -360,11 +366,14 @@ class Animator: self._animations[animation_key] = animation self._timer.resume() self._idle_event.clear() + self._fully_idle_event.clear() async def __call__(self) -> None: if not self._animations: self._timer.pause() self._idle_event.set() + if not self._scheduled: + self._fully_idle_event.set() else: animation_time = self._get_time() animation_keys = list(self._animations.keys()) @@ -386,3 +395,7 @@ class Animator: async def wait_for_idle(self) -> None: """Wait for any animations to complete.""" await self._idle_event.wait() + + async def wait_for_fully_idle(self) -> None: + """Wait for any current and scheduled animations to complete.""" + await self._fully_idle_event.wait() diff --git a/src/textual/pilot.py b/src/textual/pilot.py index 13ae9fe03..d6daf1a78 100644 --- a/src/textual/pilot.py +++ b/src/textual/pilot.py @@ -43,9 +43,13 @@ class Pilot(Generic[ReturnType]): await asyncio.sleep(delay) async def wait_for_animation(self) -> None: - """Wait for any animation to complete.""" + """Wait for any current animation to complete.""" await self._app.animator.wait_for_idle() + async def wait_for_scheduled_animations(self) -> None: + """Wait for any current and scheduled animations to complete.""" + await self._app.animator.wait_for_fully_idle() + async def exit(self, result: ReturnType) -> None: """Exit the app with the given result. From c8ff5bd14bb74277b6c15472e5f719c549c98381 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 24 Jan 2023 18:30:24 +0000 Subject: [PATCH 05/10] Add tests. --- tests/test_animation.py | 122 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 121 insertions(+), 1 deletion(-) diff --git a/tests/test_animation.py b/tests/test_animation.py index 0498e7693..14d57f6fc 100644 --- a/tests/test_animation.py +++ b/tests/test_animation.py @@ -9,7 +9,7 @@ class AnimApp(App): #foo { height: 1; } - + """ def compose(self) -> ComposeResult: @@ -37,3 +37,123 @@ async def test_animate_height() -> None: assert elapsed >= 0.5 # Check the height reached the maximum assert static.styles.height.value == 100 + + +async def test_scheduling_animation() -> None: + """Test that scheduling an animation works.""" + + app = AnimApp() + delay = 0.1 + + async with app.run_test() as pilot: + styles = app.query_one(Static).styles + styles.background = "black" + + styles.animate("background", "white", delay=delay, duration=0) + + await pilot.pause(0.9 * delay) + assert styles.background.rgb == (0, 0, 0) # Still black + + await pilot.wait_for_scheduled_animations() + assert styles.background.rgb == (255, 255, 255) + + +async def test_wait_for_current_animations() -> None: + """Test that we can wait only for the current animations taking place.""" + + app = AnimApp() + + delay = 10 + + async with app.run_test() as pilot: + styles = app.query_one(Static).styles + styles.animate("height", 100, duration=0.1) + start = perf_counter() + styles.animate("height", 200, duration=0.1, delay=delay) + + # Wait for the first animation to finish + await pilot.wait_for_animation() + elapsed = perf_counter() - start + assert elapsed < (delay / 2) + + +async def test_wait_for_current_and_scheduled_animations() -> None: + """Test that we can wait for current and scheduled animations.""" + + app = AnimApp() + + async with app.run_test() as pilot: + styles = app.query_one(Static).styles + + start = perf_counter() + styles.animate("height", 50, duration=0.01) + styles.animate("background", "black", duration=0.01, delay=0.05) + + await pilot.wait_for_scheduled_animations() + elapsed = perf_counter() - start + assert elapsed >= 0.06 + assert styles.background.rgb == (0, 0, 0) + + +async def test_reverse_animations() -> None: + """Test that you can create reverse animations. + + Regression test for #1372 https://github.com/Textualize/textual/issues/1372 + """ + + app = AnimApp() + + async with app.run_test() as pilot: + static = app.query_one(Static) + styles = static.styles + + # Starting point. + styles.background = "black" + assert styles.background.rgb == (0, 0, 0) + + # First, make sure we can go from black to white and back, step by step. + styles.animate("background", "white", duration=0.01) + await pilot.wait_for_animation() + assert styles.background.rgb == (255, 255, 255) + + styles.animate("background", "black", duration=0.01) + await pilot.wait_for_animation() + assert styles.background.rgb == (0, 0, 0) + + # Now, the actual test is to make sure we go back to black if scheduling both at once. + styles.animate("background", "white", duration=0.01) + styles.animate("background", "black", duration=0.01) + await pilot.wait_for_animation() + assert styles.background.rgb == (0, 0, 0) + + +async def test_schedule_reverse_animations() -> None: + """Test that you can schedule reverse animations. + + Regression test for #1372 https://github.com/Textualize/textual/issues/1372 + """ + + app = AnimApp() + + async with app.run_test() as pilot: + static = app.query_one(Static) + styles = static.styles + + # Starting point. + styles.background = "black" + assert styles.background.rgb == (0, 0, 0) + + # First, make sure we can go from black to white and back, step by step. + styles.animate("background", "white", delay=0.01, duration=0.01) + await pilot.wait_for_scheduled_animations() + assert styles.background.rgb == (255, 255, 255) + + styles.animate("background", "black", delay=0.01, duration=0.01) + await pilot.wait_for_scheduled_animations() + assert styles.background.rgb == (0, 0, 0) + + # Now, the actual test is to make sure we go back to black if scheduling both at once. + styles.animate("background", "white", delay=0.01, duration=0.01) + styles.animate("background", "black", delay=0.01, duration=0.01) + await pilot.wait_for_scheduled_animations() + assert styles.background.rgb == (0, 0, 0) From cd0cc6030b6b24bc961cee0c77fcd00f9006792d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 24 Jan 2023 18:36:55 +0000 Subject: [PATCH 06/10] Changelog. --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b014c4fbb..db72150e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [0.11.0] - Unreleased +### Added + +- Added the coroutines `Animator.wait_for_fully_idle` and `pilot.wait_for_scheduled_animations` that allow waiting for all current and scheduled animations https://github.com/Textualize/textual/issues/1658 + ### Fixed - Fixed stuck screen https://github.com/Textualize/textual/issues/1632 - Fixed relative units in `grid-rows` and `grid-columns` being computed with respect to the wrong dimension https://github.com/Textualize/textual/issues/1406 +- Fixed bug with animations that were triggered back to back, where the second one wouldn't start https://github.com/Textualize/textual/issues/1372 +- Fixed bug with animations that were scheduled where all but the first would be skipped https://github.com/Textualize/textual/issues/1372 ## [0.10.1] - 2023-01-20 From ad24349bb24a7dac35cc213b89dd1d0576c343b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 24 Jan 2023 18:38:30 +0000 Subject: [PATCH 07/10] Move auxiliary type to TYPE_CHECKING. --- src/textual/_animator.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/textual/_animator.py b/src/textual/_animator.py index 85ad72e87..98e0ee413 100644 --- a/src/textual/_animator.py +++ b/src/textual/_animator.py @@ -21,9 +21,10 @@ else: # pragma: no cover if TYPE_CHECKING: from textual.app import App + AnimationKey = tuple[int, str] + """Animation keys are the id of the object and the attribute being animated.""" + EasingFunction = Callable[[float], float] -AnimationKey = tuple[int, str] -"""Animation keys are the id of the object and the attribute being animated.""" class AnimationError(Exception): From fa6bd4486692bfafef322eded276e34f9e4d4087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 24 Jan 2023 18:46:37 +0000 Subject: [PATCH 08/10] Housekeeping. --- CHANGELOG.md | 1 + tests/test_animation.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db72150e0..d23f4705f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - Added the coroutines `Animator.wait_for_fully_idle` and `pilot.wait_for_scheduled_animations` that allow waiting for all current and scheduled animations https://github.com/Textualize/textual/issues/1658 +- Added the method `Animator.is_being_animated` that checks if an attribute of an object is being animated or is scheduled for animation ### Fixed diff --git a/tests/test_animation.py b/tests/test_animation.py index 14d57f6fc..eea935a33 100644 --- a/tests/test_animation.py +++ b/tests/test_animation.py @@ -120,7 +120,7 @@ async def test_reverse_animations() -> None: await pilot.wait_for_animation() assert styles.background.rgb == (0, 0, 0) - # Now, the actual test is to make sure we go back to black if scheduling both at once. + # Now, the actual test is to make sure we go back to black if creating both at once. styles.animate("background", "white", duration=0.01) styles.animate("background", "black", duration=0.01) await pilot.wait_for_animation() From 0f2e991bf5c02fe982b21aaf7ad56610ab53b4cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Wed, 25 Jan 2023 13:04:14 +0000 Subject: [PATCH 09/10] Rename event. --- CHANGELOG.md | 2 +- src/textual/_animator.py | 16 ++++++++-------- src/textual/pilot.py | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5594b6679..e488bb6a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added -- Added the coroutines `Animator.wait_for_fully_idle` and `pilot.wait_for_scheduled_animations` that allow waiting for all current and scheduled animations https://github.com/Textualize/textual/issues/1658 +- Added the coroutines `Animator.wait_until_complete` and `pilot.wait_for_scheduled_animations` that allow waiting for all current and scheduled animations https://github.com/Textualize/textual/issues/1658 - Added the method `Animator.is_being_animated` that checks if an attribute of an object is being animated or is scheduled for animation ### Changed diff --git a/src/textual/_animator.py b/src/textual/_animator.py index 98e0ee413..330ea2dfd 100644 --- a/src/textual/_animator.py +++ b/src/textual/_animator.py @@ -194,12 +194,12 @@ class Animator: # Flag if no animations are currently taking place. self._idle_event = asyncio.Event() # Flag if no animations are currently taking place and none are scheduled. - self._fully_idle_event = asyncio.Event() + self._complete_event = asyncio.Event() async def start(self) -> None: """Start the animator task.""" self._idle_event.set() - self._fully_idle_event.set() + self._complete_event.set() self._timer.start() async def stop(self) -> None: @@ -210,7 +210,7 @@ class Animator: pass finally: self._idle_event.set() - self._fully_idle_event.set() + self._complete_event.set() def bind(self, obj: object) -> BoundAnimator: """Bind the animator to a given object.""" @@ -260,7 +260,7 @@ class Animator: ) if delay: self._scheduled.add((id(obj), attribute)) - self._fully_idle_event.clear() + self._complete_event.clear() self.app.set_timer(delay, animate_callback) else: animate_callback() @@ -367,14 +367,14 @@ class Animator: self._animations[animation_key] = animation self._timer.resume() self._idle_event.clear() - self._fully_idle_event.clear() + self._complete_event.clear() async def __call__(self) -> None: if not self._animations: self._timer.pause() self._idle_event.set() if not self._scheduled: - self._fully_idle_event.set() + self._complete_event.set() else: animation_time = self._get_time() animation_keys = list(self._animations.keys()) @@ -397,6 +397,6 @@ class Animator: """Wait for any animations to complete.""" await self._idle_event.wait() - async def wait_for_fully_idle(self) -> None: + async def wait_until_complete(self) -> None: """Wait for any current and scheduled animations to complete.""" - await self._fully_idle_event.wait() + await self._complete_event.wait() diff --git a/src/textual/pilot.py b/src/textual/pilot.py index d6daf1a78..5f3fbe817 100644 --- a/src/textual/pilot.py +++ b/src/textual/pilot.py @@ -48,7 +48,7 @@ class Pilot(Generic[ReturnType]): async def wait_for_scheduled_animations(self) -> None: """Wait for any current and scheduled animations to complete.""" - await self._app.animator.wait_for_fully_idle() + await self._app.animator.wait_until_complete() async def exit(self, result: ReturnType) -> None: """Exit the app with the given result. From 11f470b59bd61564e8b8c549cf4a1a6baa17c5ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Wed, 25 Jan 2023 13:40:57 +0000 Subject: [PATCH 10/10] Ensure animation scheduling order. --- tests/test_animation.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_animation.py b/tests/test_animation.py index eea935a33..8aa1615a2 100644 --- a/tests/test_animation.py +++ b/tests/test_animation.py @@ -154,6 +154,7 @@ async def test_schedule_reverse_animations() -> None: # Now, the actual test is to make sure we go back to black if scheduling both at once. styles.animate("background", "white", delay=0.01, duration=0.01) + await pilot.pause(0.005) styles.animate("background", "black", delay=0.01, duration=0.01) await pilot.wait_for_scheduled_animations() assert styles.background.rgb == (0, 0, 0)