Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows build support #7

Merged
merged 13 commits into from
Dec 19, 2022
Merged

Conversation

DSchroer
Copy link
Contributor

Add the necessary build parameters in order to cross compile for windows.

Test with x86_64-pc-windows-gnu.

Copy link
Owner

@bschwind bschwind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this! I can't immediately verify it on a windows machine myself, so I might wait until I try it out on a machine at work.

Side-note: I'd appreciate if you base your commits on my main branch so the commits tab in github only shows the new work you've done for this PR. Thanks!

@@ -36,6 +37,8 @@ fn main() {
cxx_build::bridge("src/lib.rs")
.cpp(true)
.flag_if_supported("-std=c++11")
.define("_USE_MATH_DEFINES", "TRUE")
.include(format!("{}", dst.join("inc").display()))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is CMake generating this directory only for windows targets?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be. The structure is different in subtle ways.

One option is to add conditionals to enable/disable these different paths. I opted not to do that yet. Once a pattern emerges it might be a good idea.

Im not sure what OCCT does for macos or webassembly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried for macos. It uses the lib directory.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, I should mention that I develop mainly on MacOS so that target should be well tested.

I'm currently not planning to target web assembly, though I may have plans in the future to have a model viewer that accepts web assembly blobs which generate instructions for generating an opencascade model.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I am primarily on Linux myself so that covers those two. I will probably need webassembly at some point but I am in no rush for it.

I'm working on a project similar to OpenSCAD. Last year I did the webassembly port of OpenSCAD and judging from the community reaction, I will probably need it for this project as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha I wanted fillets and chamfers as well. Got those in first thing.

I made an adapter for my cars cup holder so I can fit my favorite mug in it. So I can say its usable. However there is lots to do before its ready for real use. However I did get a little 3D editor in it so it really feels like working with OpenSCAD at this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldnt have gotten so far if this crate didnt exist. I really appreciate the work you did here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to leave issues and suggestions for things if you want. Especially syntax for the language since that needs lots of inspiration.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool to get a usable, real-world object as a result of some Rust hacking :)

I did get a little 3D editor in it so it really feels like working with OpenSCAD at this point.

MacOS has a native STL viewer that I've been using as a crutch, otherwise I probably would have gotten frustrated and made a viewer as well.

I wouldnt have gotten so far if this crate didnt exist. I really appreciate the work you did here.

Thank the opencascade devs! And dtolnay for the cxx crate :) I'm just gluing things together here, though I did suffer through the initial work of getting build.rs working properly, I'll take credit for that haha

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll clone it and give it a spin!

@DSchroer
Copy link
Contributor Author

Side-note: I'd appreciate if you base your commits on my main branch so the commits tab in github only shows the new work you've done for this PR. Thanks!

Im trying. Maintaining a fork is not something I am used to. Especially where I need a main branch to work on my changes as well with potentially multiple of the PRs merged. Ill do my best to keep it simple in the future.

@bschwind
Copy link
Owner

@DSchroer let me know if you need any help with git operations.

My basic workflow for forks is to have two remotes. As an example, here's my local git repo for cameleon, a crate I've contributed to which I don't own:

cameleon $ git remote -v
bschwind	[email protected]:bschwind/cameleon.git (fetch)
bschwind	[email protected]:bschwind/cameleon.git (push)
origin	https://github.com/cameleon-rs/cameleon.git (fetch)
origin	https://github.com/cameleon-rs/cameleon.git (push)

You can add remotes with git remote add <REMOTE_NAME> <REMOTE_URL>. Example:

git remote add bschwind [email protected]:bschwind/cameleon.git

You can then fetch branches with git fetch bschwind.

To checkout my latest main branch, assuming your remote for my repo is called bschwind, you would run:

git fetch bschwind

# checks out a local branch named "bschwind/main"
git switch -c bschwind/main

You can then create a new branch based on bschwind/main with:

git checkout -b your-feature-name

When done with your work, push to your remote and make a PR:

git push origin your-feature-name

When you want to update bschwind/main with the latest changes:

git checkout bschwind/main
git pull bschwind main

@DSchroer
Copy link
Contributor Author

Thanks! Ill use that for future PRs. I appreciate the detailed response.

@DSchroer
Copy link
Contributor Author

Just a note. You can test this on linux if you do the following:

rustup toolchain install nightly-x86_64-pc-windows-gnu
rustup target add x86_64-pc-windows-gnu
cargo build --release --target x86_64-pc-windows-gnu

Just make sure that you have mingw-w64 installed as well.

@bschwind
Copy link
Owner

Still waiting to test this on a windows machine, sorry! Work is busy this week until Thursday, I'll try to test it then.

@bschwind
Copy link
Owner

On the msvc toolchain, after adding the correct linker path, I get this output:

  = note:    Creating library C:\Users\Maker\projects\opencascade-rs\target\release\examples\bottle.lib and object C:\Users\Maker\projects\opencascade-rs\target\release\examples\bottle.exp
          libopencascade_sys-b15f482ef2568063.rlib(OSD_signal.obj) : error LNK2019: unresolved external symbol __imp_MessageBoxA referenced in function "long __cdecl CallHandler(unsigned long,struct _EXCEPTION_POINTERS *)" (?CallHandler@@YAJKPEAU_EXCEPTION_POINTERS@@@Z)
          libopencascade_sys-b15f482ef2568063.rlib(OSD_signal.obj) : error LNK2019: unresolved external symbol __imp_MessageBeep referenced in function "long __cdecl CallHandler(unsigned long,struct _EXCEPTION_POINTERS *)" (?CallHandler@@YAJKPEAU_EXCEPTION_POINTERS@@@Z)
          C:\Users\Maker\projects\opencascade-rs\target\release\examples\bottle.exe : fatal error LNK1120: 2 unresolved externals

I'll investigate this more later but I have to leave this machine for now.

@bschwind
Copy link
Owner

Hi @DSchroer,

I have a PR on your branch here (a bit confusing, I know) to get the build working on the native windows rust toolchain. If it looks good, we can merge that into your branch, then I think the PR here will be good to go!

Make the build work on windows msvc targets
@DSchroer
Copy link
Contributor Author

Hey. I built your changes against my project and it worked just fine. So I merged them. Now I run into a weird error with the examples though. My project gives no errors but the examples give me:

ustlib/x86_64-pc-windows-gnu/lib" "-o" "/home/dschroer/Projects/opencascade-rs/target/x86_64-pc-windows-gnu/debug/examples/simple-82e2bede0d1a71a5.exe" "-Wl,--gc-sections" "-no-pie" "-nodefaultlibs" "/home/dschroer/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-pc-windows-gnu/lib/rsend.o"
  = note: /usr/lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld: /home/dschroer/Projects/opencascade-rs/target/x86_64-pc-windows-gnu/debug/deps/libopencascade_sys-9ec0e326a19c17a9.rlib(Standard_ErrorHandler.cxx.obj): in function `Standard_ErrorHandler::Callback::RegisterCallback()':
          /home/dschroer/Projects/opencascade-rs/crates/opencascade-sys/OCCT/src/Standard/Standard_ErrorHandler.cxx:289: multiple definition of `Standard_ErrorHandler::Callback::RegisterCallback()'; /home/dschroer/Projects/opencascade-rs/target/x86_64-pc-windows-gnu/debug/deps/libopencascade_sys-9ec0e326a19c17a9.rlib(wrapper.o):/home/dschroer/Projects/opencascade-rs/target/x86_64-pc-windows-gnu/debug/build/opencascade-sys-ff866f5ac74453b6/out/inc/Standard_ErrorHandler.hxx:221: first defined here
          /usr/lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld: /home/dschroer/Projects/opencascade-rs/target/x86_64-pc-windows-gnu/debug/deps/libopencascade_sys-9ec0e326a19c17a9.rlib(Standard_ErrorHandler.cxx.obj): in function `Standard_ErrorHandler::Callback::UnregisterCallback()':
          /home/dschroer/Projects/opencascade-rs/crates/opencascade-sys/OCCT/src/Standard/Standard_ErrorHandler.cxx:306: multiple definition of `Standard_ErrorHandler::Callback::UnregisterCallback()'; /home/dschroer/Projects/opencascade-rs/target/x86_64-pc-windows-gnu/debug/deps/libopencascade_sys-9ec0e326a19c17a9.rlib(wrapper.o):/home/dschroer/Projects/opencascade-rs/target/x86_64-pc-windows-gnu/debug/build/opencascade-sys-ff866f5ac74453b6/out/inc/Standard_ErrorHandler.hxx:224: first defined here
          collect2: error: ld returned 1 exit status

Ive got no idea where those are coming from at this point. Any ideas? I probably wont have time to look deep for a few weeks right now. However we can let this sit on ice until then.

Can you build the examples with MSVC?

@bschwind
Copy link
Owner

bschwind commented Dec 9, 2022

Hi @DSchroer sorry to get back to you so late on this!

I am able to build and run the bottle example on the MSVC toolchain. I'll try reproducing your error with x86_64-pc-windows-gnu.

@bschwind
Copy link
Owner

bschwind commented Dec 9, 2022

@DSchroer actually I haven't used mingw in forever on windows, do you have a list of steps I can follow to reproduce the issue you're seeing, on a windows machine specifically? That'll probably be faster than me struggling with windows for hours 😅

@DSchroer
Copy link
Contributor Author

@bschwind I have some time to look into it now.

For repro steps on linux (I am corss compiling):

  1. Install the mingw toolset
  2. Run cargo build --target x86_64-pc-windows-gnu --example simple

@DSchroer
Copy link
Contributor Author

Got it fixed. We need to set the OCC_CONVERT_SIGNALS define on windows builds. https://dev.opencascade.org/doc/overview/html/occt_user_guides__foundation_classes.html

cxx_build::bridge("src/lib.rs")
.cpp(true)
.flag_if_supported("-std=c++11")
.include(format!("{}", dst.join("include").join("opencascade").display()))
.define("_USE_MATH_DEFINES", "TRUE")
.define("OCC_CONVERT_SIGNALS", occ_convert_define)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this page

On Windows with MSVC compiler, exceptions can be thrown directly from signal handler, this OCC_CONVERT_SIGNALS is not needed. Note however that this requires that compiler option /EHa is used.

Not sure if we need /EHa here or not. I built this on the default MSVC toolchain you get with a windows Rust installation and I get two linking errors:

unresolved external symbol "public: void __cdec1 Standard_ErrorHandler::Callback::RegisterCallback(void)"
unresolved external symbol "public: void __cdec1 Standard_ErrorHandler::Callback::UnregisterCallback(void)"

Currently still working this out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ive limited the flag to only *-windows-gnu builds. It should fix the error for you now.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DSchroer I think if it's not *-windows-gnu you don't want to define OCC_CONVERT_SIGNALS at all. On MSVC, if you set occ_convert_define to None, you're still defining OCC_CONVERT_SIGNALS and I still get the issue I mentioned above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh shoot. I assumed based on the interface .define("thing", None) would disable it. Ill look into it soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ive done another pass. This time its only ever set for windows-gnu. Hopefully this works.

Copy link
Owner

@bschwind bschwind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Tested and working on the MSVC toolchain, sorry for all the back-and-forth on this but glad we finally got it working!

@DSchroer
Copy link
Contributor Author

Awesome! Want to merge the PR?

@bschwind
Copy link
Owner

Haha yep, got distracted right after I commented that, sorry!

Thanks for the PR and patience :)

@bschwind bschwind merged commit 0e4696e into bschwind:main Dec 19, 2022
@bschwind bschwind mentioned this pull request Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants