andriyDev / recastnavigation-rs-sys

Raw Rust FFI bindings for recastnavigation.
MIT License
7 stars 2 forks source link

Missing functions when compiling against 32bit targets #3

Closed Sunderous closed 1 year ago

Sunderous commented 1 year ago

Hey,

Sorry this is a bit of a niche problem, but my use case for recast/detour in rust requires I compile it against x86 and not x64 and produce a 32 bit binary.

Here's the command I'm using to build a 32 bit copy from source: rustup run nightly-i686-pc-windows-msvc cargo build --features detour,recast,detour_tile_cache

It's the same way I'm building my 32bit DLL that I'd like to use this library within. It says it successfully builds, and it does generate output files.

But the bindings are missing the actual functions for things, ie: dtNavMesh_init() does not get produced in the detour.rs output file.

I've verified if I do a normal 64 bit cargo build, with a 64 bit clang lib, it will produce the bindings correctly. But I can't use that in my project, so I'm kind of stuck.

Any ideas what I might be missing here?

Thanks for any insight.

andriyDev commented 1 year ago

Welp you aren't missing anything, I was able to reproduce this on my machine as well! I also tried using stable 32-bit Rust and it also failed. I'll dig in soon(ish), I'm betting there's something wrong with CMake whether its Rust's cmake crate or Recast's CMakeLists.

Sunderous commented 1 year ago

Awesome, thank you!

I did as much digging as I could, there's only 2 related github issues I could find:

rust-bindgen #1178

rust-bindgen #1739

One talks about needing to use nightly so that the 32 bit "thiscall" ABI the functions would be using is available. But I'm using nightly as far as I can tell with my installed i686 toolchain.

The other talks about possibly needing a special clang argument to make them visible, but I tried patching that into the build.rs file locally in a couple spots and still no dice:

.clang_arg("-fvisibility=default")

I'm mostly an FFI noob, so that's really as far as I got. If I have time I might see if I can enable more verbose output and see if anything is in the clang or cmake logs that at least says why those methods are being skipped in case it's not related to thiscall ABI at all (but it definitely sounds like it should be).

andriyDev commented 1 year ago

Alright, so it does seem to be a bindgen issue and your thiscall thing was dead on! With the bindgen CLI, I was able to make it work with nightly Rust and passing --rust-target=nightly. I have no idea why it needs the additional --rust-target flag.

I'm gonna see if I can pass this flag in using the bindgen crate somehow. Maybe at least we can have a feature or something? Or detect if we're compiling for 32 bit and automatically fix this up?

andriyDev commented 1 year ago

@Sunderous do you mind checking out #5 and see if it's a good solution? In case I'm missing something obvious haha.

Sunderous commented 1 year ago

@andriyDev

Hey, I'm not an expert, but assuming those 2 things are handled by cargo/rustup correctly that seems reasonable, at least as a patch. Hopefully not other weird architecture specific hacks come up later haha.

The only thing I'd maybe suggest as a nitpick would be this line: println!("cargo:warning=Windows 32 bit uses the \"thiscall\" ABI. This feature is not enabled, so compilation may fail!");

That warning message should probably mention that the feature is available when using nighty rust? Or suggest trying to build with nightly rust or something after explaning that it's not currently enabled.

But other than that, excited to try it out!

I saw that rust-target thing in another issue, and it wasn't until you commented that I put together it's a bindgen specific parameter not a rust/cargo parameter which explains why my experiments with it produced nothing but command errors haha.

Thanks again!

andriyDev commented 1 year ago

Ah yes I completely missed that, thank you! I'll make a release in a moment and it should all work!

Sunderous commented 1 year ago

I just built it with the branch right before you merged it haha, and it looks good.

My actual recast/detour usage needs some fixing, but the bindings are created properly and it runs so looks good on your end.

Thanks again for the super fast turnaround!