Replace GLEW with glbinding #89

9prady9 · 2016-05-09T13:02:12Z

Fixes #85
Fixes #101

Also fixes, virtual trackball rotation based interaction for 3d chart objects.

All examples also use glbinding from now, but OpenCL examples are not functional yet due incompatibility between gl.h header included by opencl headers and glbinding header

shehzan10 · 2016-05-09T15:46:15Z

@9prady9 Can you detail the OpenCL issue? And perhaps file a bug request with glbinding?

shehzan10 · 2016-05-09T15:50:06Z

@arrayfire/core-devel I support merging this PR. However, I understand that there are concerns, among other things, about

CMake minimum version incremented to 3.0 (this will need an increment on ArrayFire too)
glBinding not being as widely available as GLEW.

The later of those is not such a big issue as we can make glBinding is an external build. So it will be built from source. We could add an option to use system one as well.

Upgrading CMake may be an issue. This is the one that we should take with caution.

shehzan10 · 2016-05-09T15:51:49Z

CMakeModules/build_glbinding.cmake

+    -DCMAKE_C_FLAGS:STRING=${CFLAGS}
+    -DCMAKE_BUILD_TYPE:STRING=${CMAKE_BUILD_TYPE}
+    -DCMAKE_INSTALL_PREFIX:STRING=${prefix}
+    -DBUILD_SHARED_LIBS:BOOL=ON


We want this to be static. Is there any reason why it's shared?

Nothing of concern i guess, start started off like that.

pavanky · 2016-05-09T15:58:41Z

I don't think we should use a library that's not widely used if there isn't a strong enough reason to do it.

shehzan10 · 2016-05-09T16:07:12Z

Among others, are the following advantages:

It is type safe (GLEW uses enums everywhere and it can be messed around with)
C++11, templates, lambdas etc
Availability:
- On Ubuntu its available via a PPA https://launchpad.net/~cginternals/+archive/ubuntu/ppa
- On Arch, there is an AUR package
- OSX it is available on brew
- There is an installer for Windows
In my opinion, it has better multi-context and multi threading support

I don't know the amount of work that would be required, but I would be fine supporting both GLEW and glBinding similar to how we support GLFW and SDL. GLEW and glbinding are easily interchangeable as GLEW uses glFunction and glbinding uses gl::Function.

shehzan10 · 2016-05-09T16:16:02Z

CMakeModules/build_glbinding.cmake

+ExternalProject_Add(
+    glb-ext
+    GIT_REPOSITORY https://github.com/cginternals/glbinding.git
+    GIT_TAG v2.0.0


They have v2.0.1 out as of April 6.

I have seen v2.0.1 and tried it out, i faced some issues while compiling against that version. This commit from v2.0.1 is conflicting with standard gl.h header. I haven't figured how to resolve that elegantly yet.

9prady9 · 2016-05-09T16:40:45Z

@arrayfire/core-devel I think it is good if we just pick one loader. To support both would be lot of pain that is not the worth spending time on.

We have two options:

Stick with GLEW in hope that they will release 2.0 soon (there is no known timeline on this as per their github page) that fixes the problems with GLEW (cannot use in conjunction with Qt - thats one problem apart from being void of other things @shehzan10 mentioned above).
Use glbinding

9prady9 · 2016-05-09T16:54:13Z

@shehzan10 Regarding OpenCL, i didn't get the time to resolve that one before i pushed the branch. Will do soon. It is probably something simple. Will let you know once i figure out whats the issue.

UPDATED:

In file included from /home/pradeep/gitroot/forge/examples/opencl/fractal.cpp:15:0:
/home/pradeep/gitroot/forge/examples/opencl/cl.hpp: In constructor ‘cl::ImageGL::ImageGL(const cl::Context&, cl_mem_flags, gl::GLenum, gl::GLint, gl::GLuint, cl_int*)’:
/home/pradeep/gitroot/forge/examples/opencl/cl.hpp:4241:19: error: cannot convert ‘gl::GLenum’ to ‘cl_GLenum {aka unsigned int}’ for argument ‘3’ to ‘_cl_mem* clCreateFromGLTexture(cl_context, cl_mem_flags, cl_GLenum, cl_GLint, cl_GLuint, cl_int*)’
             &error);

The above type conversion is the issue, i am trying to figure out if we can by pass this problem on our end. It may not be possible i think. Any thoughts ?

pavanky · 2016-05-09T18:22:26Z

Hmm might be a good way to get glbinding into the official repos as well if forge depends on it.

9prady9 · 2016-05-18T09:21:38Z

@arrayfire/core-devel
I don't think we can use cl.hpp header with glbinding, i have raised an issue on their github page asking for work around. One way for us would be to rewrite our OpenCL examples to use OpenCL-C API and convert the glbinding's enum gl::GL* class values to cl_GL* types ourselves.

Any thoughts ?

Added the following utility functions to help with copying data from host side to rendering buffers * fg_update_vertex_buffer(cpp version is updateVertexBuffer) * fg_update_pixel_buffer(cpp version is updatePixelBuffer)

9prady9 · 2016-07-05T18:44:03Z

@pavanky @shehzan10 Review please.

pavanky · 2016-08-15T12:17:03Z

docs/pages/README.md

 * [GLFW](http://www.glfw.org/)
 * [freetype](http://www.freetype.org/)
+* [FreeImage](http://freeimage.sourceforge.net/)


Is FreeImage necessary for the library or is it necessary for the demos ?

It is needed to save frame buffers to disks.

shehzan10 · 2016-08-15T14:26:31Z

I have these 2 commits locally for the same task.

shehzan10@04ded4e
shehzan10@3dd77e9

can you combine them into one and add it to this PR.

PS. Don't miss the file renames.

Earlier, only OpenCL examples had these flags for c++ compiler.

shehzan10 · 2016-08-15T15:14:17Z

examples/CMakeLists.txt

@@ -80,6 +80,10 @@ ELSE(TARGET forge)
    ENDIF(UNIX)
 ENDIF(TARGET forge)

+IF(UNIX)
+    SET(CMAKE_CXX_FLAGS  "${CMAKE_CXX_FLAGS} -Wno-deprecated-declarations -Wno-unused-function")


You missed adding -pthread.

pavanky · 2016-08-15T19:09:43Z

src/backend/opengl/font_impl.cpp

@@ -154,7 +156,7 @@ void font_impl::loadAtlasWithGlyphs(const size_t pFontSize)
        //    FT_Stroker_Done(stroker);
        //    FT_Done_Face(face);
        //    FT_Done_FreeType(library);
-        //    FT_THROW_ERROR("FT_Glyph_Stroke", fg::FG_ERR_FREETYPE_ERROR);
+        //    FT_THROW_ERROR("FT_Glyph_Stroke", forge::FG_ERR_FREETYPE_ERROR);
        //}
        //FT_Stroker_Done(stroker);


Why are these commented out ? Can you remove this if you can.

It is related to outline strokes for characters, I added a FIXME label before the commented out code block. Removed it now.

shehzan10 · 2016-08-16T21:33:03Z

src/backend/opengl/gl_native_handles.cpp

+    const auto display = wglGetCurrentDC();
+#elif defined(OS_LNX)
+    const auto display = glXGetCurrentDisplay();
+#endif


There needs to be a OSX block here. Otherwise, display will not be defined.

Actually, there is no need for OSX block in this case. Default value of 0 is sufficient enough. OSX OpenCL-OpenGL interop doesn't use these functions at all.

The auto -> DisplayHandle change is required in gl_native_handles as const auto diplay = 0; initializes auto as int which errors out reinterpret cast. To avoid this, initialize it as DisplayHandle.

shehzan10 · 2016-08-17T14:39:25Z

All platforms are compiling. Merging this PR.
Will open new issues for failing/buggy examples.

Replace GLEW with glbinding

95e69ac

All examples also use glbinding from now, but OpenCL examples are not functional yet due incompatibility between gl.h header included by opencl headers and glbinding header

shehzan10 reviewed May 9, 2016
View reviewed changes

9prady9 added 2 commits May 18, 2016 14:26

Add glbinding dependency for cmake target, forge

06e1ef3

Fix glbinding shared library postfix on windows platform

eea8099

Corrected the order of GL header inclusion for CUDA examples

3d309a6

9prady9 added the enhancement label Jun 6, 2016

9prady9 added this to the 1.0.0 milestone Jun 6, 2016

9prady9 added the in-progress label Jun 6, 2016

Merge branch 'devel' into glew_to_glbinding

f8fdb68

Added the following utility functions to help with copying data from host side to rendering buffers * fg_update_vertex_buffer(cpp version is updateVertexBuffer) * fg_update_pixel_buffer(cpp version is updatePixelBuffer)

9prady9 changed the title ~~[WIP]: Replace GLEW with glbinding~~ Replace GLEW with glbinding Jul 5, 2016

9prady9 added 2 commits July 5, 2016 13:28

Fix typos in macro based conditionals

4d012a8

Use glbinding static libs instead of shared libs

7c93806

Fix syntax errors in sdl toolkit backend for windowing system

33cff2b

9prady9 mentioned this pull request Jul 25, 2016

GLEWmx support dropped from GLEW 2.0 arrayfire/arrayfire#1515

Closed

9prady9 added 3 commits July 27, 2016 20:36

Fix Virtual Trackball rotations for 3d objects(charts)

493aa67

Update C-API documentation

7313c52

Fix data transformation equations for surface & plot

f07f44f

9prady9 removed the in-progress label Aug 2, 2016

9prady9 mentioned this pull request Aug 15, 2016

It seems memory leaking #101

Closed

Fix memory leak in Shader maintenance code

6aaf3e7

9prady9 force-pushed the glew_to_glbinding branch from cb716de to 6aaf3e7 Compare August 15, 2016 11:35

pavanky reviewed Aug 15, 2016
View reviewed changes

9prady9 and others added 3 commits August 15, 2016 20:11

change glbinding cmake build script to be consistent with others

4ddebdd

Add warning related flags to all examples

e2558b1

Earlier, only OpenCL examples had these flags for c++ compiler.

Move ForgeConfig files into CMakeModules

d07ae62

shehzan10 reviewed Aug 15, 2016
View reviewed changes

Move all internal namespaces into global namespace forge

c26386d

pavanky reviewed Aug 15, 2016
View reviewed changes

9prady9 added 3 commits August 16, 2016 08:24

Merge branch 'devel' into glew_to_glbinding

aa92b8d

fix opencl helper functions

b2da6f0

Remove dead code from font_impl object

4ef9c6d

shehzan10 reviewed Aug 16, 2016
View reviewed changes

9prady9 and others added 4 commits August 17, 2016 16:13

fix osx specific code flow

27a7c3a

Explicitly set the GLFW library variables for glbinding

f6a578f

Add gl.h before cuda_gl_interop.h

b24529d

Compilation fixes on OSX

adb16af

The auto -> DisplayHandle change is required in gl_native_handles as const auto diplay = 0; initializes auto as int which errors out reinterpret cast. To avoid this, initialize it as DisplayHandle.

shehzan10 merged commit e2f75ff into arrayfire:devel Aug 17, 2016

9prady9 deleted the glew_to_glbinding branch September 10, 2016 05:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace GLEW with glbinding #89

Replace GLEW with glbinding #89

9prady9 commented May 9, 2016 •

edited

Loading

shehzan10 commented May 9, 2016

shehzan10 commented May 9, 2016

shehzan10 May 9, 2016

9prady9 May 9, 2016

pavanky commented May 9, 2016

shehzan10 commented May 9, 2016

shehzan10 May 9, 2016

9prady9 May 9, 2016

9prady9 commented May 9, 2016

9prady9 commented May 9, 2016 •

edited

Loading

pavanky commented May 9, 2016

9prady9 commented May 18, 2016

9prady9 commented Jul 5, 2016

pavanky Aug 15, 2016

9prady9 Aug 15, 2016

shehzan10 commented Aug 15, 2016

shehzan10 Aug 15, 2016

pavanky Aug 15, 2016

9prady9 Aug 16, 2016

shehzan10 Aug 16, 2016

9prady9 Aug 17, 2016

shehzan10 commented Aug 17, 2016

Replace GLEW with glbinding #89

Replace GLEW with glbinding #89

Conversation

9prady9 commented May 9, 2016 • edited Loading

shehzan10 commented May 9, 2016

shehzan10 commented May 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pavanky commented May 9, 2016

shehzan10 commented May 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9prady9 commented May 9, 2016

9prady9 commented May 9, 2016 • edited Loading

pavanky commented May 9, 2016

9prady9 commented May 18, 2016

9prady9 commented Jul 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shehzan10 commented Aug 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shehzan10 commented Aug 17, 2016

9prady9 commented May 9, 2016 •

edited

Loading

9prady9 commented May 9, 2016 •

edited

Loading