Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Arrow3D.put_start_and_end_on() to use the actual end of the arrow #3706

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 33 additions & 11 deletions manim/mobject/three_d/three_dimensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -621,17 +621,18 @@ def __init__(
**kwargs,
)
# used for rotations
self.new_height = height
self._current_theta = 0
self._current_phi = 0

self.base_circle = Circle(
radius=base_radius,
color=self.fill_color,
fill_opacity=self.fill_opacity,
stroke_width=0,
)
self.base_circle.shift(height * IN)
self._set_start_and_end_attributes(direction)
if show_base:
self.base_circle = Circle(
radius=base_radius,
color=self.fill_color,
fill_opacity=self.fill_opacity,
stroke_width=0,
)
self.base_circle.shift(height * IN)
self.add(self.base_circle)

self._rotate_to_direction()
Expand Down Expand Up @@ -661,6 +662,12 @@ def func(self, u: float, v: float) -> np.ndarray:
],
)

def get_start(self) -> np.ndarray:
return self.start_point.get_center()

def get_end(self) -> np.ndarray:
return self.end_point.get_center()

def _rotate_to_direction(self) -> None:
x, y, z = self.direction

Expand Down Expand Up @@ -715,6 +722,15 @@ def get_direction(self) -> np.ndarray:
"""
return self.direction

def _set_start_and_end_attributes(self, direction):
normalized_direction = direction * np.linalg.norm(direction)

start = self.base_circle.get_center()
end = start + normalized_direction * self.new_height
self.start_point = VMobject().set_points_as_corners([start] * 3)
self.end_point = VMobject().set_points_as_corners([end] * 3)
Copy link
Member

Choose a reason for hiding this comment

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

Can the tripled reference to start and end cause any harm here? (Modifying any of the entries will modify all three of them in the same way.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that it must be the same point three times. Otherwise we will get a 3d curve wich would be visible, and we don't want this. My solution based on merge #3358 or #3718. I am not sure why we might have problem here, because we are calculating everything internally based on user start and end points. Maybe I missunderstood you question

Copy link
Member

@JasonGrace2282 JasonGrace2282 May 5, 2024

Choose a reason for hiding this comment

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

I believe he is talking about this behavior

x = [1]
y = [x]*3
x.append(2)
print(y)

Which results in

[[1, 2], [1, 2], [1, 2]]

Because y simply stores a list of 3 "pointers" to x.

Copy link
Member

Choose a reason for hiding this comment

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

Exactly, this is what I was referring to!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, but should we care about it? They are local variables, as long as direction, get_center and new_height will return propper types we will not have a problem here. I could add type checking but I am not sure that we need it here. Why? Because if direction won't have a proper type cone and cilinder will through the error same for height, if get_center will have invalid type all manim will be broken. I could add it but IMO if the type of the variables used in this method other part of the code will through the error ealier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will push today the VectorizedPoint version.

Copy link
Member

Choose a reason for hiding this comment

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

Is there any update on this? It would be nice to have it merged before the v0.19.0 release :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had problems with my laptop (and I was finishing my 4th physic semester)
I will push tuday the newest version of the code
Sorry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i just switched to new PC, i will install manim tomorrow and then i will add changes -> push.|
For some reason the changes which i made, wasn't saved, i have no idea why

Copy link
Contributor

Choose a reason for hiding this comment

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

I have seen the changes, thanks!

self.add(self.start_point, self.end_point)


class Cylinder(Surface):
"""A cylinder, defined by its height, radius and direction,
Expand Down Expand Up @@ -1155,14 +1171,20 @@ def __init__(
self.end - height * self.direction,
**kwargs,
)

self.cone = Cone(
direction=self.direction, base_radius=base_radius, height=height, **kwargs
direction=self.direction,
base_radius=base_radius,
height=height,
**kwargs,
)
self.cone.shift(end)
self.add(self.cone)
self.end_point = VMobject().set_points_as_corners([end] * 3)
self.add(self.end_point, self.cone)
self.set_color(color)

def get_end(self) -> np.ndarray:
return self.end_point.get_end()


class Torus(Surface):
"""A torus.
Expand Down
23 changes: 23 additions & 0 deletions tests/test_graphical_units/test_threed.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ def test_Cone(scene):
scene.add(Cone(resolution=16))


def test_Cone_get_start_and_get_end():
cone = Cone().shift(RIGHT).rotate(PI / 4, about_point=ORIGIN, about_edge=OUT)
start = [0.70710678, 0.70710678, -1.0]
end = [0.70710678, 0.70710678, 0.0]
assert np.allclose(
cone.get_start(), start, atol=0.01
), "start points of Cone do not match"
assert np.allclose(
cone.get_end(), end, atol=0.01
), "end points of Cone do not match"


@frames_comparison(base_scene=ThreeDScene)
def test_Cylinder(scene):
scene.add(Cylinder())
Expand Down Expand Up @@ -149,3 +161,14 @@ def param_surface(u, v):
axes=axes, colorscale=[(RED, -0.4), (YELLOW, 0), (GREEN, 0.4)], axis=1
)
scene.add(axes, surface_plane)


def test_get_start_and_end_Arrow3d():
start, end = ORIGIN, np.array([2, 1, 0])
arrow = Arrow3D(start, end)
assert np.allclose(
arrow.get_start(), start, atol=0.01
), "start points of Arrow3D do not match"
assert np.allclose(
arrow.get_end(), end, atol=0.01
), "end points of Arrow3D do not match"
Loading