gltf-rs / mikktspace Goto Github PK

I am currently trying to generate tangents on the fly for the bevy engine, and to avoid pulling in the nalgebra crate, I'd like to publish a fork of this crate using glam (and of course clearly credit you in the readme). Is this something you'd be okay with?

Thank you very much for creating this library.

The initial idea for this crate was to translate the original MikkTSpace implementation to Rust using corrode. This turned out to be trickier than expected, so we went with plain C bindings. In case we want to retry in the future, here are the past findings (copied from gltf-rs/gltf#22):

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

The partially adapted code is still available in the corrode branch.

I've added mikktspace to my Cargo.toml, now I get a CMake error on Travis:

+cross build --target x86_64-unknown-linux-gnu
    Updating git repository `https://github.com/gltf-rs/mikktspace`
 Downloading cmake v0.1.26
 Downloading cc v1.0.0
Unable to find image 'japaric/x86_64-unknown-linux-gnu:v0.1.12' locally
v0.1.12: Pulling from japaric/x86_64-unknown-linux-gnu
Status: Downloaded newer image for japaric/x86_64-unknown-linux-gnu:v0.1.12
   Compiling cc v1.0.0
   Compiling cmake v0.1.26
   Compiling mikktspace v0.1.0 (https://github.com/gltf-rs/mikktspace#95ac6cfc)
error: failed to run custom build command for `mikktspace v0.1.0 (https://github.com/gltf-rs/mikktspace#95ac6cfc)`
process didn't exit successfully: `/target/debug/build/mikktspace-ce40bb6888654088/build-script-build` (exit code: 101)
--- stdout
running: "cmake" "/cargo/git/checkouts/mikktspace-a47cd97c1cd7847f/95ac6cf/libmikktspace" "-DCMAKE_INSTALL_PREFIX=/target/x86_64-unknown-linux-gnu/debug/build/mikktspace-f5ed98465e6e8dfa/out" "-DCMAKE_C_FLAGS= -ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_C_COMPILER=/usr/bin/cc" "-DCMAKE_CXX_FLAGS= -ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_CXX_COMPILER=/usr/bin/c++" "-DCMAKE_BUILD_TYPE=Debug"
-- The C compiler identification is GNU 4.6.3
-- The CXX compiler identification is GNU 4.6.3
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Configuring done
-- Generating done
-- Build files have been written to: /target/x86_64-unknown-linux-gnu/debug/build/mikktspace-f5ed98465e6e8dfa/out/build
running: "cmake" "--build" "." "--target" "install" "--config" "Debug" "--" "-j2"
Scanning dependencies of target mikktspace
[100%] Building C object CMakeFiles/mikktspace.dir/mikktspace.c.o
cc1: error: unrecognized command line option '-std=c11'
cc1: error: unrecognized command line option '-std=c11'
make[2]: *** [CMakeFiles/mikktspace.dir/mikktspace.c.o] Error 1
make[1]: *** [CMakeFiles/mikktspace.dir/all] Error 2
make: *** [all] Error 2
--- stderr
thread 'main' panicked at '
command did not execute successfully, got: exit code: 2

(https://travis-ci.org/bwasty/gltf-viewer/jobs/279266677)

Hint: cross doesn't use the newest compiler/lib versions for some targets, see https://github.com/japaric/cross#supported-targets for details.

I'm noticing this crate not being on version 1.0 yet even though it does feel fairly complete to me (I am sure there's more than enough stuff that still could be added or improved later though). I would like to know what is currently preventing us from reaching version 1.0?

The repository is already quite bloated through the removed Avocado textures - Github shows 8.56MB. Since the repo is young and unpublished I suggest rebasing master to remove it from the history.

@alteous What do you think?

Stable versions of Rust output this warning when compiling the latest mikktspace release on crates.io due to its dependency on nalgebra 0.26.2:

warning: the following packages contain code that will be rejected by a future version of Rust: nalgebra v0.26.2

Can a new release be cut so that affected users can update? Thank you!

Suggestion: Apache + MIT like gltf

Currently, mikktspace has an nalgebra feature (enabled by default) and a glam feature. This is discouraged as it means two libraries each depending on mikktspace might be incompatible with each other with no way for a user to fix it.

We ran into that incompatibility in our game, where the main game was depending on mikktspace with

mikktspace = { version = "0.3.0", default-features = false, features = ["glam"] }

but a crate that we depended on used

mikktspace = "0.3.0"

...causing a compilation error in mikktspace.

There are two pretty good approaches to solve this issue generally:

1. Pick a single math library to use internally. This can help with maintainability. Math libraries, especially glam, aren't too heavy on compilation times so this might not be so bad.
2. Inline the math functions into the crate. It looks like #23 is a step in that direction.

Make sure the packaged crate doesn't contain undesired files like .obj test data and the c example.

Reference: http://doc.crates.io/manifest.html#the-exclude-and-include-fields-optional

We use the mikktspace crate to generate tangents when loading models in our engine. Recently, we introduced bloom and found out that we've had some very rare NaN outputs in our fragment shader. We traced back the issue and found out that the mikktspace crate generates zero-length tangents occasionally, causing them to turn into NaN when normalized.

This model from the glTF Sample Model repository exhibits this issue: https://github.com/KhronosGroup/glTF-Sample-Models/blob/4ca06672ce15d6a27bfb5cf14459bc52fd9044d1/2.0/DamagedHelmet/glTF-Binary/DamagedHelmet.glb

On face 2923, vertex 2, a tangent of (0.0, 0.0, 0.0, -1.0) is generated by this crate. The model appears to have nothing weird about it, and Blender generates correct tangents for all vertices if set during export.

Hello!
I have been playing around with the Amethyst engine, which uses mikktspace 0.1 in its GLTF importer. When I was trying to run it on Windows 10, it failed during compilation with this error:

error: linking with `link.exe` failed: exit code: 1120
  |
  = note: "C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Community\\VC\\Tools\\MSVC\\14.24.28314\\bin\\HostX64\\x86\\link.exe" "/NOLOGO" "/NXCOMPAT" ... line too long to paste here
  = note: libmikktspace-ed048d53099c6187.rlib(mikktspace-ed048d53099c6187.mikktspace.24tkbvwf-cgu.0.rcgu.o) : error LNK2019: unresolved external symbol _genTangSpaceDefault@4 referenced in function __ZN10mikktspace8Closures8generate17hfe0598a16c7e4f72E
          C:\Users\mvask\Projects\amethyst\target\debug\examples\gltf.exe : fatal error LNK1120: 1 unresolved externals

It seems like there was a problem while compiling the C part of this library.

I've tried to update the dependency on mikktspace to the latest commit on master, since I've understood that since 0.1, there's been some work done to port the C parts of the library to pure Rust - which resolved my issue.

The problem with that approach is that it forces the use of a git dependency, since the last release of mikktspace I've found still has some parts written in C. As I've learned, using a git dependency has some drawbacks, which ultimately brings me here with a question:

Can a new, Rust-only version of mikktspace be released on crates.io? If not, what needs to be done?

The following snippet extracted from this crate failed under MIRI:

fn main() {
    unsafe {
        let mut piHashTable = vec![0i32; 10];
        
        let mut pTable = &mut piHashTable[2] as *mut i32;
        *pTable.offset(3) = 2 as i32;
    } 
}

error: Undefined Behavior: attempting a write access using <3294> at alloc1703[0x14], but that tag does not exist in the borrow stack for this location
--> src/main.rs:6:9
|
6 | *pTable.offset(3) = 2 as i32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| attempting a write access using <3294> at alloc1703[0x14], but that tag does not exist in the borrow stack for this location
| this error occurs as part of an access at alloc1703[0x14..0x18]
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <3294> was created by a retag at offsets [0x8..0xc]
--> src/main.rs:5:26
|
5 | let mut pTable = &mut piHashTable[2] as *mut i32;
| ^^^^^^^^^^^^^^^^^^^
= note: inside main at src/main.rs:6:9

This was noticed as part of bevyengine/bevy#4932, an effort to rewrite this crate into idiomatic Rust. There may be other soundness issues; the autogenerated code uses a lot of pointless unsafe in strange ways.