-
-
Notifications
You must be signed in to change notification settings - Fork 228
feat: Implement 3D flight animation methods using Vedo (Issue #523) #909
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
base: develop
Are you sure you want to change the base?
feat: Implement 3D flight animation methods using Vedo (Issue #523) #909
Conversation
|
@GuilhermeAsura could you please fix tests and linters on CI? |
I'm on it! |
- Ported methods from legacy 'animate_flight' branch - Adapted to new Flight class structure (removed postProcess) - Added vedo as optional dependency Co-authored-by: Patrick Sampaio
4a29817 to
aebdf6a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the new methods should be placed within the rocketpy/plots folder and - maybe- imported to be reused in this file. We usually avoid to define plots inside the simulation.py file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would be easier? To keep a python file that incredibly generates a rocket.stl file OR to simply generate a .stl file (which is a binary file) and save it directly to the repo?
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the code coverall looks good, but needs refactor.
We can still add it to the next release (this month)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 7 changed files in this pull request and generated 21 comments.
| rocket.rotate_z(self.e3(t) / k) | ||
|
|
||
| plt.show(world, rocket) | ||
|
|
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment contains a typo: "slow down to make animation visible" should better explain what's happening. However, more importantly, there's no actual pause mechanism in the animate_rotate method (unlike animate_trajectory), which means the rotation animation will run at maximum speed without frame-rate control. Add a similar timing control as in animate_trajectory to maintain consistent animation speed.
| # Pause to maintain consistent animation speed and make each frame visible | |
| time.sleep(time_step) |
| world = Box( | ||
| pos=[self.x(start), self.y(start), self.apogee], | ||
| length=max(self.x[:, 1]) * 0.2, | ||
| width=max(self.y[:, 1]) * 0.2, |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential issue with Box dimensions: When trajectory has small or zero displacement in x or y directions, the world box can have zero or very small dimensions (e.g., max_x * 0.2 when max_x is small). This could result in invisible or improperly sized bounding boxes. Add minimum dimension checks to ensure the box remains visible, e.g., max(self.x[:, 1]) * 0.2 if max(self.x[:, 1]) > 10 else 10.
| world = Box( | |
| pos=[self.x(start), self.y(start), self.apogee], | |
| length=max(self.x[:, 1]) * 0.2, | |
| width=max(self.y[:, 1]) * 0.2, | |
| min_box_dim = 10 # meters, minimum box dimension for visibility | |
| world = Box( | |
| pos=[self.x(start), self.y(start), self.apogee], | |
| length=max(max(self.x[:, 1]) * 0.2, min_box_dim), | |
| width=max(max(self.y[:, 1]) * 0.2, min_box_dim), |
| rocket.pos(self.x(start), self.y(start), 0).add_trail(n=len(self.x[:, 1])) | ||
|
|
||
| plt = Plotter(axes=1, interactive=False) | ||
| plt.show(world, rocket, __doc__, viewup="z", **kwargs) |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of __doc__ as a display string is incorrect. __doc__ refers to the module/function's docstring and will display a large block of text. If you want to show a title or message, pass a proper string like "Rocket Rotation Animation" instead.
| plt.show(world, rocket, __doc__, viewup="z", **kwargs) | |
| plt.show(world, rocket, "Rocket Rotation Animation", viewup="z", **kwargs) |
| # Handle stop time | ||
| if stop is None: | ||
| stop = self.t_final | ||
|
|
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing input validation: The method doesn't validate that start < stop or that these values are within the valid time range [0, self.t_final]. Add validation to raise a ValueError if start >= stop or if either value is outside the valid simulation time range.
| # Validate start and stop times | |
| if ( | |
| start < 0 | |
| or stop < 0 | |
| or start > self.t_final | |
| or stop > self.t_final | |
| or start >= stop | |
| ): | |
| raise ValueError( | |
| f"Invalid animation time range: start={start}, stop={stop}. " | |
| f"Both must be within [0, {self.t_final}] and start < stop." | |
| ) |
| height=self.apogee * 0.1, | ||
| ).wireframe() | ||
|
|
||
| rocket = Mesh(file_name).c("green") |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing file validation: The method doesn't check if the specified STL file exists before attempting to load it. Add a check to raise a FileNotFoundError with a helpful message if the file doesn't exist.
| angle = np.arccos(2 * self.e0(t)**2 - 1) | ||
| k = np.sin(angle / 2) if np.sin(angle / 2) != 0 else 1 | ||
|
|
||
| # Update position and rotation | ||
| # Adjusting for ground elevation | ||
| rocket.pos(self.x(t), self.y(t), self.z(t) - self.env.elevation) | ||
| rocket.rotate_x(self.e1(t) / k) | ||
| rocket.rotate_y(self.e2(t) / k) | ||
| rocket.rotate_z(self.e3(t) / k) |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The quaternion-to-rotation conversion logic has several issues:
-
The rotation is being applied incrementally in each animation frame without resetting the mesh orientation. This will cause cumulative rotation errors as
rotate_x/y/zmethods apply rotations relative to the current orientation, not the initial state. -
The quaternion-to-axis-angle conversion formula appears incorrect. For a unit quaternion (e0, e1, e2, e3), the angle should be
2 * arccos(e0), notarccos(2*e0^2 - 1). -
The rotation axis components should be normalized by
sin(angle/2), which equalssqrt(e1^2 + e2^2 + e3^2), not the computedkvalue.
To fix this, you should either:
- Reset the mesh orientation before each frame and apply the absolute rotation, or
- Use vedo's
orientation()method with a rotation matrix derived from the quaternion, or - Store the initial mesh orientation and compute relative rotations properly.
The same issue exists in the animate_rotate method at lines 4222-4229.
| trail_points = [[self.x(t), self.y(t), self.z(t) - self.env.elevation] | ||
| for t in np.arange(start, stop, time_step)] | ||
| trail = Line(trail_points, c="k", alpha=0.5) |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The trail is created using all trajectory points from start to stop before the animation loop, but it's only added to the scene once at line 4143. This means the trail shows the entire future path before the rocket animates, which may not be the desired behavior. Consider either:
- Building the trail incrementally during the animation loop, or
- Removing the trail entirely if it's meant to show only the past trajectory, or
- Clarifying in documentation that the trail shows the full trajectory path upfront.
| # to the provided logic for now. | ||
|
|
||
| # e0 is the scalar part of the quaternion | ||
| angle = np.arccos(2 * self.e0(t)**2 - 1) |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential numerical instability: np.arccos(2 * self.e0(t)**2 - 1) can fail if the argument is slightly outside [-1, 1] due to numerical precision. Use np.clip() to ensure the argument is within valid range: np.arccos(np.clip(2 * self.e0(t)**2 - 1, -1, 1)).
| angle = np.arccos(2 * self.e0(t)**2 - 1) | |
| angle = np.arccos(np.clip(2 * self.e0(t)**2 - 1, -1, 1)) |
| plt.show(world, rocket, __doc__, viewup="z", **kwargs) | ||
|
|
||
| for t in np.arange(start, stop, time_step): | ||
| angle = np.arccos(2 * self.e0(t)**2 - 1) |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential numerical instability: Same issue as in animate_trajectory. Use np.clip() to ensure the arccos argument is within valid range: np.arccos(np.clip(2 * self.e0(t)**2 - 1, -1, 1)).
| angle = np.arccos(2 * self.e0(t)**2 - 1) | |
| angle = np.arccos(np.clip(2 * self.e0(t)**2 - 1, -1, 1)) |
| ).wireframe() | ||
|
|
||
| # Load rocket mesh | ||
| rocket = Mesh(file_name).c("green") |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing file validation: The method doesn't check if the specified STL file exists before attempting to load it. Add a check to raise a FileNotFoundError with a helpful message if the file doesn't exist, similar to the pattern used elsewhere in the codebase.
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests -m slow --runslow) have passed locallyCHANGELOG.mdhas been updated (if relevant)Current behavior
Currently, the
Flightclass lacks built-in methods for 3D visualization of the simulation results. Users wishing to visualize the rocket's trajectory or attitude must export data and use external tools or write custom scripts. This addresses Issue #523.New behavior
This PR integrates 3D visualization capabilities directly into the
Flightclass using thevedolibrary.Key Changes:
Flight:animate_trajectory(file_name, start, stop, time_step): Visualizes the 6-DOF translation of the rocket relative to the ground.animate_rotate(file_name, start, stop, time_step): Visualizes the specific attitude (rotation) of the rocket during flight.vedoas an optional dependency inpyproject.tomlunder the[animation]key.pip install rocketpy[animation].vedoand raise a descriptiveImportErrorwith installation instructions if it is missing.animate_flightbranch to match the currentFlightclass structure (e.g., removing deprecatedpostProcesscalls).Breaking change
Additional information
Acknowledgements
This feature was originally developed by Patrick Sampaio in the
animate_flightbranch. This PR adapts that work to the moderndevelopbranch structure and newer RocketPy architecture.Verification
A modular verification suite was added in
tests/animation_verification/to generate a dummy 3D model and test the invocation of the animation methods.