Support loading PBR textures in OBJLoader #216
Conversation
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org> Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
graphics/src/OBJLoader.cc
Outdated
| // We use this field to determine if obj is exported by blender | ||
| // blender obj exporter puts roughness map in specular highlight | ||
| // field and metalness map in reflection map field! | ||
| // see summary in https://developer.blender.org/D8868 | ||
| // detailing the existing exporter issues | ||
| // todo(anyone) remove this hack when blender fixes their exporter | ||
| // issue? | ||
| if (!m.specular_highlight_texname.empty()) | ||
| { | ||
| pbrMat.SetRoughnessMap(m.specular_highlight_texname); | ||
| if (!m.reflection_texname.empty()) | ||
| pbrMat.SetMetalnessMap(m.reflection_texname); | ||
| } |
There was a problem hiding this comment.
I have mixed feelings about this, from one side it's nice to make blender assets work out of the box, but to be honest we might not need it for our use case since we are writing mtl and obj files manually and we have full control over the mtl fields.
Do you think for average users it is OK to have such hack to make Blender assets out of the box or do you think it's too "ugly"?
There was a problem hiding this comment.
yikes
I wonder if there's a way to detect if the file was created with Blender. It's nice to support Blender because it's widely used, but we shouldn't propagate their issues to our parser.
There was a problem hiding this comment.
ok I added a check for blender exported files. Looking at the obj files I have on disk, they have the exporter name in the first line of the .obj file so I'm using that to determine if the files are exported by blender or not. We'll likely need to add a blender version check to skip this workaround once the issue is fixed on their side. 96b109e
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Codecov Report
@@ Coverage Diff @@
## ign-common3 #216 +/- ##
===============================================
+ Coverage 75.18% 76.56% +1.37%
===============================================
Files 73 73
Lines 10326 10357 +31
===============================================
+ Hits 7764 7930 +166
+ Misses 2562 2427 -135
Continue to review full report at Codecov.
|
Signed-off-by: Ian Chen ichen@osrfoundation.org
🎉 New feature
Summary
Extends the OBJLoader to support loading PBR textures. The Wavefront OBJ format has PBR extensions which our current version of the tiny_obj_loader is already able to parse and load. This PR updates the loader to load these new fields into the
common::Materialandcommon::Pbrdata structures.Note that we found out blender's current obj exporter shoves roughness map into the specular highlight field and metalness map into the reflection map field! Here's a PR that describes the existing issue with the exporter: https://developer.blender.org/D8868.
They are rewriting the exporter in C++ that will also fix these issues but until then all obj files exported by blender will have these fields incorrectly exported. I've updated the OBJLoader to handle this particular case but I'm open to removing these specific checks for blender.
I also updated OBJLoader_TEST to load and check an obj file exported by 3ds max and an obj file exported by blender.
Test it
Here is a model on Fuel in obj format with PBR textures for testing:
https://app.ignitionrobotics.org/iche033/fuel/models/Cube%20PBR%20OBJ
This model is exported by 3ds max.
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge