open-forest-observatory / geograypher

Multiview Semantic Reasoning with Geospatial Data
BSD 3-Clause "New" or "Revised" License
10 stars 4 forks source link

Fixed out of index error for batching rendering operations #109

Closed asidhu0 closed 2 months ago

asidhu0 commented 2 months ago

When the batch_size is greater than 1, we extend the mesh so that the number of meshes is equal to batch_size, which is batch_size number of cameras. However, with this extension of meshes the indices of the mesh exceed the index for the number of faces which had caused an indexing error. To accommodate this we adjust the face indices considering the batch offset in the extended meshes.

asidhu0 commented 2 months ago

@russelldj Is there any improvement we can do with force_recreation? I remember you mentioned it beforehand but I forgot why it is needed.

russelldj commented 2 months ago

Currently, create_pytorch3d_mesh does not return a mesh, it only sets the self.pytorch3d_mesh attribute. It's lazy and if the mesh already exists, and force_recreate isn't True, it does nothing. I'm concerned that could lead to bugs because the contents of self.pytorch3d_mesh may not always be what you expect. One option would be to remove force_recreate and always just create a new pytorch3d_mesh from the pyvista one. Then this method would just return the newly-created pytorch3d mesh. But we might find this is too much of a performance penalty.

russelldj commented 2 months ago

Nice work! Can you delete your branch when you get a chance?