Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update atlas model to have textures. #73

Merged
merged 4 commits into from
Jan 9, 2025

Conversation

RussTedrake
Copy link
Collaborator

@RussTedrake RussTedrake commented Jan 7, 2025

These came from the original MIT DRC repo => openhumanoids/oh-distro (the same source as the existing models in this directory), with some elbowgrease in converting dae to obj+mtl, then obj2gltf -s.

Models and scripts can be found in:
RussTedrake/oh-distro@76f8c4b

image

This change is Reviewable

These came from the original MIT DRC repo => openhumanoids/oh-distro
(the same source as the existing models in this directory), with some
elbowgrease in converting dae to obj+mtl, then obj2gltf -s.

Models and scripts can be found in:
RussTedrake/oh-distro@76f8c4b
Copy link
Contributor

@SeanCurtis-TRI SeanCurtis-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass done.

I've made a non-blocking observation about normals. But we've got some further work to do on the textures.

Reviewed 52 of 52 files at r1, all commit messages.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee seancurtis-tri, platform LGTM missing (waiting on @RussTedrake)


a discussion (no related file):
This is clearly an improvement. But I thought I'd put a stake in the ground for what remains to yet be improved. You have funky normals.

For example, looking at l_uglut, in this PR it looks like this:

image.png

You'll see that what should be hard normals are being rounded. If we remove the textures, we can see it more clearly:

image copy 1.png

We really want the normals more like:

image copy 2.png

Which produces the final rendered image:

image copy 3.png

The effect ranges all over the robot.

We don't have to do anything about it now, but be aware that there are still fundamental issues with rendering quality.


a discussion (no related file):
We need ktx2 versions of all the textures as well. Anzu has a utility called gltf_add_ktx2.py which can be run on the .gltf files and will upgrade them to include references to .ktx2 files while simultaneously creating the ktx2 files.


a discussion (no related file):
You need to convert the .jpg to .png.

Clearly, this model loads into the meshcat viewer without problem. However, VTK's glTF importer is not so happy about them.

(This will actually be a precursor to creating the .ktx2 files as the anzu utility operates on glTF files that reference .png textures.)

Copy link
Contributor

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 3 unresolved discussions, LGTM missing from assignee seancurtis-tri, platform LGTM missing (waiting on @RussTedrake and @SeanCurtis-TRI)


a discussion (no related file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

We need ktx2 versions of all the textures as well. Anzu has a utility called gltf_add_ktx2.py which can be run on the .gltf files and will upgrade them to include references to .ktx2 files while simultaneously creating the ktx2 files.

FYI it's documented in Anzu's models/checklist_artist.md but the command is bazel run //models/tools:gltf_add_ktx2 -- --input=foo.gltf --overwrite with further detail in the file-level docstring of that program.

Copy link
Collaborator Author

@RussTedrake RussTedrake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 3 unresolved discussions, LGTM missing from assignee seancurtis-tri, platform LGTM missing (waiting on @SeanCurtis-TRI)


a discussion (no related file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

This is clearly an improvement. But I thought I'd put a stake in the ground for what remains to yet be improved. You have funky normals.

For example, looking at l_uglut, in this PR it looks like this:

image.png

You'll see that what should be hard normals are being rounded. If we remove the textures, we can see it more clearly:

image copy 1.png

We really want the normals more like:

image copy 2.png

Which produces the final rendered image:

image copy 3.png

The effect ranges all over the robot.

We don't have to do anything about it now, but be aware that there are still fundamental issues with rendering quality.

nice. I think i copied over the normals from the original source dae... though I suppose I could have mangled them in the process. How did you make the improved version? Should we just do that?

Copy link
Contributor

@SeanCurtis-TRI SeanCurtis-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 3 unresolved discussions, LGTM missing from assignee seancurtis-tri, platform LGTM missing (waiting on @RussTedrake)


a discussion (no related file):

Previously, RussTedrake (Russ Tedrake) wrote…

nice. I think i copied over the normals from the original source dae... though I suppose I could have mangled them in the process. How did you make the improved version? Should we just do that?

I dragged things into blender and ran a couple of operations. I believe you probably did copy over the original normals. The original normals themselves were crazy.

It probably takes about 30 seconds per file to fix the normals in blender (if you know the magical incantations, of course).

@RussTedrake
Copy link
Collaborator Author

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

I dragged things into blender and ran a couple of operations. I believe you probably did copy over the original normals. The original normals themselves were crazy.

It probably takes about 30 seconds per file to fix the normals in blender (if you know the magical incantations, of course).

to be clear, you opened the gltf in blender and rand the magical incantations (any hints?) and then saved? i can do that.

I wasn't actually able to open the original dae files in meshlab, blender, maya, ... Did you manage to open them?

@SeanCurtis-TRI
Copy link
Contributor

Previously, RussTedrake (Russ Tedrake) wrote…

to be clear, you opened the gltf in blender and rand the magical incantations (any hints?) and then saved? i can do that.

I wasn't actually able to open the original dae files in meshlab, blender, maya, ... Did you manage to open them?

Yep. I simply "patched" the glTF directly. The steps are (without the details of where things are):

  1. Import glTF.
  2. Merge vertices
  3. Reset the normals.
  4. Apply smoothing (as appropriate).
  5. Export as glTF.

If you like, I can run through all the models and push to your branch. I'll let you do the texture stuff.

Copy link
Collaborator Author

@RussTedrake RussTedrake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 3 unresolved discussions, LGTM missing from assignee seancurtis-tri, platform LGTM missing (waiting on @SeanCurtis-TRI)


a discussion (no related file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

You need to convert the .jpg to .png.

Clearly, this model loads into the meshcat viewer without problem. However, VTK's glTF importer is not so happy about them.

(This will actually be a precursor to creating the .ktx2 files as the anzu utility operates on glTF files that reference .png textures.)

Done.


a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

FYI it's documented in Anzu's models/checklist_artist.md but the command is bazel run //models/tools:gltf_add_ktx2 -- --input=foo.gltf --overwrite with further detail in the file-level docstring of that program.

Done.


a discussion (no related file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

Yep. I simply "patched" the glTF directly. The steps are (without the details of where things are):

  1. Import glTF.
  2. Merge vertices
  3. Reset the normals.
  4. Apply smoothing (as appropriate).
  5. Export as glTF.

If you like, I can run through all the models and push to your branch. I'll let you do the texture stuff.

That would be awesome. Thank you! I've done the texture stuff. ;-)

Copy link
Contributor

@SeanCurtis-TRI SeanCurtis-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 36 of 36 files at r2, 3 of 49 files at r3.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee seancurtis-tri, platform LGTM missing (waiting on @RussTedrake)


a discussion (no related file):

Previously, RussTedrake (Russ Tedrake) wrote…

That would be awesome. Thank you! I've done the texture stuff. ;-)

Done.

As I went through this I noticed something I hadn't particularly noted before. Specifically, a lot of the lighting is baked into the color textures. In fact, it looks like it was baked in from the original wonky normals. So, fixing the geometry takes us some way to getting us cleaner mechanical surfaces, but some of the surface weirdness is painted directly onto the surface like a bad optical illusion.

That will be a task for a later day if we decide we care. But, for now, at least if we put an environment map in with any kind of PBR materials into the glTFs, the normals will interact with the environment better than before. That's a win.


a discussion (no related file):

Previously, RussTedrake (Russ Tedrake) wrote…

Done.

FTR When editing glTF files in blender, it should always be done before doing ktx2 compatibility conversion. Blender does not preserve extensions. So, they get lost and need to be restored again. Should've been obvious in retrospect. :)


a discussion (no related file):
In the atlast_convex_hull.urdf, the following links still used .obj files in visual roles:

  • head (uses head_chull.obj)
    • I assumed this was an oversight as the head.gltf was not otherwise used (or some historical hack). I've corrected the visual geometry to use the glTF file.
    • I also removed the declared orange material.
  • hokuyo_lnik (uses head_camera_chull.obj)
    • Same treatment of head: switch to glTF and remove material.
  • left_palm and right_palm (use GRIPPER_OPEN_chull.obj)
    • There's really not a lot of recourse here as we have no alternative geometry.
    • However, I created a glTF version of the mesh so that, in meshcat, it would get illuminated the same as the rest (also "correcting" the normals).

Copy link
Collaborator Author

@RussTedrake RussTedrake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 unresolved discussion, LGTM missing from assignee seancurtis-tri, platform LGTM missing (waiting on @SeanCurtis-TRI)


a discussion (no related file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

In the atlast_convex_hull.urdf, the following links still used .obj files in visual roles:

  • head (uses head_chull.obj)
    • I assumed this was an oversight as the head.gltf was not otherwise used (or some historical hack). I've corrected the visual geometry to use the glTF file.
    • I also removed the declared orange material.
  • hokuyo_lnik (uses head_camera_chull.obj)
    • Same treatment of head: switch to glTF and remove material.
  • left_palm and right_palm (use GRIPPER_OPEN_chull.obj)
    • There's really not a lot of recourse here as we have no alternative geometry.
    • However, I created a glTF version of the mesh so that, in meshcat, it would get illuminated the same as the rest (also "correcting" the normals).

Thank you for spending your time on this! Based on the atlas_run_dynamics test in drake, things still look good in meshcat to me:

image.png

I don't know if I can check you on all of the finer details you've changed. But I'm comfortable moving forward with this. How shall we proceed?

Copy link
Contributor

@SeanCurtis-TRI SeanCurtis-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:LGTM:

Reviewable status: :shipit: complete! all discussions resolved, platform LGTM from [seancurtis-tri] (waiting on @RussTedrake)


a discussion (no related file):

Previously, RussTedrake (Russ Tedrake) wrote…

Thank you for spending your time on this! Based on the atlas_run_dynamics test in drake, things still look good in meshcat to me:

image.png

I don't know if I can check you on all of the finer details you've changed. But I'm comfortable moving forward with this. How shall we proceed?

For my money, we've looked at the files and looked at the rendered model and are both convinced things look good/sane. I stamp and a merge seems sufficient. (This repo is typically single reviewer.)

@SeanCurtis-TRI SeanCurtis-TRI merged commit 43a7e91 into RobotLocomotion:master Jan 9, 2025
2 checks passed
RussTedrake added a commit to RussTedrake/drake that referenced this pull request Jan 10, 2025
SeanCurtis-TRI pushed a commit to RobotLocomotion/drake that referenced this pull request Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants