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

Adding support for Box shapes to dyviz js #484

Merged
merged 1 commit into from
Mar 4, 2023
Merged

Conversation

ebranlard
Copy link
Contributor

The python code pydy.viz.shapes.py provides support for Box shapes but the type was not supported by the javascript code. This pull request add support for this, including the possibility to edit the parameters in the GUI when selecting Edit Objects.

I can try to add documentation and unittests but I will wait for your opinion.

Below is a sample code to test the feature (it might need a "Shift-F5" to reload the javascript from the browser's cache):

import sympy as sp
from sympy.physics.mechanics import Point, ReferenceFrame
from pydy.viz.scene import Scene
from pydy.viz.shapes import Box
from pydy.viz.visualization_frame import VisualizationFrame

frame = ReferenceFrame('e')
point = Point('A')

box = Box(width=4, height=2, depth=0.5, color='blue', name='testbox')
vizf = VisualizationFrame('vizbox',frame, point, box)
scene = Scene(frame, point, vizf)
scene.constants = {}
scene.states_symbols = [sp.Symbol('x')]
scene.states_trajectories = np.zeros((2,1))
scene.times = np.array([0.,1.0])
scene.display()

New Feature

  • There are no merge conflicts.
  • If there is a related issue, a reference to that issue is in the
    commit message.
  • Unit tests have been added for the new feature.
  • The PR passes tests both locally (run nosetests) and on Travis CI.
  • All public methods and classes have docstrings. (We use the numpydoc
    format
    .)
  • An explanation has been added to the online documentation. (docs
    directory)
  • The code follows PEP8 guidelines. (use a linter, e.g.
    pylint, to check your code)
  • The new feature is documented in the Release
    Notes
    .
  • The code is backwards compatible. (All public methods/classes must
    follow deprecation cycles.)
  • All reviewer comments have been addressed.

@moorepants
Copy link
Member

Is there a reason "Cube" isn't sufficient?

@moorepants
Copy link
Member

There does seem to be a BoxGeometry in Three.js: https://threejs.org/docs/#api/en/geometries/BoxGeometry

I'm not seeing a CubeGeometry in the three.js docs.

@ebranlard
Copy link
Contributor Author

ebranlard commented Dec 5, 2022

Having the support for boxes is convenient to visualize geometries that have different lengths in all three directions (like a ladder, a cart, etc.). To some extent, supporting Cube is redundant, but offers some convenience to the user.

The python code already have support for both Cube and Box (see here), and Cube is actually just implemented as a Box.

The problem is that the current Javascript code of pydy that does not support Box but Three.js does support it. My code changes fix that.

You are right that in my code change, I've used CubeGeometry, and not BoxGeometry, which is a bit confusing. The former is an alias for the later (see here) so the code works as intended.

Since CubeGeometry is getting depreciated, we can use BoxGeometry both for Cube and Box in the javascript code. I can update the code if you agree.

@moorepants moorepants merged commit 8e36333 into pydy:master Mar 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants