Update python mesh #492
Update python mesh #492
Conversation
|
Will have a look at this. I think we should also add at least one python example using MeshGL. |
|
|
Sorry I was writing a comment then decided it was wrong and tried to delete, and somehow fat fingered close PR (on phone). My bad! |
|
I'm going to update our examples to this, because I'd prefer to only use MeshGL in the bindings. I may even deprecate Mesh in the C++, as I don't love supporting both. If you know a more efficient way to access the vectors, that would be great. I'm no python expert. I just want the API to be the right shape so I can feel good publishing it. |
|
It seems that we don't really need to copy vectors of primitive types into a python array, pybind11 does that automatically for us already. I also updated the python example to use the |
|
For the other python changes, e.g. hull, should I open another PR or should I add them here? should be relatively small change. |
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## master #492 +/- ##
=======================================
Coverage 90.34% 90.34%
=======================================
Files 35 35
Lines 4414 4414
=======================================
Hits 3988 3988
Misses 426 426 ☔ View full report in Codecov by Sentry. |
|
I've got a bindings PR in the works that I included python in. Just haven't finished the JS bits yet. |
|
@axel-angel this should be our final breaking change. You can see the updated |
|
@elalish maybe we can merge this? As I modified this, it doesn't make sense for me to review it again, and I cannot choose you to be the reviewer. |
|
Thank you so much, @pca006132! I had intended to get back to this earlier, but I got laid up with a sinus infection instead. Thanks for figuring this out. I'll review soon. |
|
Btw I am thinking if we should return a flattened array, i.e. the same as our C++ interface, or a reshaped array, as numpy can do reshaping without actually copying the memory. |
bindings/python/examples/run_all.py
Outdated
| meshOut = trimesh.Trimesh( | ||
| vertices=mesh.vert_pos, faces=mesh.tri_verts) | ||
| vertices=vertices, faces=np.reshape(mesh.tri_verts, (-1, 3))) |
There was a problem hiding this comment.
I want to update this for properties, which will involve splitting this up and putting the rest into Trimesh.visual. That can happen as a follow-on though.
There was a problem hiding this comment.
I think it would be better to finish everything in this PR, to test the new APIs are working correctly. (in case we understood python incorrectly...)
There was a problem hiding this comment.
Yeah, fair enough. Do you want to take a stab at it? Otherwise I can work on it tomorrow.
There was a problem hiding this comment.
yeah you can work on it
|
Note that some parameters are not constant because reshape is not a constant function. |
| self.halfedgeTangent.data()) | ||
| .reshape(std::array<int, 3>{-1, 3, 4}); | ||
| }) | ||
| .def_readonly("merge_from_vert", &MeshGL::mergeFromVert) |
There was a problem hiding this comment.
Do these automatically take the pointer, or are they copying the std::vector instead?
There was a problem hiding this comment.
I found that I forgot to add opaque types. https://pybind11.readthedocs.io/en/stable/advanced/cast/stl.html#making-opaque-types
Now, it should auotmatically take the pointer.
* updated python mesh * updated python meshgl * clean up array related stuff * fix hsplit * fix according to comments * make to_mesh accept MeshGL * fixes --------- Co-authored-by: pca006132 <[email protected]>
Fixes #361 - at least I think so; please remind me if I've forgotten anything.
The main thing I'm adding here is
MeshGLsupport, which is to say updating the Python Mesh class to work as MeshGL instead, akin to our JS binding. This PR is still WIP, as I need to update ourtrimeshintegration, but we should now be able to export vertex colors and such right through to glTF with that soon.