bjornblissing / osgoculusviewer

An OsgViewer with support for the Oculus Rift
Other
106 stars 67 forks source link

Added API export/import directive, declared in oculusconfig.h #108

Open rickyviking opened 3 years ago

rickyviking commented 3 years ago

Useful when building as DLL and want to export classes.

bjornblissing commented 3 years ago

When reading about how to best handle the BUILD_SHARED_LIBS I came across the following blog post: https://blog.kitware.com/create-dlls-on-windows-without-declspec-using-new-cmake-export-all-feature/

If I understands this correct all of the above could be replaced by simply adding the following to the master CMakeLists.txt:

IF(BUILD_SHARED_LIBS)
  SET(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON).
ENDIF()

Documentation here: https://cmake.org/cmake/help/v3.4/prop_tgt/WINDOWS_EXPORT_ALL_SYMBOLS.html

rickyviking commented 3 years ago

Interesting, but the WINDOWS_EXPORT_ALL_SYMBOLS automatically creates a .def file for the dll exported symbols. This is a different strategy to link against a DLL compared to the use of __declspec(dllexport)/import directives proposed in the PR.

bjornblissing commented 3 years ago

Interesting, but the WINDOWS_EXPORT_ALL_SYMBOLS automatically creates a .def file for the dll exported symbols. This is a different strategy to link against a DLL compared to the use of __declspec(dllexport)/import directives proposed in the PR.

Do you know if one option is preferable to the other?

rickyviking commented 3 years ago

In my experience I've always seen the dllexport/dllimport directives used in cross platform projects, I believe mainly as they are natively "embedded" in the source code, and avoid extra effort of keeping the .def file in sync.

bjornblissing commented 3 years ago

This article from Microsoft explains the benefits and drawbacks with each solution: https://docs.microsoft.com/en-us/cpp/build/determining-which-exporting-method-to-use

My interpretation is that if you want to change the library without having to rebuild the application that depends on a certain DLL file, then you have to use the .def file approach. Since __declspec(dllexport) might mangle the names differently for each new build.

rickyviking commented 3 years ago

Sure, little advantage in my opinion, provided that 1) you can maintain binary compatibility across a number of versions at project level 2) different compiler versions may mangle the names in a different way, but with MSVC you need to ensure same compiler anyway, as mixing different runtime libraries (to use the STL for instance) might give issues.

IMHO anyone using a 3rd party library like osgOculusViewer would recompile anyway when changing compiler. The dllexport/import approach is used in all the most popular projects I've seen, notably OpenSceneGraph, VulkanSceneGraph, Dear ImGui, JsonCpp, etc.

As it's evident I'm slightly inclined towards the dllexport directives :) but of course that's you choice to make!

bjornblissing commented 3 years ago

I do agree with your conclusion, even though it makes me a tiny bit sad to have to decorate the code with all these OSGOCULUS_EXPORT preprocessor definitions.

rickyviking commented 3 years ago

ahah I understand, as I said it's up to you. As I keep a decorated version, I thought it might be useful to share the work