google / brax

Massively parallel rigidbody physics simulation on accelerator hardware.
Apache License 2.0
2.25k stars 249 forks source link

Added rgba colour from MuJoCo file to renderer #295

Closed ManuCorrea closed 1 year ago

ManuCorrea commented 1 year ago

Removed hardcoded 'target' colour since it can be specified in XML files.

Also added dependencies in setup to run brax/v2/visualizer/visualizer.py

I am also working on adding textures to the renderer. Using the current way all the information is dumped into the brax system json. Encoding de picture in the json makes the file really big and can slow down the process. I can try to finish out the texture feature so you could judge and we could discuss the implementation if necessary,


I was not sure how to run all the tests until I realized with pytest, that is why I commited without the test passing.

Since my decision seems that would required changes in the rest of the project, should be convenient a default rgba value in Geometry class? With Optional argument all the parts in the rest of the code should be modified to add None. Which way should I proceed, any suggestion or I find out my way?

ManuCorrea commented 1 year ago

I have seen that with Python 3.10 it can be set defaults with parent and children dataclasses without problems.

If I put rgba as default None it errors because its default before the arguments in the children Classes:

@struct.dataclass
class Geometry(_Base):
  link_idx: Optional[jp.ndarray]
  transform: Transform
  friction: jp.ndarray
  elasticity: jp.ndarray
  # TypeError: non-default argument 'radius' follows default argument
  rgba: Optional[jp.ndarray] = None

I could do something like this for each of the children Classes:

@struct.dataclass
class Sphere(Geometry):
  """A sphere.

  Attributes:
    radius: radius of the sphere
  """

  radius: jp.ndarray
  rgba: Optional[jp.ndarray] = None

@struct.dataclass
class Capsule(Geometry):
  """A capsule.

  Attributes:
    radius: radius of the capsule end
    length: distance between the two capsule end centroids
  """

  radius: jp.ndarray
  length: jp.ndarray
  rgba: Optional[jp.ndarray] = None
ManuCorrea commented 1 year ago

I have seem quite some has evolved in the repo since the PR! I will work on the texture implementation soon, should I also commit in the PR the work in progress for having a more continuous feedback?

btaba commented 1 year ago

I have seem quite some has evolved in the repo since the PR! I will work on the texture implementation soon, should I also commit in the PR the work in progress for having a more continuous feedback?

Hey @ManuCorrea , I'm not sure what you mean by the work in progress, but this PR looks good, let's get it in! Just some failing tests to resolve on this branch

ManuCorrea commented 1 year ago

Now with the new mjcf.py version the tests fails. Seems to be that it generated Convex objects which has rgba value as None and the arguments from mjcf.py are not passed to the Convex object in base.py but to a constructor from mesh.py

btaba commented 1 year ago

Will merge and then patch the test since we will be releasing v2 shortly...