gltfio: enable extended implementation #7776
Conversation
|
Please add a release note line to NEW_RELEASE_NOTES.md. If this PR does not warrant a release note, add the 'internal' label to this PR. |
| struct AssetConfigurationExtended { | ||
| //! Optional The same parameter as provided to \struct ResourceConfiguration ResourceLoader.h | ||
| //! gltfio/ResourceLoader.h | ||
| char const* gltfPath; |
There was a problem hiding this comment.
I don't understand why this is realted to mikkt space? Why would setting a glTF path in a separate struct change the tangent space computations? This doesn't seem like a user-friendly API (and does it mean it only works with local files?)
There was a problem hiding this comment.
Yes, deally, we'd just add support for mikktspace without changing the existing API, but that's not possible with how AssetLoader/ResourceLoader is structured. I opted to introduce minimal changes to existing API to address the opened bugs. So that any use case with the right criteria (desktop and only disk resources) can make use of it. But, the real, long-term solution should be a new loader API (separate from AssetLoader/ResourceLoader) that would better fit the assumption of running a remeshing algo like mikktspace.
There was a problem hiding this comment.
I added some comments in the header and in the commit to be more explicit about this approach's limitation.
There was a problem hiding this comment.
ah.. we should probably have a bug open then.
There was a problem hiding this comment.
good point, I filed #7782 for tracking.
poweifeng
force-pushed
the
pf/gltfio-integrate-extended
branch
from
efb5358
to
b37f983
Compare
libs/gltfio/src/AssetLoader.cpp
Outdated
| #if defined(__ANDROID__) || defined(IOS) || defined(__EMSCRIPTEN__) | ||
| PANIC_PRECONDITION("Extend asset loading is not supported on this platform"); | ||
| #endif |
There was a problem hiding this comment.
Is there a way to query if it's supported?
There was a problem hiding this comment.
moved this to a static method so that client can use the method to check for support.
poweifeng
force-pushed
the
pf/gltfio-integrate-extended
branch
from
b37f983
to
5eefaa6
Compare
This change will enable proper flat-shading and MikkTSpace.
Fixes #6358, #7444
Flat Shading comparison (model from #6358)
Before:
After:
Tangent space comparison (model from #7444)
Before:
After:
