search

szellmann / visionaray

A C++-based, cross platform ray tracing library
https://vis.uni-koeln.de/visionaray.html
MIT License
429 stars 37 forks source link

PR: Changed some conditional preprocessor directives #30

Closed wldhg closed 5 years ago

wldhg commented 5 years ago

I'm now trying to build visionaray as a static library using MSVC. But MSVC does not supports syntax like this:

#if VSNRAY_HAVE_GLEW
#endif

So I changed these cases to:

#ifdef VSNRAY_HAVE_GLEW

(OR)

#if defined(VSNRAY_HAVE_GLEW)

It seems that you're preparing for supporting build on MSVC, so I'm making a PR.

Thanks for creating amazing libaray.

szellmann commented 5 years ago

Thanks for submitting the PR! Are you sure though that this is the right fix? The VSNRAY_HAVE_XXX defines come from auto-generated config headers and are either 0 or 1, i.e. they should be defined in any case, e.g. even if GLES is not available, there should be a define VSNRAY_HAVE_OPENGLES=0.

This here for example is the content of my config header:

// This file is distributed under the MIT license.
// See the LICENSE file for details.

#pragma once

#ifndef VSNRAY_CONFIG_H
#define VSNRAY_CONFIG_H 1

// This file is automatically generated.
// Changes will be lost the next time cmake is run.

#define VSNRAY_HAVE_BOOST 1
#define VSNRAY_HAVE_CUDA 0
#define VSNRAY_HAVE_GLEW 1
#define VSNRAY_HAVE_OPENGL 1
#define VSNRAY_HAVE_OPENGLES 0
#define VSNRAY_HAVE_TBB 1
#define VSNRAY_HAVE_THREADS 1

#endif // VSNRAY_CONFIG_H

I see that this could fail if the macro is not found and thus msvc thinks it's not defined at all, then it will maybe fail with a "not defined" error. Could it be that msvc just doesn't find the auto-generated config header(s) and thus assumes the macros aren't defined at all? Mine are located in visionaray/build/config/visionaray/, and visionaray/build/config/common/ where build is the build directory I created myself. If so, we'd probably have to fix the build system to find the auto-generated headers. (Note that msvc support is a bit stale currently, I only fire up Windows every once in a while and only if I have to, so any help at this front is highly appreciated :-) )

wldhg commented 5 years ago

Oh, I misunderstood the functionality of #cmakedefine01... I'm now very ashamed of my ignorance... Thanks for your teachings...!!!

szellmann commented 5 years ago

Haha, no problem. cmake can be overly complicated sometimes. Does this mean it now works for you or is there still an issue that needs fixing?

wldhg commented 5 years ago

It works well! Thanks very much, szellmann.