martijnberger / clew Goto Github PK

Hi!

I've looked through the project build configuration, and I found few moments I can improve it:

• Merge all CMakeLists.txt into a the topmost one. You cannot build only part of the project by issuing cmake ../src or cmake ../clewTest, so why have them separated at all?
• Options like BUILD_TESTS or BUILD_SHARED_LIBRARY should have CLEW_ prefix in their name.
• Maybe rename clewTest directory to clew_test or simply test?

What you you think about the proposal?
Option renaming is a breaking change, so I'd want to discuss it prior PR'ing. See the EasyCL issue discussion for more information why this rename maybe a right thing to do.

has anyone tested this library with the cl.hpp binder?
Thanks.

Just ran into this building against blender's copy of clew, inside /blender/extern/clew path off of Trunk.

I don't know why they didn't roll it up to you but they've modified clew.c and clew.h making it useless against their new split kernel.

You can get their changes that compile Blender with no mistakes from blender.git.

There is no information about how this code is licensed and copyrighted. Would be convenient, if this is added.

Blender has a need for libGLEW.so instead of libglew.so and fails to build without it. On Linux I just made a ln -s libglew.so libGLEW.so but I imagine you can make a cleaner solution by providing a link installed version of it.

There is some strange code in clew.h:

// #define CLEW_STATIC

#ifdef _WIN32
  #ifdef clew_STATIC
  #  define CLEWAPI extern
  #else
  #  ifdef clew_EXPORTS
#pragma message("exporting")
  #    define CLEWAPI extern __declspec(dllexport)
  #  else
#pragma message("importing")
  #    define CLEWAPI extern __declspec(dllimport)
  #  endif
  #endif
#else
#  define CLEWAPI extern
#endif

1. clew_STATIC is a strange combination of lower and upper case letters.
2. If producing static lib is the default, clew should have CLEW_DYNAMIC which should NOT be defined by default. There should be a note saying that CLEW_DYNAMIC needs to be defined (or uncommented) for .dll builds.

clGetGLContextInfoKHR (https://www.khronos.org/registry/OpenCL/sdk/1.2/docs/man/xhtml/clGetGLContextInfoKHR.html) is defined in the cl_khr_gl_sharing (https://www.khronos.org/registry/OpenCL/sdk/1.2/docs/man/xhtml/cl_khr_gl_sharing.html) extension. However, clew doesn't necessarily always find it.

That's because clGetGLContextInfoKHR is an extension function, and according to https://www.khronos.org/registry/OpenCL/sdk/1.0/docs/man/xhtml/clGetExtensionFunctionAddress.html

For functions that are queryable with clGetExtensionFunctionAddress, implementations may choose to also export those functions statically from the object libraries implementing those functions. However, portable applications cannot rely on this behavior.

Since clew uses dlsym-like mechanism, it fails to find the function.

AFAIK that's the only extension function currently exposed by clew. We wound need a proper mechanism to get it, i.e. clGetExtensionFunctionAddress() if we don't want to use the platform, or clGetExtensionFunctionAddressForPlatform() if we want to do it right.

That might require introducing another version of clew initialization that is called with a platform ID to initialize the functions for a given platform.