Skip to content

Conversation

xlz
Copy link
Member

@xlz xlz commented Jun 10, 2015

Previous attempt: #282.

Static Protonect.exe is easier for testing as it does not require managing DLLS. As a result, only libfreenect2static.lib is provided by default. (Only libusb, OpenCV, TurboJPEG and GLFW are statically linked now. OpenCL not.)

This is because it is impossible to import both dynamic and static versions of OpenCV at the same time, also building both dll and lib requires recompiling objects with /MD and /MT (dll needs /MD, lib needs /MT).

libfreenect2static.lib is compiled with /MT by default for static linking with Protonect.exe. If BUILD_SHARED_LIBS is true, it is compiled with /MD (not ideal, but if the user want static library, don't enable this).

Dynamic verison of Protonect.exe and libfreenect2.dll can be built with -DBUILD_SHARED_LIBS=ON. Manual DLL management is required to run dynamic Protonect.exe.

xlz added 2 commits June 10, 2015 05:50
Static Protonect.exe does not require managing DLLS. As a result,
only libfreenect2static.lib is provided by default. (Only libusb,
OpenCV, TurboJPEG and GLFW are statically linked now. OpenCL not.)

This is because it is impossible to import both dynamic and static
versions of OpenCV at the same time, also building both dll and
lib requires recompiling objects with /MD and /MT.

libfreenect2static.lib is compiled with /MT by default for static
linking with Protonect.exe. If BUILD_SHARED_LIBS is true, it is
compiled with /MD.

Dynamic verion of Protonect.exe and libfreenect2.dll can be built
with -DBUILD_SHARED_LIBS=ON. Manual DLL management is required
to run dynamic Protonect.exe.
@HenningJ
Copy link
Contributor

Ok, now I see the problem you had before. Lots of previously defined symbols while linking. That's because my OpenCV was compiled with /MD, right? If I change it back to /MD, it works.
But since my OpenCV configuration is non-standard, you could just ignore that.

Or maybe there is some nice generic solution? What happens when using dynamic version of OpenCV with the static versions of everything else and then compiling with /MD? From what I gather, it's only OpenCV that has a problem with mixing /MT and /MD. And when you installed OpenCV, you'll most likely have the required DLLs in your PATH anyway, so you wouldn't have to worry about copying them around.

Other notes:

  • You could make BUILD_SHARED_LIBS an CMake option, like ENABLE_OPENCL. It's exactly the kind of thing the options are there for, right?
  • When building the dynamic version, you could still install the DLLs like you tried in Copy DLLs for Protonect on Windows #282.

@xlz
Copy link
Member Author

xlz commented Jun 10, 2015

OpenCV installer does not set PATH so we can't assume OpenCV DLLs are in PATH.

This patch implemented exactly what you said: using BUILD_SHARED_LIBS to toggle linking mode.

Copying DLLs is totally a hack. You made good point that static binary is easier to debug because of less DLL environment setup. The previous PR's heuristics can totally fail, and it does not integrate nicely with VS build folder hierarchy.

@HenningJ
Copy link
Contributor

OpenCV installer does not set PATH so we can't assume OpenCV DLLs are in PATH.

Ok. But still, it would be no worse than now, and better in many ways. You'd have to make sure the OpenCV DLLs are found only once and still could build static and dynamic versions of libfreenect2 at the same time.
But as I said, it doesn't really matter to me, since I might be a very special case. :-)

This patch implemented exactly what you said: using BUILD_SHARED_LIBS to toggle linking mode.

Yes, it does. But when you define it using the cmake OPTION() command, it shows up in the cmake gui and can be modified there. This way, it's pretty much hidden and you have to know about it to use it. Not a big thing, but still nice.

@xlz
Copy link
Member Author

xlz commented Aug 25, 2015

Closed in favor of #282.

The problem with static binary is the confusion between /MD and /MT which is hard to control given various prebuilt libraries.

@xlz xlz closed this Aug 25, 2015
@xlz xlz deleted the windows-static branch November 18, 2015 11:44
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.

3 participants