From 736f6b44b3f987469fd2862ef0a21a5ebd7f39b2 Mon Sep 17 00:00:00 2001 From: chopan Date: Wed, 20 Dec 2023 14:43:04 +0100 Subject: [PATCH 01/29] Optimized AnimationGroup computation of start-end times with lag ratio --- manim/animation/animation.py | 1 + manim/animation/composition.py | 56 +++++++++++++++++++++------------- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/manim/animation/animation.py b/manim/animation/animation.py index cb8fcaddfe..3979ba081c 100644 --- a/manim/animation/animation.py +++ b/manim/animation/animation.py @@ -403,6 +403,7 @@ def set_run_time(self, run_time: float) -> Animation: self.run_time = run_time return self + # TODO: is this getter even necessary? def get_run_time(self) -> float: """Get the run time of the animation. diff --git a/manim/animation/composition.py b/manim/animation/composition.py index ff219405f2..679add6b11 100644 --- a/manim/animation/composition.py +++ b/manim/animation/composition.py @@ -134,22 +134,33 @@ def init_run_time(self, run_time) -> float: The duration of the animation in seconds. """ self.build_animations_with_timings() - if self.anims_with_timings: - self.max_end_time = np.max([awt[2] for awt in self.anims_with_timings]) - else: - self.max_end_time = 0 + # Note: not necessarily the final animation's end time is the max end time! + self.max_end_time = max(self.anim_end_times, default=0) return self.max_end_time if run_time is None else run_time + # TODO: self.anims_with_timings is only kept for backwards compatibility, + # but may no longer be necessary? def build_animations_with_timings(self) -> None: """Creates a list of triplets of the form (anim, start_time, end_time).""" - self.anims_with_timings = [] - curr_time: float = 0 - for anim in self.animations: - start_time: float = curr_time - end_time: float = start_time + anim.get_run_time() - self.anims_with_timings.append((anim, start_time, end_time)) - # Start time of next animation is based on the lag_ratio - curr_time = (1 - self.lag_ratio) * start_time + self.lag_ratio * end_time + self.anim_run_times = np.array([anim.run_time for anim in self.animations]) + if self.anim_run_times.shape[0] == 0: + self.anim_start_times = np.empty(0) + self.anim_end_times = np.empty(0) + self.anims_with_timings = [] + return + + lags = self.anim_run_times[:-1] * self.lag_ratio + self.anim_start_times = np.zeros(self.anim_run_times.shape) + self.anim_start_times[1:] = np.add.accumulate(lags) + self.anim_end_times = self.anim_start_times + self.anim_run_times + + # + self.anims_with_timings = [ + (anim, start, end) + for anim, start, end in zip( + self.animations, self.anim_start_times, self.anim_end_times + ) + ] def interpolate(self, alpha: float) -> None: # Note, if the run_time of AnimationGroup has been @@ -158,12 +169,15 @@ def interpolate(self, alpha: float) -> None: # e.g. of the surrounding scene. Instead they'd # be a rescaled version. But that's okay! time = self.rate_func(alpha) * self.max_end_time - for anim, start_time, end_time in self.anims_with_timings: - anim_time = end_time - start_time - if anim_time == 0: - sub_alpha = 0 - else: - sub_alpha = np.clip((time - start_time) / anim_time, 0, 1) + sub_alphas = np.zeros(self.anim_run_times.shape) + sub_alphas[time >= self.anim_end_times] = 1 + f = ( + (self.anim_run_times > 0) + & (time > self.anim_start_times) + & (time < self.anim_end_times) + ) + sub_alphas[f] = (time - self.anim_start_times[f]) / self.anim_run_times[f] + for anim, sub_alpha in zip(self.animations, sub_alphas): anim.interpolate(sub_alpha) @@ -239,8 +253,8 @@ def update_active_animation(self, index: int) -> None: self.active_animation = self.animations[index] self.active_animation._setup_scene(self.scene) self.active_animation.begin() - self.active_start_time = self.anims_with_timings[index][1] - self.active_end_time = self.anims_with_timings[index][2] + self.active_start_time = self.anim_start_times[index] + self.active_end_time = self.anim_end_times[index] def next_animation(self) -> None: """Proceeds to the next animation. @@ -257,7 +271,7 @@ def interpolate(self, alpha: float) -> None: self.next_animation() if self.active_animation is not None and self.active_start_time is not None: elapsed = current_time - self.active_start_time - active_run_time = self.active_animation.get_run_time() + active_run_time = self.active_animation.run_time subalpha = elapsed / active_run_time if active_run_time != 0.0 else 1.0 self.active_animation.interpolate(subalpha) From 488a4e13a237a1deb92cc83fffc2b105c0e46768 Mon Sep 17 00:00:00 2001 From: chopan Date: Wed, 20 Dec 2023 15:49:38 +0100 Subject: [PATCH 02/29] Added extra comment for init_run_time --- manim/animation/composition.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/manim/animation/composition.py b/manim/animation/composition.py index 679add6b11..e8e108a313 100644 --- a/manim/animation/composition.py +++ b/manim/animation/composition.py @@ -134,7 +134,8 @@ def init_run_time(self, run_time) -> float: The duration of the animation in seconds. """ self.build_animations_with_timings() - # Note: not necessarily the final animation's end time is the max end time! + # Note: if lag_ratio < 1, then not necessarily the final animation's + # end time will be the max end time! self.max_end_time = max(self.anim_end_times, default=0) return self.max_end_time if run_time is None else run_time From c22a1fca69fc6c2ffcf4d193b2aceabdaafa4500 Mon Sep 17 00:00:00 2001 From: chopan Date: Wed, 20 Dec 2023 19:32:14 +0100 Subject: [PATCH 03/29] Added full path to imports in composition.py --- manim/animation/composition.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/manim/animation/composition.py b/manim/animation/composition.py index e8e108a313..5fc9585bb1 100644 --- a/manim/animation/composition.py +++ b/manim/animation/composition.py @@ -8,21 +8,19 @@ import numpy as np +from manim._config import config +from manim.animation.animation import Animation, prepare_animation +from manim.constants import RendererType +from manim.mobject.mobject import Group, Mobject from manim.mobject.opengl.opengl_mobject import OpenGLGroup +from manim.scene.scene import Scene +from manim.utils.iterables import remove_list_redundancies from manim.utils.parameter_parsing import flatten_iterable_parameters - -from .._config import config -from ..animation.animation import Animation, prepare_animation -from ..constants import RendererType -from ..mobject.mobject import Group, Mobject -from ..scene.scene import Scene -from ..utils.iterables import remove_list_redundancies -from ..utils.rate_functions import linear +from manim.utils.rate_functions import linear if TYPE_CHECKING: from manim.mobject.opengl.opengl_vectorized_mobject import OpenGLVGroup - - from ..mobject.types.vectorized_mobject import VGroup + from manim.mobject.types.vectorized_mobject import VGroup __all__ = ["AnimationGroup", "Succession", "LaggedStart", "LaggedStartMap"] @@ -155,7 +153,6 @@ def build_animations_with_timings(self) -> None: self.anim_start_times[1:] = np.add.accumulate(lags) self.anim_end_times = self.anim_start_times + self.anim_run_times - # self.anims_with_timings = [ (anim, start, end) for anim, start, end in zip( From 7a40ac59506b7f5e4c6b2a2428e887fe9399557d Mon Sep 17 00:00:00 2001 From: chopan Date: Wed, 20 Dec 2023 22:35:58 +0100 Subject: [PATCH 04/29] Optimized AnimationGroup.interpolate --- manim/animation/composition.py | 71 ++++++++++++---------- tests/module/animation/test_composition.py | 2 +- tests/opengl/test_composition_opengl.py | 2 +- 3 files changed, 41 insertions(+), 34 deletions(-) diff --git a/manim/animation/composition.py b/manim/animation/composition.py index 5fc9585bb1..00e90fc556 100644 --- a/manim/animation/composition.py +++ b/manim/animation/composition.py @@ -92,6 +92,7 @@ def begin(self) -> None: f"{self} has a run_time of 0 seconds, this cannot be " f"rendered correctly. {tmp}." ) + self.anim_time = 0.0 if self.suspend_mobject_updating: self.group.suspend_updating() for anim in self.animations: @@ -134,31 +135,23 @@ def init_run_time(self, run_time) -> float: self.build_animations_with_timings() # Note: if lag_ratio < 1, then not necessarily the final animation's # end time will be the max end time! - self.max_end_time = max(self.anim_end_times, default=0) + self.max_end_time = max(self.anims_with_timings["end"], default=0) return self.max_end_time if run_time is None else run_time - # TODO: self.anims_with_timings is only kept for backwards compatibility, - # but may no longer be necessary? def build_animations_with_timings(self) -> None: """Creates a list of triplets of the form (anim, start_time, end_time).""" - self.anim_run_times = np.array([anim.run_time for anim in self.animations]) - if self.anim_run_times.shape[0] == 0: - self.anim_start_times = np.empty(0) - self.anim_end_times = np.empty(0) - self.anims_with_timings = [] + run_times = np.array([anim.run_time for anim in self.animations]) + num_animations = run_times.shape[0] + dtype = [("anim", "O"), ("start", "f8"), ("end", "f8")] + self.anims_with_timings = np.empty(num_animations, dtype=dtype) + self.ongoing_anim_bools = np.zeros(num_animations, dtype=bool) + if num_animations == 0: return - lags = self.anim_run_times[:-1] * self.lag_ratio - self.anim_start_times = np.zeros(self.anim_run_times.shape) - self.anim_start_times[1:] = np.add.accumulate(lags) - self.anim_end_times = self.anim_start_times + self.anim_run_times - - self.anims_with_timings = [ - (anim, start, end) - for anim, start, end in zip( - self.animations, self.anim_start_times, self.anim_end_times - ) - ] + lags = run_times[:-1] * self.lag_ratio + self.anims_with_timings["anim"] = self.animations + self.anims_with_timings["start"][1:] = np.add.accumulate(lags) + self.anims_with_timings["end"] = self.anims_with_timings["start"] + run_times def interpolate(self, alpha: float) -> None: # Note, if the run_time of AnimationGroup has been @@ -166,17 +159,31 @@ def interpolate(self, alpha: float) -> None: # times might not correspond to actual times, # e.g. of the surrounding scene. Instead they'd # be a rescaled version. But that's okay! - time = self.rate_func(alpha) * self.max_end_time - sub_alphas = np.zeros(self.anim_run_times.shape) - sub_alphas[time >= self.anim_end_times] = 1 - f = ( - (self.anim_run_times > 0) - & (time > self.anim_start_times) - & (time < self.anim_end_times) - ) - sub_alphas[f] = (time - self.anim_start_times[f]) / self.anim_run_times[f] - for anim, sub_alpha in zip(self.animations, sub_alphas): - anim.interpolate(sub_alpha) + anim_time = self.rate_func(alpha) * self.max_end_time + time_goes_back = anim_time <= self.anim_time + + # Only update ongoing animations + A = self.anims_with_timings + new_ongoing_anim_bools = (anim_time > A["start"]) & (anim_time < A["end"]) + A = A[self.ongoing_anim_bools | new_ongoing_anim_bools] + + run_times = A["end"] - A["start"] + null = run_times == 0.0 + sub_alphas = anim_time - A["start"] + sub_alphas[~null] /= run_times[~null] + if time_goes_back: + sub_alphas[null | (sub_alphas < 0)] = 0 + else: + sub_alphas[null | (sub_alphas > 1)] = 1 + + for ongoing_anim, sub_alpha in zip(A["anim"], sub_alphas): + ongoing_anim.interpolate(sub_alpha) + + print(sub_alphas) + if alpha == 1.0: + print("A\n" * 10) + self.anim_time = anim_time + self.ongoing_anim_bools = new_ongoing_anim_bools class Succession(AnimationGroup): @@ -251,8 +258,8 @@ def update_active_animation(self, index: int) -> None: self.active_animation = self.animations[index] self.active_animation._setup_scene(self.scene) self.active_animation.begin() - self.active_start_time = self.anim_start_times[index] - self.active_end_time = self.anim_end_times[index] + self.active_start_time = self.anims_with_timings[index]["start"] + self.active_end_time = self.anims_with_timings[index]["end"] def next_animation(self) -> None: """Proceeds to the next animation. diff --git a/tests/module/animation/test_composition.py b/tests/module/animation/test_composition.py index 6837699fdf..878176de7e 100644 --- a/tests/module/animation/test_composition.py +++ b/tests/module/animation/test_composition.py @@ -128,7 +128,7 @@ def test_animationgroup_with_wait(): animation_group.begin() timings = animation_group.anims_with_timings - assert timings == [(wait, 0.0, 1.0), (sqr_anim, 1.0, 2.0)] + assert timings.tolist() == [(wait, 0.0, 1.0), (sqr_anim, 1.0, 2.0)] @pytest.mark.parametrize( diff --git a/tests/opengl/test_composition_opengl.py b/tests/opengl/test_composition_opengl.py index 1bedc3edcf..c09cd691a1 100644 --- a/tests/opengl/test_composition_opengl.py +++ b/tests/opengl/test_composition_opengl.py @@ -104,4 +104,4 @@ def test_animationgroup_with_wait(using_opengl_renderer): animation_group.begin() timings = animation_group.anims_with_timings - assert timings == [(wait, 0.0, 1.0), (sqr_anim, 1.0, 2.0)] + assert timings.tolist() == [(wait, 0.0, 1.0), (sqr_anim, 1.0, 2.0)] From 4e0321e1e02e65410665f851eabe992d4b08c511 Mon Sep 17 00:00:00 2001 From: chopan Date: Wed, 20 Dec 2023 23:11:48 +0100 Subject: [PATCH 05/29] Fixed final bugs --- manim/animation/composition.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/manim/animation/composition.py b/manim/animation/composition.py index 00e90fc556..ee3d1a1889 100644 --- a/manim/animation/composition.py +++ b/manim/animation/composition.py @@ -105,6 +105,8 @@ def _setup_scene(self, scene) -> None: def finish(self) -> None: for anim in self.animations: anim.finish() + self.interpolate(1) + self.ongoing_anim_bools[:] = False if self.suspend_mobject_updating: self.group.resume_updating() @@ -143,7 +145,7 @@ def build_animations_with_timings(self) -> None: run_times = np.array([anim.run_time for anim in self.animations]) num_animations = run_times.shape[0] dtype = [("anim", "O"), ("start", "f8"), ("end", "f8")] - self.anims_with_timings = np.empty(num_animations, dtype=dtype) + self.anims_with_timings = np.zeros(num_animations, dtype=dtype) self.ongoing_anim_bools = np.zeros(num_animations, dtype=bool) if num_animations == 0: return @@ -164,7 +166,7 @@ def interpolate(self, alpha: float) -> None: # Only update ongoing animations A = self.anims_with_timings - new_ongoing_anim_bools = (anim_time > A["start"]) & (anim_time < A["end"]) + new_ongoing_anim_bools = (anim_time >= A["start"]) & (anim_time <= A["end"]) A = A[self.ongoing_anim_bools | new_ongoing_anim_bools] run_times = A["end"] - A["start"] From aa161d82f8a318b270d286245c47830f739518db Mon Sep 17 00:00:00 2001 From: chopan Date: Wed, 20 Dec 2023 23:13:40 +0100 Subject: [PATCH 06/29] Removed accidental print --- manim/animation/composition.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/manim/animation/composition.py b/manim/animation/composition.py index ee3d1a1889..81f52a0ed5 100644 --- a/manim/animation/composition.py +++ b/manim/animation/composition.py @@ -181,9 +181,6 @@ def interpolate(self, alpha: float) -> None: for ongoing_anim, sub_alpha in zip(A["anim"], sub_alphas): ongoing_anim.interpolate(sub_alpha) - print(sub_alphas) - if alpha == 1.0: - print("A\n" * 10) self.anim_time = anim_time self.ongoing_anim_bools = new_ongoing_anim_bools From 3e1bc50c21bcd0bc2ec997b0f1c53520817e94c9 Mon Sep 17 00:00:00 2001 From: chopan Date: Wed, 20 Dec 2023 23:20:28 +0100 Subject: [PATCH 07/29] Final fix to AnimationGroup.interpolate --- manim/animation/composition.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/manim/animation/composition.py b/manim/animation/composition.py index 81f52a0ed5..c284845792 100644 --- a/manim/animation/composition.py +++ b/manim/animation/composition.py @@ -162,27 +162,24 @@ def interpolate(self, alpha: float) -> None: # e.g. of the surrounding scene. Instead they'd # be a rescaled version. But that's okay! anim_time = self.rate_func(alpha) * self.max_end_time - time_goes_back = anim_time <= self.anim_time # Only update ongoing animations A = self.anims_with_timings - new_ongoing_anim_bools = (anim_time >= A["start"]) & (anim_time <= A["end"]) - A = A[self.ongoing_anim_bools | new_ongoing_anim_bools] + new_ongoing = (anim_time >= A["start"]) & (anim_time <= A["end"]) + A = A[self.ongoing_anim_bools | new_ongoing] run_times = A["end"] - A["start"] null = run_times == 0.0 sub_alphas = anim_time - A["start"] sub_alphas[~null] /= run_times[~null] - if time_goes_back: - sub_alphas[null | (sub_alphas < 0)] = 0 - else: - sub_alphas[null | (sub_alphas > 1)] = 1 + sub_alphas[null | (sub_alphas < 0)] = 0 + sub_alphas[sub_alphas > 1] = 1 for ongoing_anim, sub_alpha in zip(A["anim"], sub_alphas): ongoing_anim.interpolate(sub_alpha) self.anim_time = anim_time - self.ongoing_anim_bools = new_ongoing_anim_bools + self.ongoing_anim_bools = new_ongoing class Succession(AnimationGroup): From 57fc7246335e73839c84a7cc7c20ebf0eb3758e8 Mon Sep 17 00:00:00 2001 From: chopan Date: Thu, 21 Dec 2023 17:21:40 +0100 Subject: [PATCH 08/29] Fixed animations being skipped unintentionally --- manim/animation/composition.py | 47 ++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/manim/animation/composition.py b/manim/animation/composition.py index c284845792..a8ec987be2 100644 --- a/manim/animation/composition.py +++ b/manim/animation/composition.py @@ -92,7 +92,7 @@ def begin(self) -> None: f"{self} has a run_time of 0 seconds, this cannot be " f"rendered correctly. {tmp}." ) - self.anim_time = 0.0 + self.anim_group_time = 0.0 if self.suspend_mobject_updating: self.group.suspend_updating() for anim in self.animations: @@ -103,10 +103,9 @@ def _setup_scene(self, scene) -> None: anim._setup_scene(scene) def finish(self) -> None: - for anim in self.animations: - anim.finish() self.interpolate(1) - self.ongoing_anim_bools[:] = False + self.anims_begun[:] = False + self.anims_finished[:] = False if self.suspend_mobject_updating: self.group.resume_updating() @@ -118,7 +117,9 @@ def clean_up_from_scene(self, scene: Scene) -> None: anim.clean_up_from_scene(scene) def update_mobjects(self, dt: float) -> None: - for anim in self.animations: + for anim in self.anims_with_timings["anim"][ + self.anims_begun & ~self.anims_finished + ]: anim.update_mobjects(dt) def init_run_time(self, run_time) -> float: @@ -146,7 +147,8 @@ def build_animations_with_timings(self) -> None: num_animations = run_times.shape[0] dtype = [("anim", "O"), ("start", "f8"), ("end", "f8")] self.anims_with_timings = np.zeros(num_animations, dtype=dtype) - self.ongoing_anim_bools = np.zeros(num_animations, dtype=bool) + self.anims_begun = np.zeros(num_animations, dtype=bool) + self.anims_finished = np.zeros(num_animations, dtype=bool) if num_animations == 0: return @@ -161,25 +163,32 @@ def interpolate(self, alpha: float) -> None: # times might not correspond to actual times, # e.g. of the surrounding scene. Instead they'd # be a rescaled version. But that's okay! - anim_time = self.rate_func(alpha) * self.max_end_time + anim_group_time = self.rate_func(alpha) * self.max_end_time + time_goes_back = anim_group_time < self.anim_group_time # Only update ongoing animations - A = self.anims_with_timings - new_ongoing = (anim_time >= A["start"]) & (anim_time <= A["end"]) - A = A[self.ongoing_anim_bools | new_ongoing] - - run_times = A["end"] - A["start"] + awt = self.anims_with_timings + new_begun = anim_group_time >= awt["start"] + new_finished = anim_group_time > awt["end"] + to_update = awt[ + (self.anims_begun | new_begun) & (~self.anims_finished | ~new_finished) + ] + + run_times = to_update["end"] - to_update["start"] null = run_times == 0.0 - sub_alphas = anim_time - A["start"] + sub_alphas = anim_group_time - to_update["start"] sub_alphas[~null] /= run_times[~null] - sub_alphas[null | (sub_alphas < 0)] = 0 - sub_alphas[sub_alphas > 1] = 1 + if time_goes_back: + sub_alphas[null | (sub_alphas < 0)] = 0 + else: + sub_alphas[null | (sub_alphas > 1)] = 1 - for ongoing_anim, sub_alpha in zip(A["anim"], sub_alphas): - ongoing_anim.interpolate(sub_alpha) + for anim_to_update, sub_alpha in zip(to_update["anim"], sub_alphas): + anim_to_update.interpolate(sub_alpha) - self.anim_time = anim_time - self.ongoing_anim_bools = new_ongoing + self.anim_group_time = anim_group_time + self.anims_begun = new_begun + self.anims_finished = new_finished class Succession(AnimationGroup): From a388df743d71a718600f7e0ca0a8edcd3eb82ab7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Wed, 1 May 2024 20:39:48 -0400 Subject: [PATCH 09/29] Port ManimGL's family handling --- manim/mobject/mobject.py | 90 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 86 insertions(+), 4 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 9127c2a1d6..e00e03934c 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -70,7 +70,16 @@ class Mobject: Attributes ---------- submobjects : List[:class:`Mobject`] - The contained objects. + The contained objects, or "children" of this Mobject. + parents : List[:class:`Mobject`] + The Mobjects which contain this as part of their submobjects. It is + important to have a backreference to the parents in case this Mobject's + family changes in order to notify them of that change, because this + Mobject is also a part of their families. + family : List[:class:`Mobject`] | None + An optional, memoized list containing this Mobject, its submobjects, + those submobjects' submobjects, and so on. If the family must be + recalculated for any reason, this attribute is set to None. points : :class:`numpy.ndarray` The points of the objects. @@ -107,6 +116,8 @@ def __init__( self.z_index = z_index self.point_hash = None self.submobjects = [] + self.parents: list[Mobject] = [] + self.family: list[Mobject] | None = [self] self.updaters: list[Updater] = [] self.updating_suspended = False self.color = ManimColor.parse(color) @@ -337,7 +348,10 @@ def __deepcopy__(self, clone_from_id) -> Self: result = cls.__new__(cls) clone_from_id[id(self)] = result for k, v in self.__dict__.items(): + if k == "parents": + continue setattr(result, k, copy.deepcopy(v, clone_from_id)) + result.parents = [] # Must be set manually because result has no attributes result.original_id = str(id(self)) return result @@ -446,6 +460,10 @@ def add(self, *mobjects: Mobject) -> Self: ) self.submobjects = list_update(self.submobjects, unique_mobjects) + for mobject in unique_mobjects: + if self not in mobject.parents: + mobject.parents.append(self) + self.note_changed_family() return self def insert(self, index: int, mobject: Mobject) -> None: @@ -467,7 +485,12 @@ def insert(self, index: int, mobject: Mobject) -> None: raise TypeError("All submobjects must be of type Mobject") if mobject is self: raise ValueError("Mobject cannot contain self") + # TODO: should verify that subsequent submobjects are not repeated self.submobjects.insert(index, mobject) + if self not in mobject.parents: + mobject.parents.append(self) + self.note_changed_family() + # TODO: should return Self instead of None? def __add__(self, mobject: Mobject): raise NotImplementedError @@ -529,6 +552,10 @@ def add_to_back(self, *mobjects: Mobject) -> Self: self.remove(*mobjects) # dict.fromkeys() removes duplicates while maintaining order self.submobjects = list(dict.fromkeys(mobjects)) + self.submobjects + for mobject in mobjects: + if self not in mobject.parents: + mobject.parents.append(self) + self.note_changed_family() return self def remove(self, *mobjects: Mobject) -> Self: @@ -556,6 +583,9 @@ def remove(self, *mobjects: Mobject) -> Self: for mobject in mobjects: if mobject in self.submobjects: self.submobjects.remove(mobject) + if self in mobject.parents: + mobject.parents.remove(self) + self.note_changed_family() return self def __sub__(self, other): @@ -2260,15 +2290,59 @@ def get_mobject_type_class() -> type[Mobject]: def split(self) -> list[Self]: result = [self] if len(self.points) > 0 else [] return result + self.submobjects + + def note_changed_family(self, only_changed_order=False) -> Self: + """Indicates that this Mobject's family should be recalculated, by + setting it to None to void the previous computation. If this Mobject + has parents, it is also part of their respective families, so they must + be notified as well. + + This method must be called after any change which involves modifying + some Mobject's submobjects, such as a call to Mobject.add. + """ + self.family = None + # TODO: Implement when bounding boxes and updater statuses are implemented + # if not only_changed_order: + # self.refresh_has_updater_status() + # self.refresh_bounding_box() + for parent in self.parents: + parent.note_changed_family() + return self def get_family(self, recurse: bool = True) -> list[Self]: - sub_families = [x.get_family() for x in self.submobjects] - all_mobjects = [self] + list(it.chain(*sub_families)) - return remove_list_redundancies(all_mobjects) + if not recurse: + return [self] + if self.family is None: + # Reconstruct and save + sub_families = (sm.get_family() for sm in self.submobjects) + family = [self, *it.chain(*sub_families)] + self.family = remove_list_redundancies(family) + return self.family def family_members_with_points(self) -> list[Self]: return [m for m in self.get_family() if m.get_num_points() > 0] + def get_ancestors(self, extended: bool = False) -> list[Mobject]: + """ + Returns parents, grandparents, etc. + Order of result should be from higher members of the hierarchy down. + + If extended is set to true, it includes the ancestors of all family members, + e.g. any other parents of a submobject + """ + ancestors = [] + to_process = list(self.get_family(recurse=extended)) + excluded = set(to_process) + while to_process: + for p in to_process.pop().parents: + if p not in excluded: + ancestors.append(p) + to_process.append(p) + # Ensure mobjects highest in the hierarchy show up first + ancestors.reverse() + # Remove list redundancies while preserving order + return list(dict.fromkeys(ancestors)) + def arrange( self, direction: Vector3D = RIGHT, @@ -2552,6 +2626,7 @@ def submob_func(m: Mobject): return point_to_num_func(m.get_center()) self.submobjects.sort(key=submob_func) + self.note_changed_family(only_changed_order=True) return self def shuffle(self, recursive: bool = False) -> None: @@ -2560,6 +2635,8 @@ def shuffle(self, recursive: bool = False) -> None: for submob in self.submobjects: submob.shuffle(recursive=True) random.shuffle(self.submobjects) + self.note_changed_family(only_changed_order=True) + # TODO: should return Self instead of None? def invert(self, recursive: bool = False) -> None: """Inverts the list of :attr:`submobjects`. @@ -2586,6 +2663,8 @@ def construct(self): for submob in self.submobjects: submob.invert(recursive=True) self.submobjects.reverse() + self.note_changed_family(only_changed_order=True) + # TODO: should return Self instead of None? # Just here to keep from breaking old scenes. def arrange_submobjects(self, *args, **kwargs) -> Self: @@ -2715,6 +2794,8 @@ def add_n_more_submobjects(self, n: int) -> Self | None: if curr == 0: # If empty, simply add n point mobjects self.submobjects = [self.get_point_mobject() for k in range(n)] + self.note_changed_family() + # TODO: shouldn't this return Self instead? return None target = curr + n @@ -2728,6 +2809,7 @@ def add_n_more_submobjects(self, n: int) -> Self | None: for _ in range(1, sf): new_submobs.append(submob.copy().fade(1)) self.submobjects = new_submobs + self.note_changed_family() return self def repeat_submobject(self, submob: Mobject) -> Self: From fc04be4a1d824c80a4d0745545e7caa81210ebfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Wed, 1 May 2024 23:10:28 -0400 Subject: [PATCH 10/29] Make Mobject.submobjects a property --- manim/mobject/mobject.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index e00e03934c..697f16ea8f 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -115,7 +115,7 @@ def __init__( self.target = target self.z_index = z_index self.point_hash = None - self.submobjects = [] + self._submobjects = [] self.parents: list[Mobject] = [] self.family: list[Mobject] | None = [self] self.updaters: list[Updater] = [] @@ -126,6 +126,15 @@ def __init__( self.generate_points() self.init_colors() + @property + def submobjects(self) -> list[Mobject]: + return self._submobjects + + @submobjects.setter + def submobjects(self, new_submobjects) -> None: + self._submobjects = new_submobjects + self.note_changed_family() + @classmethod def animation_override_for( cls, From 42420de85f64e7725ad23eaa1aabb1c523b3d780 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Wed, 1 May 2024 23:23:46 -0400 Subject: [PATCH 11/29] Move assignation of parents to the middle of the for loop in __deepcopy__ --- manim/mobject/mobject.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 697f16ea8f..69cb27e4f1 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -357,10 +357,13 @@ def __deepcopy__(self, clone_from_id) -> Self: result = cls.__new__(cls) clone_from_id[id(self)] = result for k, v in self.__dict__.items(): + # This must be set manually because result has no attributes, + # and specifically INSIDE the loop to preserve attribute order, + # or test_hash_consistency() will fail! if k == "parents": + result.parents = [] continue setattr(result, k, copy.deepcopy(v, clone_from_id)) - result.parents = [] # Must be set manually because result has no attributes result.original_id = str(id(self)) return result From c5598a5ff6f21425efb34d44b503391e9fdf0575 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 2 May 2024 12:49:41 +0000 Subject: [PATCH 12/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- manim/mobject/mobject.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 69cb27e4f1..944d1c917e 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -2302,13 +2302,13 @@ def get_mobject_type_class() -> type[Mobject]: def split(self) -> list[Self]: result = [self] if len(self.points) > 0 else [] return result + self.submobjects - + def note_changed_family(self, only_changed_order=False) -> Self: """Indicates that this Mobject's family should be recalculated, by setting it to None to void the previous computation. If this Mobject has parents, it is also part of their respective families, so they must be notified as well. - + This method must be called after any change which involves modifying some Mobject's submobjects, such as a call to Mobject.add. """ From ccfffbfb081be42923ae18fc5bfaa6c3017c0434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Fri, 10 May 2024 22:25:24 -0400 Subject: [PATCH 13/29] Implement requested changes and an _assert_valid_submobjects() method --- manim/mobject/mobject.py | 37 +++++++++++------------ manim/mobject/types/vectorized_mobject.py | 6 +++- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 944d1c917e..56e0a5d953 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -115,7 +115,7 @@ def __init__( self.target = target self.z_index = z_index self.point_hash = None - self._submobjects = [] + self._submobjects: list[Mobject] = [] self.parents: list[Mobject] = [] self.family: list[Mobject] | None = [self] self.updaters: list[Updater] = [] @@ -126,12 +126,23 @@ def __init__( self.generate_points() self.init_colors() + def _assert_valid_submobjects(self, submobjects: list[Mobject]): + self._assert_valid_submobjects_internal(submobjects, Mobject) + + def _assert_valid_submobjects_internal(self, submobjects: list[Mobject], mob_class: type): + for submob in submobjects: + if not isinstance(submob, mob_class): + raise TypeError(f"All submobjects must be of type {mob_class.__name__}") + if submob is self: + raise ValueError("Mobject cannot contain self") + @property def submobjects(self) -> list[Mobject]: return self._submobjects @submobjects.setter - def submobjects(self, new_submobjects) -> None: + def submobjects(self, new_submobjects: list[Mobject]) -> None: + self._assert_valid_submobjects(new_submobjects) self._submobjects = new_submobjects self.note_changed_family() @@ -458,12 +469,7 @@ def add(self, *mobjects: Mobject) -> Self: [child] """ - for m in mobjects: - if not isinstance(m, Mobject): - raise TypeError("All submobjects must be of type Mobject") - if m is self: - raise ValueError("Mobject cannot contain self") - + self._assert_valid_submobjects(mobjects) unique_mobjects = remove_list_redundancies(mobjects) if len(mobjects) != len(unique_mobjects): logger.warning( @@ -493,10 +499,7 @@ def insert(self, index: int, mobject: Mobject) -> None: mobject The mobject to be inserted. """ - if not isinstance(mobject, Mobject): - raise TypeError("All submobjects must be of type Mobject") - if mobject is self: - raise ValueError("Mobject cannot contain self") + self._assert_valid_submobjects([mobject]) # TODO: should verify that subsequent submobjects are not repeated self.submobjects.insert(index, mobject) if self not in mobject.parents: @@ -554,13 +557,7 @@ def add_to_back(self, *mobjects: Mobject) -> Self: :meth:`add` """ - if self in mobjects: - raise ValueError("A mobject shouldn't contain itself") - - for mobject in mobjects: - if not isinstance(mobject, Mobject): - raise TypeError("All submobjects must be of type Mobject") - + self._assert_valid_submobjects(mobjects) self.remove(*mobjects) # dict.fromkeys() removes duplicates while maintaining order self.submobjects = list(dict.fromkeys(mobjects)) + self.submobjects @@ -2343,7 +2340,7 @@ def get_ancestors(self, extended: bool = False) -> list[Mobject]: e.g. any other parents of a submobject """ ancestors = [] - to_process = list(self.get_family(recurse=extended)) + to_process = self.get_family(recurse=extended).copy() excluded = set(to_process) while to_process: for p in to_process.pop().parents: diff --git a/manim/mobject/types/vectorized_mobject.py b/manim/mobject/types/vectorized_mobject.py index 6a2fffd2cb..b440557473 100644 --- a/manim/mobject/types/vectorized_mobject.py +++ b/manim/mobject/types/vectorized_mobject.py @@ -168,7 +168,8 @@ def __init__( self.n_points_per_cubic_curve: int = n_points_per_cubic_curve self.cap_style: CapStyleType = cap_style super().__init__(**kwargs) - self.submobjects: list[VMobject] + self._submobjects: list[VMobject] + self.family: list[VMobject] | None # TODO: Find where color overwrites are happening and remove the color doubling # if "color" in kwargs: @@ -179,6 +180,9 @@ def __init__( if stroke_color is not None: self.stroke_color = ManimColor.parse(stroke_color) + def _assert_valid_submobjects(self, submobjects: list[VMobject]): + self._assert_valid_submobjects_internal(submobjects, VMobject) + # OpenGL compatibility @property def n_points_per_curve(self) -> int: From 17f30b7777464ed0c46c7371fa3be412e49f9786 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 11 May 2024 02:25:12 +0000 Subject: [PATCH 14/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- manim/mobject/mobject.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 56e0a5d953..622bc80f5f 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -129,13 +129,15 @@ def __init__( def _assert_valid_submobjects(self, submobjects: list[Mobject]): self._assert_valid_submobjects_internal(submobjects, Mobject) - def _assert_valid_submobjects_internal(self, submobjects: list[Mobject], mob_class: type): + def _assert_valid_submobjects_internal( + self, submobjects: list[Mobject], mob_class: type + ): for submob in submobjects: if not isinstance(submob, mob_class): raise TypeError(f"All submobjects must be of type {mob_class.__name__}") if submob is self: raise ValueError("Mobject cannot contain self") - + @property def submobjects(self) -> list[Mobject]: return self._submobjects From df2a3c8714a66a9191077d99e94a6ee936812132 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Sat, 11 May 2024 19:48:20 -0400 Subject: [PATCH 15/29] Added docstrings to get_family and _assert_valid_submobjects --- manim/mobject/mobject.py | 74 ++++++++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 11 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 622bc80f5f..442d5468ee 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -22,9 +22,9 @@ from manim.mobject.opengl.opengl_compatibility import ConvertToOpenGL -from .. import config, logger -from ..constants import * -from ..utils.color import ( +from manim import config, logger +from manim.constants import * +from manim.utils.color import ( BLACK, WHITE, YELLOW_C, @@ -33,10 +33,10 @@ color_gradient, interpolate_color, ) -from ..utils.exceptions import MultiAnimationOverrideException -from ..utils.iterables import list_update, remove_list_redundancies -from ..utils.paths import straight_path -from ..utils.space_ops import angle_between_vectors, normalize, rotation_matrix +from manim.utils.exceptions import MultiAnimationOverrideException +from manim.utils.iterables import list_update, remove_list_redundancies, tuplify +from manim.utils.paths import straight_path +from manim.utils.space_ops import angle_between_vectors, normalize, rotation_matrix if TYPE_CHECKING: from typing_extensions import Self, TypeAlias @@ -53,7 +53,7 @@ Vector3D, ) - from ..animation.animation import Animation + from manim.animation.animation import Animation TimeBasedUpdater: TypeAlias = Callable[["Mobject", float], object] NonTimeBasedUpdater: TypeAlias = Callable[["Mobject"], object] @@ -126,7 +126,27 @@ def __init__( self.generate_points() self.init_colors() - def _assert_valid_submobjects(self, submobjects: list[Mobject]): + def _assert_valid_submobjects(self, submobjects: list[Mobject]) -> None: + """Check that all submobjects are actually values of type Mobject, and + that none of them is ``self`` (a :class:`Mobject` cannot contain + itself). + + This is an auxiliary function called when adding Mobjects to the + :attr:`submobjects` list. + + Parameters + ---------- + submobjects + The list containing values which should be Mobjects. + + Raises + ------ + TypeError + If any of the values in `submobjects` is not a :class:`Mobject`. + ValueError + If there was an attempt to add a :class:`Mobject` as its own + submobject. + """ self._assert_valid_submobjects_internal(submobjects, Mobject) def _assert_valid_submobjects_internal( @@ -2310,6 +2330,20 @@ def note_changed_family(self, only_changed_order=False) -> Self: This method must be called after any change which involves modifying some Mobject's submobjects, such as a call to Mobject.add. + + Parameters + ---------- + only_changed_order + If True, indicate that the family still contains the same Mobjects, + only in a different order. This prevents recalculating bounding + boxes and updater statuses, because they remain the same. If False, + indicate that some Mobjects were added or removed to the family, + and trigger the aforementioned recalculations. Default is False. + + Returns + ------- + :class:`Mobject` + The Mobject itself. """ self.family = None # TODO: Implement when bounding boxes and updater statuses are implemented @@ -2317,10 +2351,28 @@ def note_changed_family(self, only_changed_order=False) -> Self: # self.refresh_has_updater_status() # self.refresh_bounding_box() for parent in self.parents: - parent.note_changed_family() + parent.note_changed_family(only_changed_order) return self def get_family(self, recurse: bool = True) -> list[Self]: + """Obtain the family of this Mobject, consisting of itself, its + submobjects, the submobjects' submobjects, and so on. If the + family was calculated previously and memoized into the :attr:`family` + attribute as a list, return that list. Otherwise, if the attribute is + None, calculate and memoize it now. + + Parameters + ---------- + recurse + If True, explore this Mobject's submobjects and so on, to + compute the full family. Otherwise, stop at this Mobject and + return a list containing only this one. Default is True. + + Returns + ------- + list[:class:`Mobject`] + The family of this Mobject. + """ if not recurse: return [self] if self.family is None: @@ -2339,7 +2391,7 @@ def get_ancestors(self, extended: bool = False) -> list[Mobject]: Order of result should be from higher members of the hierarchy down. If extended is set to true, it includes the ancestors of all family members, - e.g. any other parents of a submobject + e.g. any other parents of a submobject. """ ancestors = [] to_process = self.get_family(recurse=extended).copy() From ac296f898ca118805484465500c11813fda0a5af Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 11 May 2024 23:47:50 +0000 Subject: [PATCH 16/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- manim/mobject/mobject.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 442d5468ee..eb86fbe5f4 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -20,10 +20,9 @@ import numpy as np -from manim.mobject.opengl.opengl_compatibility import ConvertToOpenGL - from manim import config, logger from manim.constants import * +from manim.mobject.opengl.opengl_compatibility import ConvertToOpenGL from manim.utils.color import ( BLACK, WHITE, @@ -41,6 +40,7 @@ if TYPE_CHECKING: from typing_extensions import Self, TypeAlias + from manim.animation.animation import Animation from manim.typing import ( FunctionOverride, Image, @@ -53,8 +53,6 @@ Vector3D, ) - from manim.animation.animation import Animation - TimeBasedUpdater: TypeAlias = Callable[["Mobject", float], object] NonTimeBasedUpdater: TypeAlias = Callable[["Mobject"], object] Updater: TypeAlias = NonTimeBasedUpdater | TimeBasedUpdater @@ -2360,14 +2358,14 @@ def get_family(self, recurse: bool = True) -> list[Self]: family was calculated previously and memoized into the :attr:`family` attribute as a list, return that list. Otherwise, if the attribute is None, calculate and memoize it now. - + Parameters ---------- recurse If True, explore this Mobject's submobjects and so on, to compute the full family. Otherwise, stop at this Mobject and return a list containing only this one. Default is True. - + Returns ------- list[:class:`Mobject`] From 17d168122bd53bb93a85752ad962a10ab40e7be1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Mon, 20 May 2024 16:55:43 -0400 Subject: [PATCH 17/29] Add self as parents in self.submobjects.setter --- manim/mobject/mobject.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 4f545e6f93..467656041e 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -165,6 +165,9 @@ def submobjects(self) -> list[Mobject]: def submobjects(self, new_submobjects: list[Mobject]) -> None: self._assert_valid_submobjects(new_submobjects) self._submobjects = new_submobjects + for submob in new_submobjects: + if self not in submob.parents: + submob.parents.append(self) self.note_changed_family() @classmethod From ac7643368b306cdd4f593eba06ea4d73d721c7d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Wed, 29 May 2024 21:51:28 -0400 Subject: [PATCH 18/29] Remove duplicated assertion in Mobject.add() --- manim/mobject/mobject.py | 1 - 1 file changed, 1 deletion(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index b26392c8e3..41f5d172c4 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -526,7 +526,6 @@ def add(self, *mobjects: Mobject) -> Self: """ self._assert_valid_submobjects(mobjects) - self._assert_valid_submobjects(mobjects) unique_mobjects = remove_list_redundancies(mobjects) if len(mobjects) != len(unique_mobjects): logger.warning( From 2ababf45835f137f0c76aff5bbd043f1d3fe0216 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Thu, 30 May 2024 09:19:55 -0400 Subject: [PATCH 19/29] Skip _family attribute when JSON encoding with manim.utils.hashing._CustomEncoder --- manim/mobject/mobject.py | 24 ++++++++++++++++------- manim/mobject/types/vectorized_mobject.py | 2 +- manim/utils/hashing.py | 7 +++++++ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 41f5d172c4..aa823d3d32 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -114,9 +114,12 @@ def __init__( self.target = target self.z_index = z_index self.point_hash = None - self._submobjects: list[Mobject] = [] + # NOTE: the parents, _submobjects and _family attributes MUST BE + # IN THIS ORDER! Otherwise, Mobject.__deepcopy__() will break or + # generate a Mobject with a different hash! self.parents: list[Mobject] = [] - self.family: list[Mobject] | None = [self] + self._submobjects: list[Mobject] = [] + self._family: list[Mobject] | None = None self.updaters: list[Updater] = [] self.updating_suspended = False self.color = ManimColor.parse(color) @@ -423,7 +426,14 @@ def __deepcopy__(self, clone_from_id) -> Self: if k == "parents": result.parents = [] continue - setattr(result, k, copy.deepcopy(v, clone_from_id)) + if k == "_submobjects": + # Call the submobjects property which calculates parents + result._submobjects = [] + result.submobjects = copy.deepcopy(v, clone_from_id) + elif k == "_family": + result._family = None + else: + setattr(result, k, copy.deepcopy(v, clone_from_id)) result.original_id = str(id(self)) return result @@ -2379,7 +2389,7 @@ def note_changed_family(self, only_changed_order=False) -> Self: :class:`Mobject` The Mobject itself. """ - self.family = None + self._family = None # TODO: Implement when bounding boxes and updater statuses are implemented # if not only_changed_order: # self.refresh_has_updater_status() @@ -2409,12 +2419,12 @@ def get_family(self, recurse: bool = True) -> list[Self]: """ if not recurse: return [self] - if self.family is None: + if self._family is None: # Reconstruct and save sub_families = (sm.get_family() for sm in self.submobjects) family = [self, *it.chain(*sub_families)] - self.family = remove_list_redundancies(family) - return self.family + self._family = remove_list_redundancies(family) + return self._family def family_members_with_points(self) -> list[Self]: return [m for m in self.get_family() if m.get_num_points() > 0] diff --git a/manim/mobject/types/vectorized_mobject.py b/manim/mobject/types/vectorized_mobject.py index c9159069fa..fc0f9941a4 100644 --- a/manim/mobject/types/vectorized_mobject.py +++ b/manim/mobject/types/vectorized_mobject.py @@ -165,7 +165,7 @@ def __init__( self.cap_style: CapStyleType = cap_style super().__init__(**kwargs) self._submobjects: list[VMobject] - self.family: list[VMobject] | None + self._family: list[VMobject] | None # TODO: Find where color overwrites are happening and remove the color doubling # if "color" in kwargs: diff --git a/manim/utils/hashing.py b/manim/utils/hashing.py index 9557ba18f1..c9a20b1b40 100644 --- a/manim/utils/hashing.py +++ b/manim/utils/hashing.py @@ -301,6 +301,13 @@ def encode(self, obj: Any): The object encoder with the standard json process. """ _Memoizer.mark_as_processed(obj) + if isinstance(obj, Mobject): + # Skip the _family attribute when encoding + family = obj._family + obj._family = None + encoded = super().encode(obj) + obj._family = family + return encoded if isinstance(obj, (dict, list, tuple)): return super().encode(self._cleaned_iterable(obj)) return super().encode(obj) From 9f30e3309aa2ad89052ea8fabf9ab3368389fc47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Thu, 30 May 2024 09:30:05 -0400 Subject: [PATCH 20/29] Remove unused import of manim.utils.iterables.tuplify --- manim/mobject/mobject.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index aa823d3d32..3d7e25183d 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -34,7 +34,7 @@ interpolate_color, ) from manim.utils.exceptions import MultiAnimationOverrideException -from manim.utils.iterables import list_update, remove_list_redundancies, tuplify +from manim.utils.iterables import list_update, remove_list_redundancies from manim.utils.paths import straight_path from manim.utils.space_ops import angle_between_vectors, normalize, rotation_matrix From 71a4beac94701f3f570d9b3efadab93e622d3628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Mon, 10 Jun 2024 21:39:17 -0400 Subject: [PATCH 21/29] Make parents a property and add Mobject._add_as_parent_of_submobs() method --- manim/mobject/mobject.py | 60 +++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 3d7e25183d..81b27fd0b2 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -117,7 +117,7 @@ def __init__( # NOTE: the parents, _submobjects and _family attributes MUST BE # IN THIS ORDER! Otherwise, Mobject.__deepcopy__() will break or # generate a Mobject with a different hash! - self.parents: list[Mobject] = [] + self._parents: list[Mobject] = [] self._submobjects: list[Mobject] = [] self._family: list[Mobject] | None = None self.updaters: list[Updater] = [] @@ -193,11 +193,13 @@ def submobjects(self) -> list[Mobject]: def submobjects(self, new_submobjects: list[Mobject]) -> None: self._assert_valid_submobjects(new_submobjects) self._submobjects = new_submobjects - for submob in new_submobjects: - if self not in submob.parents: - submob.parents.append(self) + self._add_as_parent_of_submobs(new_submobjects) self.note_changed_family() + @property + def parents(self) -> list[Mobject]: + return self._parents + @classmethod def animation_override_for( cls, @@ -423,13 +425,12 @@ def __deepcopy__(self, clone_from_id) -> Self: # This must be set manually because result has no attributes, # and specifically INSIDE the loop to preserve attribute order, # or test_hash_consistency() will fail! - if k == "parents": - result.parents = [] + if k == "_parents": + result._parents = [] continue if k == "_submobjects": - # Call the submobjects property which calculates parents - result._submobjects = [] - result.submobjects = copy.deepcopy(v, clone_from_id) + result._submobjects = copy.deepcopy(v, clone_from_id) + result._add_as_parent_of_submobs(result._submobjects) elif k == "_family": result._family = None else: @@ -458,6 +459,12 @@ def generate_points(self) -> None: subclasses. """ + def _add_as_parent_of_submobs(self, mobjects: Iterable[Mobject]) -> Self: + for mobject in mobjects: + if self not in mobject._parents: + mobject._parents.append(self) + return self + def add(self, *mobjects: Mobject) -> Self: """Add mobjects as submobjects. @@ -543,10 +550,8 @@ def add(self, *mobjects: Mobject) -> Self: "this is not possible. Repetitions are ignored.", ) - self.submobjects = list_update(self.submobjects, unique_mobjects) - for mobject in unique_mobjects: - if self not in mobject.parents: - mobject.parents.append(self) + self._submobjects = list_update(self._submobjects, unique_mobjects) + self._add_as_parent_of_submobs(mobjects) self.note_changed_family() return self @@ -567,9 +572,8 @@ def insert(self, index: int, mobject: Mobject) -> None: """ self._assert_valid_submobjects([mobject]) # TODO: should verify that subsequent submobjects are not repeated - self.submobjects.insert(index, mobject) - if self not in mobject.parents: - mobject.parents.append(self) + self._submobjects.insert(index, mobject) + self._add_as_parent_of_submobs([mobject]) self.note_changed_family() # TODO: should return Self instead of None? @@ -626,10 +630,8 @@ def add_to_back(self, *mobjects: Mobject) -> Self: self._assert_valid_submobjects(mobjects) self.remove(*mobjects) # dict.fromkeys() removes duplicates while maintaining order - self.submobjects = list(dict.fromkeys(mobjects)) + self.submobjects - for mobject in mobjects: - if self not in mobject.parents: - mobject.parents.append(self) + self._submobjects = list(dict.fromkeys(mobjects)) + self._submobjects + self._add_as_parent_of_submobs(mobjects) self.note_changed_family() return self @@ -656,10 +658,10 @@ def remove(self, *mobjects: Mobject) -> Self: """ for mobject in mobjects: - if mobject in self.submobjects: - self.submobjects.remove(mobject) - if self in mobject.parents: - mobject.parents.remove(self) + if mobject in self._submobjects: + self._submobjects.remove(mobject) + if self in mobject._parents: + mobject._parents.remove(self) self.note_changed_family() return self @@ -2394,7 +2396,7 @@ def note_changed_family(self, only_changed_order=False) -> Self: # if not only_changed_order: # self.refresh_has_updater_status() # self.refresh_bounding_box() - for parent in self.parents: + for parent in self._parents: parent.note_changed_family(only_changed_order) return self @@ -2732,7 +2734,7 @@ def sort( def submob_func(m: Mobject): return point_to_num_func(m.get_center()) - self.submobjects.sort(key=submob_func) + self._submobjects.sort(key=submob_func) self.note_changed_family(only_changed_order=True) return self @@ -2741,7 +2743,7 @@ def shuffle(self, recursive: bool = False) -> None: if recursive: for submob in self.submobjects: submob.shuffle(recursive=True) - random.shuffle(self.submobjects) + random.shuffle(self._submobjects) self.note_changed_family(only_changed_order=True) # TODO: should return Self instead of None? @@ -2769,7 +2771,7 @@ def construct(self): if recursive: for submob in self.submobjects: submob.invert(recursive=True) - self.submobjects.reverse() + self._submobjects.reverse() self.note_changed_family(only_changed_order=True) # TODO: should return Self instead of None? @@ -2888,7 +2890,7 @@ def null_point_align(self, mobject: Mobject): def push_self_into_submobjects(self) -> Self: copy = self.copy() - copy.submobjects = [] + copy._submobjects = [] self.reset_points() self.add(copy) return self From a46c6ed22aa6074d85bfa490dfee2462738d1db8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Mon, 10 Jun 2024 21:40:13 -0400 Subject: [PATCH 22/29] parents -> _parents --- manim/mobject/mobject.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 81b27fd0b2..c321d485fa 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -114,7 +114,7 @@ def __init__( self.target = target self.z_index = z_index self.point_hash = None - # NOTE: the parents, _submobjects and _family attributes MUST BE + # NOTE: the _parents, _submobjects and _family attributes MUST BE # IN THIS ORDER! Otherwise, Mobject.__deepcopy__() will break or # generate a Mobject with a different hash! self._parents: list[Mobject] = [] From 232b2f68f99b6116e0ca357a5ace3ef243fef8ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Mon, 10 Jun 2024 21:41:22 -0400 Subject: [PATCH 23/29] Add family property --- manim/mobject/mobject.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index c321d485fa..c53a9d8895 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -200,6 +200,10 @@ def submobjects(self, new_submobjects: list[Mobject]) -> None: def parents(self) -> list[Mobject]: return self._parents + @property + def family(self) -> list[Mobject]: + return self.get_family() + @classmethod def animation_override_for( cls, From c569304e2094151771935839a94c07b72b19c543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Mon, 15 Jul 2024 09:29:21 -0400 Subject: [PATCH 24/29] Apply requested changes --- manim/mobject/mobject.py | 104 ++++++++++-------- manim/mobject/opengl/opengl_mobject.py | 16 +-- manim/mobject/opengl/opengl_surface.py | 2 +- .../opengl/opengl_vectorized_mobject.py | 10 +- 4 files changed, 74 insertions(+), 58 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index c53a9d8895..2c5da3ef9a 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -14,7 +14,7 @@ import sys import types import warnings -from collections.abc import Iterable +from collections.abc import Iterable, Sequence from functools import partialmethod, reduce from pathlib import Path from typing import TYPE_CHECKING, Callable, Literal @@ -103,7 +103,7 @@ def __init_subclass__(cls, **kwargs) -> None: def __init__( self, - color: ParsableManimColor | list[ParsableManimColor] = WHITE, + color: ParsableManimColor | Sequence[ParsableManimColor] = WHITE, name: str | None = None, dim: int = 3, target=None, @@ -186,22 +186,27 @@ def _assert_valid_submobjects_internal( return self @property - def submobjects(self) -> list[Mobject]: + def submobjects(self) -> Sequence[Mobject]: return self._submobjects + # TODO: remove or modify this to prevent users from directly setting submobjects @submobjects.setter - def submobjects(self, new_submobjects: list[Mobject]) -> None: + def submobjects(self, new_submobjects: Iterable[Mobject]) -> None: + self.set_submobjects(new_submobjects) + + def set_submobjects(self, new_submobjects: Iterable[Mobject]) -> Self: self._assert_valid_submobjects(new_submobjects) - self._submobjects = new_submobjects - self._add_as_parent_of_submobs(new_submobjects) + self.remove(*self._submobjects) + self.add(*new_submobjects) self.note_changed_family() + return self @property - def parents(self) -> list[Mobject]: + def parents(self) -> Sequence[Mobject]: return self._parents @property - def family(self) -> list[Mobject]: + def family(self) -> Sequence[Mobject]: return self.get_family() @classmethod @@ -559,7 +564,7 @@ def add(self, *mobjects: Mobject) -> Self: self.note_changed_family() return self - def insert(self, index: int, mobject: Mobject) -> None: + def insert(self, index: int, mobject: Mobject) -> Self: """Inserts a mobject at a specific position into self.submobjects Effectively just calls ``self.submobjects.insert(index, mobject)``, @@ -579,7 +584,7 @@ def insert(self, index: int, mobject: Mobject) -> None: self._submobjects.insert(index, mobject) self._add_as_parent_of_submobs([mobject]) self.note_changed_family() - # TODO: should return Self instead of None? + return self def __add__(self, mobject: Mobject): raise NotImplementedError @@ -875,7 +880,7 @@ def depth(self, value: float): self.scale_to_fit_depth(value) # Can't be staticmethod because of point_cloud_mobject.py - def get_array_attrs(self) -> list[Literal["points"]]: + def get_array_attrs(self) -> Sequence[Literal["points"]]: return ["points"] def apply_over_attr_arrays(self, func: MappingFunction) -> Self: @@ -964,7 +969,7 @@ def update(self, dt: float = 0, recursive: bool = True) -> Self: submob.update(dt, recursive) return self - def get_time_based_updaters(self) -> list[TimeBasedUpdater]: + def get_time_based_updaters(self) -> Sequence[TimeBasedUpdater]: """Return all updaters using the ``dt`` parameter. The updaters use this parameter as the input for difference in time. @@ -1004,7 +1009,7 @@ def has_time_based_updater(self) -> bool: "dt" in inspect.signature(updater).parameters for updater in self.updaters ) - def get_updaters(self) -> list[Updater]: + def get_updaters(self) -> Sequence[Updater]: """Return all updaters. Returns @@ -1020,7 +1025,7 @@ def get_updaters(self) -> list[Updater]: """ return self.updaters - def get_family_updaters(self) -> list[Updater]: + def get_family_updaters(self) -> Sequence[Updater]: return list(it.chain(*(sm.get_updaters() for sm in self.get_family()))) def add_updater( @@ -2070,7 +2075,7 @@ def reduce_across_dimension(self, reduce_func: Callable, dim: int): rv = reduce_func([value, rv]) return rv - def nonempty_submobjects(self) -> list[Self]: + def nonempty_submobjects(self) -> Sequence[Mobject]: return [ submob for submob in self.submobjects @@ -2368,11 +2373,11 @@ def get_mobject_type_class() -> type[Mobject]: """Return the base class of this mobject type.""" return Mobject - def split(self) -> list[Self]: + def split(self) -> Sequence[Mobject]: result = [self] if len(self.points) > 0 else [] - return result + self.submobjects + return result + self._submobjects - def note_changed_family(self, only_changed_order=False) -> Self: + def note_changed_family(self, *, only_changed_order=False) -> Self: """Indicates that this Mobject's family should be recalculated, by setting it to None to void the previous computation. If this Mobject has parents, it is also part of their respective families, so they must @@ -2401,10 +2406,10 @@ def note_changed_family(self, only_changed_order=False) -> Self: # self.refresh_has_updater_status() # self.refresh_bounding_box() for parent in self._parents: - parent.note_changed_family(only_changed_order) + parent.note_changed_family(only_changed_order=only_changed_order) return self - def get_family(self, recurse: bool = True) -> list[Self]: + def get_family(self, *, recurse: bool = True) -> Sequence[Mobject]: """Obtain the family of this Mobject, consisting of itself, its submobjects, the submobjects' submobjects, and so on. If the family was calculated previously and memoized into the :attr:`family` @@ -2432,16 +2437,28 @@ def get_family(self, recurse: bool = True) -> list[Self]: self._family = remove_list_redundancies(family) return self._family - def family_members_with_points(self) -> list[Self]: + def family_members_with_points(self) -> Sequence[Mobject]: return [m for m in self.get_family() if m.get_num_points() > 0] - def get_ancestors(self, extended: bool = False) -> list[Mobject]: - """ - Returns parents, grandparents, etc. - Order of result should be from higher members of the hierarchy down. + def get_ancestors(self, *, extended: bool = False) -> Sequence[Mobject]: + """Returns the parents, grandparents, etc. of this :class:`Mobject`. + The order of the result is top-down: from the highest members of the + hierarchy, down to the parents of this Mobjects. + + If ``extended`` is set to ``True``, it includes the ancestors of all + family members, e.g. any other parents of a submobject. - If extended is set to true, it includes the ancestors of all family members, - e.g. any other parents of a submobject. + Parameters + ---------- + extended + Whether to also include the ancestors of all the family members + of this Mobject or not. Defaults to ``False``. + + Returns + ------- + list[:class:`Mobject`] + The ancestors of this Mobject, and possibly also the ancestors + of this Mobject's family members. """ ancestors = [] to_process = self.get_family(recurse=extended).copy() @@ -2587,7 +2604,7 @@ def construct(self): """ from manim.mobject.geometry.line import Line - mobs = self.submobjects.copy() + mobs = self._submobjects.copy() start_pos = self.get_center() # get cols / rows values if given (implicitly) @@ -2742,16 +2759,16 @@ def submob_func(m: Mobject): self.note_changed_family(only_changed_order=True) return self - def shuffle(self, recursive: bool = False) -> None: + def shuffle(self, recursive: bool = False) -> Self: """Shuffles the list of :attr:`submobjects`.""" if recursive: - for submob in self.submobjects: + for submob in self._submobjects: submob.shuffle(recursive=True) random.shuffle(self._submobjects) self.note_changed_family(only_changed_order=True) - # TODO: should return Self instead of None? + return self - def invert(self, recursive: bool = False) -> None: + def invert(self, recursive: bool = False) -> Self: """Inverts the list of :attr:`submobjects`. Parameters @@ -2773,11 +2790,11 @@ def construct(self): self.play(Write(s), Write(s2)) """ if recursive: - for submob in self.submobjects: + for submob in self._submobjects: submob.invert(recursive=True) self._submobjects.reverse() self.note_changed_family(only_changed_order=True) - # TODO: should return Self instead of None? + return self # Just here to keep from breaking old scenes. def arrange_submobjects(self, *args, **kwargs) -> Self: @@ -2805,7 +2822,7 @@ def sort_submobjects(self, *args, **kwargs) -> Self: """Sort the :attr:`submobjects`""" return self.sort(*args, **kwargs) - def shuffle_submobjects(self, *args, **kwargs) -> None: + def shuffle_submobjects(self, *args, **kwargs) -> Self: """Shuffles the order of :attr:`submobjects` Examples @@ -2845,7 +2862,7 @@ def align_data(self, mobject: Mobject, skip_point_alignment: bool = False) -> No if not skip_point_alignment: self.align_points(mobject) # Recurse - for m1, m2 in zip(self.submobjects, mobject.submobjects): + for m1, m2 in zip(self._submobjects, mobject._submobjects): m1.align_data(m2) def get_point_mobject(self, center=None): @@ -2870,8 +2887,8 @@ def align_points_with_larger(self, larger_mobject: Mobject): def align_submobjects(self, mobject: Mobject) -> Self: mob1 = self mob2 = mobject - n1 = len(mob1.submobjects) - n2 = len(mob2.submobjects) + n1 = len(mob1._submobjects) + n2 = len(mob2._submobjects) mob1.add_n_more_submobjects(max(0, n2 - n1)) mob2.add_n_more_submobjects(max(0, n1 - n2)) return self @@ -2899,17 +2916,16 @@ def push_self_into_submobjects(self) -> Self: self.add(copy) return self - def add_n_more_submobjects(self, n: int) -> Self | None: + def add_n_more_submobjects(self, n: int) -> Self: if n == 0: return None - curr = len(self.submobjects) + curr = len(self._submobjects) if curr == 0: # If empty, simply add n point mobjects self.submobjects = [self.get_point_mobject() for k in range(n)] self.note_changed_family() - # TODO: shouldn't this return Self instead? - return None + return self target = curr + n # TODO, factor this out to utils so as to reuse @@ -2917,7 +2933,7 @@ def add_n_more_submobjects(self, n: int) -> Self | None: repeat_indices = (np.arange(target) * curr) // target split_factors = [sum(repeat_indices == i) for i in range(curr)] new_submobs = [] - for submob, sf in zip(self.submobjects, split_factors): + for submob, sf in zip(self._submobjects, split_factors): new_submobs.append(submob) for _ in range(1, sf): new_submobs.append(submob.copy().fade(1)) @@ -3157,7 +3173,7 @@ def construct(self): self.add(circle) """ if family: - for submob in self.submobjects: + for submob in self._submobjects: submob.set_z_index(z_index_value, family=family) self.z_index = z_index_value return self diff --git a/manim/mobject/opengl/opengl_mobject.py b/manim/mobject/opengl/opengl_mobject.py index c907c4c2e0..cefa5e9edb 100644 --- a/manim/mobject/opengl/opengl_mobject.py +++ b/manim/mobject/opengl/opengl_mobject.py @@ -723,7 +723,7 @@ def compute_bounding_box(self) -> npt.NDArray[float]: def refresh_bounding_box( self, recurse_down: bool = False, recurse_up: bool = True ) -> Self: - for mob in self.get_family(recurse_down): + for mob in self.get_family(recurse=recurse_down): mob.needs_new_bounding_box = True if recurse_up: for parent in self.parents: @@ -762,7 +762,7 @@ def assemble_family(self) -> Self: parent.assemble_family() return self - def get_family(self, recurse: bool = True) -> Sequence[OpenGLMobject]: + def get_family(self, *, recurse: bool = True) -> Sequence[OpenGLMobject]: if recurse and hasattr(self, "family"): return self.family else: @@ -2071,7 +2071,7 @@ def set_rgba_array( # Color only if color is not None and opacity is None: - for mob in self.get_family(recurse): + for mob in self.get_family(recurse=recurse): mob.data[name] = resize_array( mob.data[name] if name in mob.data else np.empty((1, 3)), len(rgbs) ) @@ -2079,7 +2079,7 @@ def set_rgba_array( # Opacity only if color is None and opacity is not None: - for mob in self.get_family(recurse): + for mob in self.get_family(recurse=recurse): mob.data[name] = resize_array( mob.data[name] if name in mob.data else np.empty((1, 3)), len(opacities), @@ -2089,7 +2089,7 @@ def set_rgba_array( # Color and opacity if color is not None and opacity is not None: rgbas = np.array([[*rgb, o] for rgb, o in zip(*make_even(rgbs, opacities))]) - for mob in self.get_family(recurse): + for mob in self.get_family(recurse=recurse): mob.data[name] = rgbas.copy() return self @@ -2112,7 +2112,7 @@ def set_rgba_array_direct( recurse set to true to recursively apply this method to submobjects """ - for mob in self.get_family(recurse): + for mob in self.get_family(recurse=recurse): mob.data[name] = rgbas.copy() def set_color( @@ -2172,7 +2172,7 @@ def get_gloss(self) -> float: return self.gloss def set_gloss(self, gloss: float, recurse: bool = True) -> Self: - for mob in self.get_family(recurse): + for mob in self.get_family(recurse=recurse): mob.gloss = gloss return self @@ -2180,7 +2180,7 @@ def get_shadow(self) -> float: return self.shadow def set_shadow(self, shadow: float, recurse: bool = True) -> Self: - for mob in self.get_family(recurse): + for mob in self.get_family(recurse=recurse): mob.shadow = shadow return self diff --git a/manim/mobject/opengl/opengl_surface.py b/manim/mobject/opengl/opengl_surface.py index 565b8c71cf..59504819f3 100644 --- a/manim/mobject/opengl/opengl_surface.py +++ b/manim/mobject/opengl/opengl_surface.py @@ -437,7 +437,7 @@ def init_colors(self): self.opacity = np.array([self.uv_surface.rgbas[:, 3]]) def set_opacity(self, opacity, recurse=True): - for mob in self.get_family(recurse): + for mob in self.get_family(recurse=recurse): mob.opacity = np.array([[o] for o in listify(opacity)]) return self diff --git a/manim/mobject/opengl/opengl_vectorized_mobject.py b/manim/mobject/opengl/opengl_vectorized_mobject.py index 2380d6d805..dc8696dcb2 100644 --- a/manim/mobject/opengl/opengl_vectorized_mobject.py +++ b/manim/mobject/opengl/opengl_vectorized_mobject.py @@ -268,11 +268,11 @@ def set_stroke( self.set_rgba_array(color, opacity, "stroke_rgba", recurse) if width is not None: - for mob in self.get_family(recurse): + for mob in self.get_family(recurse=recurse): mob.stroke_width = np.array([[width] for width in tuplify(width)]) if background is not None: - for mob in self.get_family(recurse): + for mob in self.get_family(recurse=recurse): mob.draw_stroke_behind_fill = background return self @@ -438,7 +438,7 @@ def get_opacity(self): return self.get_stroke_opacity() def set_flat_stroke(self, flat_stroke=True, recurse=True): - for mob in self.get_family(recurse): + for mob in self.get_family(recurse=recurse): mob.flat_stroke = flat_stroke return self @@ -548,7 +548,7 @@ def is_closed(self): return self.consider_points_equals(self.points[0], self.points[-1]) def subdivide_sharp_curves(self, angle_threshold=30 * DEGREES, recurse=True): - vmobs = [vm for vm in self.get_family(recurse) if vm.has_points()] + vmobs = [vm for vm in self.get_family(recurse=recurse) if vm.has_points()] for vmob in vmobs: new_points = [] for tup in vmob.get_bezier_tuples(): @@ -1248,7 +1248,7 @@ def insert_n_curves(self, n: int, recurse=True) -> OpenGLVMobject: OpenGLVMobject for chaining. """ - for mob in self.get_family(recurse): + for mob in self.get_family(recurse=recurse): if mob.get_num_curves() > 0: new_points = mob.insert_n_curves_to_point_list(n, mob.points) # TODO, this should happen in insert_n_curves_to_point_list From 143a0a95e1189d2eb409aad46ec813c4c49e6257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Mon, 15 Jul 2024 11:58:25 -0400 Subject: [PATCH 25/29] Explicit import of constants --- manim/mobject/mobject.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 9d905a0aeb..2546067b4a 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -22,7 +22,20 @@ import numpy as np from manim import config, logger -from manim.constants import * +from manim.constants import ( + DEFAULT_MOBJECT_TO_EDGE_BUFFER, + DEFAULT_MOBJECT_TO_MOBJECT_BUFFER, + DL, + DOWN, + IN, + LEFT, + MED_SMALL_BUFF, + ORIGIN, + OUT, + RIGHT, + TAU, + UP, +) from manim.mobject.opengl.opengl_compatibility import ConvertToOpenGL from manim.utils.color import ( BLACK, From 41016697303a28bc421078e74b4241e4b0916d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Mon, 15 Jul 2024 12:07:31 -0400 Subject: [PATCH 26/29] Changed a return None to return self --- manim/mobject/mobject.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 2546067b4a..18a5504c1d 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -2931,7 +2931,7 @@ def push_self_into_submobjects(self) -> Self: def add_n_more_submobjects(self, n: int) -> Self: if n == 0: - return None + return self curr = len(self._submobjects) if curr == 0: From 2d4fd9aef15ec11287ac5f8cf537d1006cd7bf1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Mon, 15 Jul 2024 12:08:09 -0400 Subject: [PATCH 27/29] Remove unused variable k in list comprehension and use underscore --- manim/mobject/mobject.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 18a5504c1d..55ce0f3f9f 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -2936,7 +2936,7 @@ def add_n_more_submobjects(self, n: int) -> Self: curr = len(self._submobjects) if curr == 0: # If empty, simply add n point mobjects - self.submobjects = [self.get_point_mobject() for k in range(n)] + self.submobjects = [self.get_point_mobject() for _ in range(n)] self.note_changed_family() return self From 78a294a6cde9ad5c313c2be8cb77aa1f6d11cae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Mon, 15 Jul 2024 12:57:06 -0400 Subject: [PATCH 28/29] Change mentions of submobjects to _submobjects --- manim/mobject/mobject.py | 50 ++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 55ce0f3f9f..039f7680ae 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -573,7 +573,7 @@ def add(self, *mobjects: Mobject) -> Self: ) self._submobjects = list_update(self._submobjects, unique_mobjects) - self._add_as_parent_of_submobs(mobjects) + self._add_as_parent_of_submobs(unique_mobjects) self.note_changed_family() return self @@ -978,7 +978,7 @@ def update(self, dt: float = 0, recursive: bool = True) -> Self: else: updater(self) if recursive: - for submob in self.submobjects: + for submob in self._submobjects: submob.update(dt, recursive) return self @@ -1169,7 +1169,7 @@ def clear_updaters(self, recursive: bool = True) -> Self: """ self.updaters = [] if recursive: - for submob in self.submobjects: + for submob in self._submobjects: submob.clear_updaters() return self @@ -1226,7 +1226,7 @@ def suspend_updating(self, recursive: bool = True) -> Self: self.updating_suspended = True if recursive: - for submob in self.submobjects: + for submob in self._submobjects: submob.suspend_updating(recursive) return self @@ -1251,7 +1251,7 @@ def resume_updating(self, recursive: bool = True) -> Self: """ self.updating_suspended = False if recursive: - for submob in self.submobjects: + for submob in self._submobjects: submob.resume_updating(recursive) self.update(dt=0, recursive=recursive) return self @@ -1411,7 +1411,7 @@ def apply_function_to_position(self, function: MappingFunction) -> Self: return self def apply_function_to_submobject_positions(self, function: MappingFunction) -> Self: - for submob in self.submobjects: + for submob in self._submobjects: submob.apply_function_to_position(function) return self @@ -1811,7 +1811,7 @@ def set_z(self, z: float, direction: Vector3D = ORIGIN) -> Self: def space_out_submobjects(self, factor: float = 1.5, **kwargs) -> Self: self.scale(factor, **kwargs) - for submob in self.submobjects: + for submob in self._submobjects: submob.scale(1.0 / factor) return self @@ -1833,7 +1833,7 @@ def move_to( def replace( self, mobject: Mobject, dim_to_match: int = 0, stretch: bool = False ) -> Self: - if not mobject.get_num_points() and not mobject.submobjects: + if not mobject.get_num_points() and not mobject._submobjects: raise Warning("Attempting to replace mobject with no points") if stretch: self.stretch_to_fit_width(mobject.width) @@ -1926,7 +1926,7 @@ def add_background_rectangle( return self def add_background_rectangle_to_submobjects(self, **kwargs) -> Self: - for submobject in self.submobjects: + for submobject in self._submobjects: submobject.add_background_rectangle(**kwargs) return self @@ -1946,7 +1946,7 @@ def set_color( of color """ if family: - for submob in self.submobjects: + for submob in self._submobjects: submob.set_color(color, family=family) self.color = ManimColor.parse(color) @@ -2022,13 +2022,13 @@ def fade_to( new_color = interpolate_color(self.get_color(), color, alpha) self.set_color(new_color, family=False) if family: - for submob in self.submobjects: + for submob in self._submobjects: submob.fade_to(color, alpha) return self def fade(self, darkness: float = 0.5, family: bool = True) -> Self: if family: - for submob in self.submobjects: + for submob in self._submobjects: submob.fade(darkness, family) return self @@ -2067,7 +2067,7 @@ def restore(self) -> Self: def reduce_across_dimension(self, reduce_func: Callable, dim: int): """Find the min or max value from a dimension across all points in this and submobjects.""" assert dim >= 0 and dim <= 2 - if len(self.submobjects) == 0 and len(self.points) == 0: + if len(self._submobjects) == 0 and len(self.points) == 0: # If we have no points and no submobjects, return 0 (e.g. center) return 0 @@ -2080,7 +2080,7 @@ def reduce_across_dimension(self, reduce_func: Callable, dim: int): rv = reduce_func(self.points[:, dim]) # Recursively ask submobjects (if any) for the biggest/ # smallest dimension they have and compare it to the return value. - for mobj in self.submobjects: + for mobj in self._submobjects: value = mobj.reduce_across_dimension(reduce_func, dim) if rv is None: rv = value @@ -2091,8 +2091,8 @@ def reduce_across_dimension(self, reduce_func: Callable, dim: int): def nonempty_submobjects(self) -> Sequence[Mobject]: return [ submob - for submob in self.submobjects - if len(submob.submobjects) != 0 or len(submob.points) != 0 + for submob in self._submobjects + if len(submob._submobjects) != 0 or len(submob.points) != 0 ] def get_merged_array(self, array_attr: str) -> np.ndarray: @@ -2102,7 +2102,7 @@ def get_merged_array(self, array_attr: str) -> np.ndarray: traversal of the submobjects. """ result = getattr(self, array_attr) - for submob in self.submobjects: + for submob in self._submobjects: result = np.append(result, submob.get_merged_array(array_attr), axis=0) return result @@ -2276,7 +2276,7 @@ def proportion_from_point(self, point: Point3D) -> float: def get_pieces(self, n_pieces: float) -> Group: template = self.copy() - template.submobjects = [] + template.set_submobjects([]) alphas = np.linspace(0, 1, n_pieces + 1) return Group( *( @@ -2445,7 +2445,7 @@ def get_family(self, *, recurse: bool = True) -> Sequence[Mobject]: return [self] if self._family is None: # Reconstruct and save - sub_families = (sm.get_family() for sm in self.submobjects) + sub_families = (sm.get_family() for sm in self._submobjects) family = [self, *it.chain(*sub_families)] self._family = remove_list_redundancies(family) return self._family @@ -2510,7 +2510,7 @@ def construct(self): x = VGroup(s1, s2, s3, s4).set_x(0).arrange(buff=1.0) self.add(x) """ - for m1, m2 in zip(self.submobjects, self.submobjects[1:]): + for m1, m2 in zip(self._submobjects, self._submobjects[1:]): m2.next_to(m1, direction, buff, **kwargs) if center: self.center() @@ -2936,7 +2936,9 @@ def add_n_more_submobjects(self, n: int) -> Self: curr = len(self._submobjects) if curr == 0: # If empty, simply add n point mobjects - self.submobjects = [self.get_point_mobject() for _ in range(n)] + self._submobjects = [self.get_point_mobject() for _ in range(n)] + for submob in self._submobjects: + self._parents.append(self) self.note_changed_family() return self @@ -2949,8 +2951,10 @@ def add_n_more_submobjects(self, n: int) -> Self: for submob, sf in zip(self._submobjects, split_factors): new_submobs.append(submob) for _ in range(1, sf): - new_submobs.append(submob.copy().fade(1)) - self.submobjects = new_submobs + submob_copy = submob.copy().fade(1) + submob_copy._parents.append(self) + new_submobs.append(submob_copy) + self._submobjects = new_submobs self.note_changed_family() return self From 836958e4e961eabdb26bfd5b47d07a587d4d1ee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= Date: Mon, 15 Jul 2024 14:05:08 -0400 Subject: [PATCH 29/29] Changed a self to submob in add_n_more_submobjects --- manim/mobject/mobject.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 039f7680ae..ffcbd0e29a 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -2938,7 +2938,7 @@ def add_n_more_submobjects(self, n: int) -> Self: # If empty, simply add n point mobjects self._submobjects = [self.get_point_mobject() for _ in range(n)] for submob in self._submobjects: - self._parents.append(self) + submob._parents.append(self) self.note_changed_family() return self