Generate normals/tangents/bitangents about gltf

[alteous]

Fixed the problem. We finally have a MikkTSpace interface!

[alteous]

Thanks for raising this issue. This is quite a noteworthy section of the specification.

I think we should wait until it's officially agreed upon before deciding to implement such features. Perhaps this could be an opt-in feature?

[bwasty]

Agreed. I already had a quick look at MikkTSpace since I never heard of that before. Looks like this is the best source: https://wiki.blender.org/index.php/Dev:Shading/Tangent_Space_Normal_Maps
The linked C implementation in the Blender repo apparently is the reference implementation, so it would need to be ported or safely wrapped for Rust.

[bwasty]

Just read a comment on the spec repo that bitangents can be generated in the shader and noticed there's not even a BITANGENT "semantic property name" (Meshes). So that last implementation note doesn't apply for the "loader" I'd say.

[alteous]

Updated link: https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#meshes

The glTF team have agreed on this implementation note. I think it would be a good idea to implement this in the wrapper interface.

[bwasty]

I've tried using corrode on the original MikkTSpace implementation, but ran into a few issues:

• jameysharp/corrode#98
  • easily solved by changing the syntax in a few dozen places
• jameysharp/corrode#99
  • math issue solved with -ffast-math
  • assert issue postponed by commenting out all asserts
• jameysharp/corrode#22
  • No idea for a quick solution - SEdge is a union and a pretty central data type

I'll stop here for the moment. Maybe I'll look at normal generation soon, that's nice and simple :)

[alteous]

Thanks for checking this out.

I like that fact they chose to use Haskell and Python to convert C to Rust. 😆

[alteous]

On closer inspection, the reference implementation has quite a simple interface. Creating an FFI wrapper around it shouldn't be too difficult, as long as it doesn't allocate memory.

[bwasty]

Creating an FFI wrapper around it shouldn't be too difficult

That's true, but after some trouble getting glfw-rs to compile on Windows recently (although I already had the correct Visual Studio version...), I'm quite in favor of pure Rust :)

Actually, making the code corrode-compatible should not be too hard, I'm just hesitant to change C code I barely understand and has no tests. But maybe the 36 asserts are enough^^
Anyway, I commited what I did so far: mikktspace-rs (last commit is the interesting one and this is the union).

[alteous]

I failed to realise this is a clone of https://github.com/alteous/gltf/issues/65

[alteous]

I really want to be able to provide this functionality but coming up with a good abstraction for it is quite tough. For now, let's consider writing a reference implementation in the documentation and let the user copy, paste, and amend as necessary. This code can be in the public domain for legal purposes.

[alteous]

Implementation planned in #78

[alteous]

Finally getting round to this...

I've got an interface to the C implementation set up. It's working correctly in the read phase but there's some undefined behaviour during the writing phase. See the generate example to reproduce.

[alteous]

three needs this right now, so a published crate will be necessary. I'm not sure who the owner of this code should be. @bwasty: Would you mind if I published / took ownership of a mikktspace crate? I've opened gltf-rs/mikktspace#1 so you have the option.

[alteous]

Maybe a bit overkill, but... perhaps we should consider a gltf-rs organisation?

[bwasty]

Not sure..for now I merged with just a cursory review and added you as a collaborator. Feel free to publish on crates.io.
I also thought of an org before...might even add the viewer to it.

[bwasty]

Quickly added a travis config. Almost wanted to publish, but noticed this:
warning: manifest has no description, license, license-file, documentation, homepage or repository.
At least the license should be clarified.

[alteous]

Let's test a bit more before publishing. The code runs but I haven't checked the correctness of the results yet.

[alteous]

Now that we have gltf-rs/mikktspace, is there anything else we could do to address this issue?

[bwasty]

Do you have a working example for using mikktspace with gltf? I had an idea for a sanity test: load all sample models with my screenshot-script, but always generate (ignoring existing normals+tangents) and compare the screenshots.

[alteous]

Do you have a working example for using mikktspace with gltf?

No. We could make an example showcasing the tangent / normal test model.

[alteous]

Your suggestion doesn't take into consideration provided smooth normals nor tangents that were not generated by mikktspace.

[bwasty]

I know it won't look exactly the same, but any major issue with the generated normals and tangents should hopefully become visible.

[alteous]

I'm tempted to close this issue with the resolution of 'delegated to the user'. It's the only reasonable strategy given the many conditions and situations an automatic solution would have to accommodate for. Besides, with mikktspace, it's not terribly difficult to implement correctly target generation anyway, and we already kind of delegate importing to the user.

[alteous]

Resolution: Delegated to the user.