search

google-research / kubric

A data generation pipeline for creating semi-realistic synthetic multi-object videos with rich annotations such as instance segmentation masks, depth maps, and optical flow.
Apache License 2.0
2.26k stars 221 forks source link

Shapenet converter: duplicate file `collision_geometry.obj` in exported archives #225

Open gabriel-v opened 2 years ago

gabriel-v commented 2 years ago

After rebuilding the shapelib container and starting the parfor script, I observed something off with the archives: Screenshot from 2022-04-26 18-33-49

There are 2 files with the same name.

When extracting, one of the files overwrites the other one.

Here are the offending lines: https://github.com/google-research/kubric/blob/a3d5eaa073c451bfe7797acf2c5235c6958d855e/shapenet2kubric/convert.py#L296-L299

I suppose the second file should have arcname='model_watertight.obj', correct? I can't find documentation for the specific format this is converting to; I assume it's using the same format as the MOVi objects?

MrXandbadas commented 2 years ago

From what I've read in the code and here online your assumption on the name convention is correct. When we add an object we need to pass through the noted two .obj files (both a collision_geometry file and a model_watertight file) Try changing line 299 to arcname='model_watertight.obj' and let me know if it succeeds, i'll happily make a quick PR for someone to review if it does work

Edit (some Context I found:

185

Qwlouse said:

To make this work for simulation you would need to create a collision mesh in obj format and a corresponding urdf file that you can pass as simulation_filename. If your mesh is watertight, you can take a look at the GSO preprocessing script about how to do that. If not, then things get complicated, and you will have to do more than that. The ShapeNet preprocessing scripts can serve as a starting point there.

Looking into the ShapeNet processing linked we see:

https://github.com/google-research/kubric/blob/9dd4ed18e27eadac3a28c9f6b2ae3e273b707aaf/shapenet2kubric/convert.py#L88-L90

Edit edit -- Woah brain broke, what is this

gabriel-v commented 2 years ago

I made that change locally for line 299 and the archives look good. I have yet to verify them in the renderer, through; but the conflicting filename bug is solved by that change.

MrXandbadas commented 2 years ago

Instead of spam the PR section I've added it into the current fork I have listed in the Pull Requests xD

Qwlouse commented 2 years ago

That is an excellent catch! I'll have to regenerate the ShapeNet assets once the fix is in....