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 code for loading normal and albedo maps from image data #1068

Merged
merged 7 commits into from
Sep 23, 2024

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Sep 18, 2024

🦟 Bug fix

Summary

Currently when we load a normal map from an image data (e.g. from a glb file that has an embedded normal map texture), we upload the image data as RGBA8 with 1 mip map level. This is different from what ogre's texture manager does when it loads a normal map from file. To make the process consistent, we follow the same steps: convert to an RG signed 8 bit format then generate multiple levels of mip maps. I found that the mip map generation process made a difference to the final appearance. It helps to make the visual look smoother when viewed from a distance.

Before: The visuals appears to have some artifacts / noise, e.g more evident on the cushions behind the tables

ionic_demo_normal_map_before

After: visuals appear smoother

ionic_demo_normal_map

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Ian Chen <ichen@openrobotics.org>
@github-actions github-actions bot added the 🏛️ ionic Gazebo Ionic label Sep 18, 2024
@iche033 iche033 requested a review from azeey September 18, 2024 03:13
ogre2/src/Ogre2Material.cc Outdated Show resolved Hide resolved
ogre2/src/Ogre2Material.cc Outdated Show resolved Hide resolved
ogre2/src/Ogre2Material.cc Show resolved Hide resolved
@azeey
Copy link
Contributor

azeey commented Sep 18, 2024

This is certainly better, but it looks like there still some differences
image
Left: this PR, Right: Using https://github.com/azeey/ionic_demo/tree/gltf_with_textures with https://github.com/azeey/gz-common/tree/fix_path

@azeey
Copy link
Contributor

azeey commented Sep 18, 2024

I just also noticed that the some of the objects in the scene appear to have a metallic look? Or at least a black band that I don't think should be there.
image

For reference, with all lights turned off and using #1067
image

And for additional reference, here's the render using Blender Cycles
cycles_reference

@iche033
Copy link
Contributor Author

iche033 commented Sep 18, 2024

This is certainly better, but it looks like there still some differences

While testing with ogre built from source, I noticed that it also generates mip maps for the albedo textures (and maybe others) which we're not doing. It could contribute to the differences, and there could also be others.

Signed-off-by: Ian Chen <ichen@openrobotics.org>
Signed-off-by: Ian Chen <ichen@openrobotics.org>
@iche033
Copy link
Contributor Author

iche033 commented Sep 18, 2024

While testing with ogre built from source, I noticed that it also generates mip maps for the albedo textures (and maybe others) which we're not doing. It could contribute to the differences, and there could also be others.

Updated to also generate mip maps for albedo textures. It made subtle difference. Surfaces appear slightly smoother

ionic_demo_normal_and_albedo

@iche033 iche033 changed the title Update code for loading normal map from image data Update code for loading normal and albedo maps from image data Sep 18, 2024
Signed-off-by: Ian Chen <ichen@openrobotics.org>
@iche033
Copy link
Contributor Author

iche033 commented Sep 18, 2024

I just also noticed that the some of the objects in the scene appear to have a metallic look? Or at least a black band that I don't think should be there.

For that particular black band, I think at least partially it's because GI is low res and not working well. If I turn off GI, that part of the ceiling is dark since it's not lit by the spot lights. With GI enabled, it lights up part of that surface but not all, leaving the black band. If I change GI resolution to [64, 64, 64], that band is gone. However, there are still small artifacts in other places.

@iche033
Copy link
Contributor Author

iche033 commented Sep 19, 2024

I think the appearance is now closer to the result from loading textures files. Can we get this in first?

Signed-off-by: Ian Chen <ichen@openrobotics.org>
@azeey azeey requested a review from ahcorde September 20, 2024 05:01
@ahcorde ahcorde merged commit 7ef4528 into gz-rendering9 Sep 23, 2024
10 of 11 checks passed
@ahcorde ahcorde deleted the glb_normal_map branch September 23, 2024 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏛️ ionic Gazebo Ionic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants