[RTR] Added mesh scaling #61
Conversation
Ipuch
left a comment
Ipuch
left a comment
There was a problem hiding this comment.
Some minor comments, Thanks for contributing !
| return self.segment.characteristics().mesh().path().absolutePath().to_string() | ||
|
|
||
| @cached_property | ||
| def mesh_scale(self) -> str: |
There was a problem hiding this comment.
Tell me if i'm wrong:
def mesh_scale(self) -> np.darray:There was a problem hiding this comment.
Add a one-line doc to say the format of the output; (3,1) (3,)
| else: | ||
| self.__rerun_mesh = rr.Mesh3D( | ||
| vertex_positions=self.__mesh.vertices, | ||
| vertex_positions=transformed_trimesh.vertices, |
There was a problem hiding this comment.
You missed the self.__mesh in the first if case.
There was a problem hiding this comment.
Please, let me know if what I did is ok
| """ | ||
| A class to handle a trimesh object and its transformations | ||
| and always 'apply_transform' from its initial position | ||
| """ |
There was a problem hiding this comment.
Can you add the
'''
Attribute
-------------
scaling_factor: np.ndarray
A (3,1) or (3,0) or (1,3) vector for each direction x y z ?
'''There was a problem hiding this comment.
Documented the init instead
| if file_path.endswith(".stl") or file_path.endswith(".STL"): | ||
| mesh = load(file_path, file_type="stl") | ||
| return cls(name, mesh, transform_callable) | ||
| return cls(name, mesh, transform_callable, scaling_factor) |
There was a problem hiding this comment.
Can we apply the scaling to the mesh before line 80 it would save an attribute to the class that is meant to update the position of the mesh not to scale it. Maybe no, but if yes, it needs to be applied in the from_filemethod.
There was a problem hiding this comment.
As I understood it, there were two options:
- Scale the mesh ourselves: modify the position of the mesh vertices (which I was afraid to do if I am honest)
- Scale the mesh during visualization at the same time as visualizing its position and orientation (I found this option easier since rerun does the hard work for us)
|
I'll wait @aceglia PR because he might have done it too. On his side, and add an extra surprise (osim model viewer). |
|
Code Climate has analyzed commit 31d5fa4 and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
solved in #63 |
2 participants
This change is