compas-dev / compas_fab

Robotic fabrication package for the COMPAS Framework.
https://compas.dev/compas_fab/
MIT License
108 stars 32 forks source link

Change temp file name in convert_mesh_to_body #306

Closed beverlylytle closed 3 years ago

beverlylytle commented 3 years ago

Closes #305 PyBullet, by default, caches the contents of obj's that it has loaded. If it encounters a recognized path to an obj, it assumes the contents are the same as when it first read it. Previously all the meshes were stored in some/temp/dir/temp.obj. Now they will be stored in some/temp/dir/{mesh.guid}.obj. I don't want to disable cacheing entirely, because that would effect performance. I also don't want to return to having a new temp dir every time convert_mesh_to_body is called, because that is essentially turning off cacheing. @gonzalocasas is this sufficient? Given the way mesh.uuid is created (that is, lazily created when guid is called, but not modified when the mesh changes), the problem still exists for the following code:

from compas.datastructures import Mesh
from compas.geometry import Translation

import compas_fab
from compas_fab.backends.pybullet import PyBulletClient
from compas_fab.robots import CollisionMesh

with PyBulletClient() as client:
    mesh = Mesh.from_stl(compas_fab.get('planning_scene/floor.stl'))
    cm = CollisionMesh(mesh, 'floor')
    client.add_collision_mesh(cm)

    mesh.transform(Translation.from_vector([1,1,1]))
    cm = CollisionMesh(mesh, 'floor_translated')
    client.add_collision_mesh(cm)

What type of change is this?

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

yijiangh commented 3 years ago

Thanks for fixing the bug!

Given the way mesh.uuid is created (that is, lazily created when guid is called, but not modified when the mesh changes)

Shall we add this case as a warning in the docstring to give the users a heads-up, until we have a better solution for this?

beverlylytle commented 3 years ago

Yes, that makes sense.

beverlylytle commented 3 years ago

Done.