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

Remove dynamic configuration requirement #89

Open
jherico opened this issue Nov 24, 2014 · 14 comments
Open

Remove dynamic configuration requirement #89

jherico opened this issue Nov 24, 2014 · 14 comments
Assignees

Comments

@jherico
Copy link

jherico commented Nov 24, 2014

I really like oglplus' utility as a header only library, but I find it frustrating that configuration step is necessary, especially when I'm working with a project where I'm including oglplus as a submodule. I don't want to configure my CMake to recursively call cmake for oglplus. Currently to get around this I have to trick my cmake into running the required static config by changing the project root temporarily ( see https://github.com/jherico/OculusRiftInAction/blob/master/CMakeLists.txt#L125 )

It would be a lot simpler (for library users granted, not the library maintainer) if oglplus used compiler identification and versioning for determining the available feature set (similar to https://github.com/jherico/OculusSDK/blob/master/LibOVR/Src/Kernel/OVR_Compiler.h or https://github.com/g-truc/glm/blob/master/glm/detail/_features.hpp ). This issue actually blocked me from adopting oglplus much sooner than I did.

@matus-chochlik matus-chochlik self-assigned this Nov 25, 2014
@matus-chochlik
Copy link
Owner

For C++ features this can be done and it is done by using the Boost.Config library. You just have to define OGLPLUS_NO_SITE_CONFIG=1 before including anything from OGLplus and you don't need the site-config.hpp header generated by CMake.

The tricky part is the detection of available OpenGL version / extensions and right now I don't see how this can be done reliably without compiling and running a couple of test programs.
With some libraries (most notably with GLEW) you cannot depend on the GL_VERSION_* and GL_EXT_* macros being defined correctly because with almost any version of GLEW there is something missing or is not implemented according to the specs, which then causes compilation errors in OGLplus.

Because of that the CMake config detects what really is available and then generates the fix_gl_version.hpp header. So if you want to bypass the OGLplus configuration in your project you can generate your own fix_gl_version.hpp to target the version that your project requires and ship it together with your sources.

@jherico
Copy link
Author

jherico commented Nov 26, 2014

I'll have to take a look at the OGLPLUS_NO_SITE_CONFIG setting. I believe I've tried it before, but didn't get it to work, perhaps because I'm not using Boost (too hard to embed in my example repository, too involved to force readers of my book to install it).

I see the fix_gl_version.hpp.in file and the pre-built one /etc/msvc11/OGLplus/oglplus/config/, but my built CMake output doesn't seem to include it and it's not causing any harm or errors. Perhaps that's because of the way I'm including the oglplus code in my project and forcing the use of GLEW.

set(SAVE_PROJECT_SOURCE_DIR ${PROJECT_SOURCE_DIR})
set(PROJECT_SOURCE_DIR ${PROJECT_SOURCE_DIR}/libraries/oglplus)
    include(libraries/oglplus/config/Compiler.cmake)
    include(libraries/oglplus/config/CPPFeature.cmake)
    set(OGLPLUS_CONFIG_SET_LOW_PROFILE 1)
    set(OGLPLUS_LOW_PROFILE 0)
    set(OGLPLUS_USE_GLCOREARB_H 0) 
    set(OGLPLUS_USE_GL3_H 0)
    set(OGLPLUS_USE_GLEW 1)
    set(OGLPLUS_USE_GL3W 0)
    set(OGLPLUS_USE_BOOST_CONFIG 0)
    configure_file(
        ${PROJECT_SOURCE_DIR}/config/oglplus/config/site.hpp.in
        ${PROJECT_BINARY_DIR}/libraries/oglplus/include/oglplus/config/site.hpp
    )
set(PROJECT_SOURCE_DIR ${SAVE_PROJECT_SOURCE_DIR})

@matus-chochlik
Copy link
Owner

The Boost.Config library is quite lightweight and I'm considering that I'll start distributing it with OGLplus source in the third_party directory. I'm using OGLplus as a header-only library in a couple of my projects without running the configure script and the OGLPLUS_NO_SITE_CONFIG option was not giving me any problems so far.

Concerning the fix_gl_version.hpp header; it basically #undefs the GL_VERSION_X_Y macros if they are defined but the build system determines that something is actually missing. It is right now included only by the 'example harnesses', not by any of the OGLplus' headers. If the GL library you are using defines everything as it is supposed to, then you don't need to include fix_gl_version.hpp.

Using the build system is generally not a requirement for using OGLplus in applications (especially with modern compilers) and it is there mostly to ensure that the examples which can be built do build without errors.

@jherico
Copy link
Author

jherico commented Nov 27, 2014

I tried grabbing the boost config code and replacing the config stuff with this:

add_definitions(-DOGLPLUS_USE_BOOST_CONFIG)
add_definitions(-DOGLPLUS_NO_SITE_CONFIG)

But I get a ton of build errors inside the oglplus headers. I'll try to narrow down the exact cause. The Boost config headers are definitely getting included (I tried putting some #error directives in the #if block where it's included to make sure)

@matus-chochlik
Copy link
Owner

Could you paste here/somewhere the output of make?

@jherico
Copy link
Author

jherico commented Dec 1, 2014

Here's the output of compiling a small C++ file:
https://gist.github.com/jherico/94944ab1e57f62621409

the file consists of the following:

#include <Windows.h>

#define OGLPLUS_USE_BOOST_CONFIG 1
#define OGLPLUS_NO_SITE_CONFIG 1
#include <oglplus/config/gl.hpp>
#include <oglplus/all.hpp>


int __stdcall WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nCmdShow) {
  return 0;
}

@matus-chochlik
Copy link
Owner

You don't include any 'GL library' like glcorearb.h or glew.h or whatever. The oglplus/gl/config.hpp does not do this for you. You should #define one of the OGLPLUS_USE_GLCOREARB, OGLPLUS_USE_GL3_H, OGLPLUS_USE_GLEW or OGLPLUS_USE_GL3W macros to a positive integer value and include oglplus/gl.hpp or if you know that you always will be using for example GLEW then include directly GL/glew.h before including anything else from OGLplus.

@jherico
Copy link
Author

jherico commented Dec 1, 2014

I was attempting to create a 'minimal example' but I think I over-minimized it. In my full code I do include GLEW before the oglplus stuff, but I don't think I'm setting the OGLPLUS_USE_GLEW in the test I'm doing. will try.

@jherico
Copy link
Author

jherico commented Dec 1, 2014

OK, adding OGLPLUS_USE_GLEW and the glew header makes it compile (on my mac at least, with mac specific changes to the main decl). It would be helpful if in the absence of any GL header if oglplus emitted a #warning or #error to let the developer know. The error output wasn't really leading me anywhere but to preprocessor macros that seemed to be related to C++ feature detection.

@jherico
Copy link
Author

jherico commented Dec 2, 2014

Ugh... nope, still not compiling on Windows. I've winnowed my test file down to this:

#include <GL/glew.h>
#define OGLPLUS_USE_GLEW 1
#define OGLPLUS_USE_BOOST_CONFIG 1
#define OGLPLUS_NO_SITE_CONFIG 1
#include <oglplus/all.hpp>

And the output I get is here: https://gist.github.com/jherico/3ae4124e00a6f4369076

I'm using Visual Studio 2013 Community edition, commit 6a48fc4 from oglplus and the boost config headers from a just downloaded version of boost.

If I explicitly add the following defines it compiles:

#define OGLPLUS_NO_DEFAULTED_FUNCTIONS 1
#define OGLPLUS_NO_VARIADIC_TEMPLATES 1
#define OGLPLUS_NO_INHERITED_CONSTRUCTORS 1

So it seems like either the boost config is misidentifying the support for these items, or OGLplus is using the features in some way that's not actually compliant, or possibly that VS2013's implementation is non-compliant.

@matus-chochlik
Copy link
Owner

Well, MSVC 'sort of' supports these features, but the implementation is horribly broken (at least in MSVC 2012 and older, I didn't try MSVC 2013 yet). I'm fairly certain that the lines that the MSVC complains about are valid, standard C++ and both g++ and clang++ compile them without warnings even in pedantic mode. So until Microsoft fixes these problems you probably need to set the macros manually, if Boost (IMO incorrectly) reports these features as supported.

@jherico
Copy link
Author

jherico commented Dec 2, 2014

Yeah, here's the line:

For defaulted functions, the use of =default to request member-wise move constructors and move assignment operators is not supported.

@matus-chochlik
Copy link
Owner

Yes and the other errors in the output come from bugs in the implementation of variadic templates.
I'll probably hardcode the values of some the OGLPLUS_NO_* macros to 1 on MSVC regardless of what Boost.Config says.

@matus-chochlik
Copy link
Owner

OK, I've updated oglplus/config/compiler.hpp to #define OGLPLUS_NO_DEFAULTED_FUNCTIONS, OGLPLUS_NO_VARIADIC_TEMPLATES and OGLPLUS_NO_INHERITED_CONSTRUCTORS as 1 if the _MSC_VER PP symbol is defined (unless these values are already set).

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

No branches or pull requests

2 participants