Did some basic review. More comes later once the patch is applyable on current master. Seems it needs an update.

Forgot to mention, SCons needs an update as well.

Also addressed in the previous revision are the removal of some dead code and changing the glew context functions to be prefixed with "mx" instead of "glew"

Tested on Linux, needed to make some edits to build with strict warnings.

Commented on minor issues.

Tried to use this on X11, Opens and closes single window fine but asserts when closing a second window.

GHOST_ContextGLX.cpp:92: virtual GHOST_ContextGLX::~GHOST_ContextGLX(): Assertion `s_sharedCount > 0' failed

Also, think it may work best to finalize this patch in its own branch (so fixes can be made there more easily then a patch - which would mean we have to commandeer each time)

A couple of responses, but the rest I'll just fix.

Should I upload another revision or make a new branch or what?

One thing I forgot to mention is that there is no sdl context. I don't actually think that will be very complex though.

Had to make a few changes to make it build on ubuntu, see P92

WITH_SYSTEM_GLEW is not very well supported. If we are to use our own version we should always disable this option.

Similarly to Campbell I'm getting trash content with two windows and a crash when attempting to close one of them (assertion failing).

Still more to review, but doing a preliminary flush.

@cambellbarton The sdl context looks exactly like I imagined it. I've got a bit on insomnia, so I'm glad I didn't start working on it myself.

I think you are right about the mx pointers for sdl. SDL will take care of that itself and only the glwe mx context is needed.

There are some things I think might need fixing here, but it might be easier to just make the changes than explain.

@Antonis Ryakiotakis (psy-fi) - Looks like your concerns have been addressed, could you check this works now?

As far as I can see this is now very close to being ready for master,

I tried to use OpenGL ESv2 and it gave many linking errors with glColor4f GL_POLYGON ... etc, Assume this is intended/known, until we support OpenGL ES properly.

One minor style issue,

I noticed C function being called like this:

::glXQueryVersion

I'd rather be consistent here, we could add :: prefix all over where C calls are made, but is this really needed? (C API's for windowing typically have verbose ID's, not sure name-space collision is worth worrying about)

Pushed some minor changes to the SDL context.

The ES profile will not compile properly until the vertex streaming code is in.

I noticed you added a _private.h. I have several files like this but I named them _intern.h, however it isn't really a big deal if I rename them.

As for :: on globals, this is just my habit from doing a lot of template programming in my research and I like how it sets apart the system calls from other functions. You are right is isn't really needed in this case.

regarding _intern.h vs _private.h, I don't have a strong opinion, it just seemed a bit odd to have intern/foo_intern.h - but if you prefer, simple to change.

Note on ::glXQueryVersion for global C calls, checked and in fact this is already used a fair bit for ghost (although mainly for Win32),

Since some OS system calls aren't named very clearly, maybe fine to have convention of using :: for C functions.

Tried building on MSVC2013, Windows7, And I get this error:

C:\b\blender\intern\ghost\intern\GHOST_ContextWGL.cpp(290): error C2065: 'GWL_HWNDPARENT' : undeclared identifier
C:\b\blender\intern\ghost\intern\GHOST_ContextWGL.cpp(296): error C2065: 'GWL_HINSTANCE' : undeclared identifier

I did not succeed to build branch under ubuntu 14.04 64 bits ( i7 4770k _ Nividia Titan Black) Nvidia Driver 340.24.

Here is compilation failure log :
http://www.pasteall.org/53095

In D643#40, @ronan ducluzeau (zeauro) wrote:

I did not succeed to build branch under ubuntu 14.04 64 bits ( i7 4770k _ Nividia Titan Black) Nvidia Driver 340.24.

Here is compilation failure log :
http://www.pasteall.org/53095

It looks like you have a version of glew that doesn't support the GLEW_MX option. You can either disable WITH_GLEW_MX or disable WITH_SYSTEM_GLEW or upgrade your development libraries.

I can't compile with the system GLEW on Debian Wheezy: http://www.pasteall.org/53121 My guess is that somewhere the file gets included twice? Not sure glewContextIsSupported is not found tho. But we really need to support building with system GLEW.

Will check on the updated code later.

Also is the revision up-to-date?

Getting linking error. after clean build.

rB07c3d981962ef17950f78961d7a504a99014d80b

../../lib/libbf_windowmanager.a(wm_draw.c.o): In function `wm_triple_gen_textures':
/src/blender/source/blender/windowmanager/intern/wm_draw.c:438: undefined reference to `_mx_context'
/src/blender/source/blender/windowmanager/intern/wm_draw.c:438: undefined reference to `_mx_context'
/src/blender/source/blender/windowmanager/intern/wm_draw.c:438: undefined reference to `_mx_context'
../../lib/libbf_editor_space_view3d.a(drawvolume.c.o): In function `draw_smoke_volume':
/src/blender/source/blender/editors/space_view3d/drawvolume.c:359: undefined reference to `_mx_context'
/src/blender/source/blender/editors/space_view3d/drawvolume.c:451: undefined reference to `_mx_context'
../../lib/libbf_editor_space_view3d.a(drawvolume.c.o):/src/blender/source/blender/editors/space_view3d/drawvolume.c:471: more undefined references to `_mx_context' follow
../../lib/libbf_intern_ghost.a(GHOST_ContextGLX.cpp.o): In function `GHOST_ContextGLX::initContextGLXEW()':
/src/blender/intern/ghost/intern/GHOST_ContextGLX.cpp:143: undefined reference to `glew_chk'
../../lib/libbf_intern_ghost.a(GHOST_ContextGLX.cpp.o): In function `GHOST_Context::~GHOST_Context()':
/src/blender/intern/ghost/intern/GHOST_Context.h:61: undefined reference to `mxDestroyContext'
../../lib/libbf_intern_ghost.a(GHOST_ContextGLX.cpp.o): In function `GHOST_Context::activateGLEW() const':
/src/blender/intern/ghost/intern/GHOST_Context.h:132: undefined reference to `mxMakeCurrentContext'
../../lib/libbf_intern_ghost.a(GHOST_Context.cpp.o): In function `GHOST_Context::initContextGLEW()':
/src/blender/intern/ghost/intern/GHOST_Context.cpp:142: undefined reference to `mxDestroyContext'
/src/blender/intern/ghost/intern/GHOST_Context.cpp:144: undefined reference to `mxCreateContext'
/src/blender/intern/ghost/intern/GHOST_Context.cpp:144: undefined reference to `mxMakeCurrentContext'
/src/blender/intern/ghost/intern/GHOST_Context.cpp:146: undefined reference to `mxGetCurrentContext'
collect2: error: ld returned 1 exit status
source/creator/CMakeFiles/blender.dir/build.make:165: recipe for target 'bin/blender' failed
make[2]: * [bin/blender] Error 1
CMakeFiles/Makefile2:4237: recipe for target 'source/creator/CMakeFiles/blender.dir/all' failed
make[1]: * [source/creator/CMakeFiles/blender.dir/all] Error 2
Makefile:147: recipe for target 'all' failed

Building works correct now on FreeBSD & Linux

Checked whole patch again at last. Looks good apart from a little thing that can be handled later anyway.
An issue is that we should probably use a glew that supports multiple contexts for this to work - btw @Jason Wilkins (jwilkins) which version is that based on?
If that's the case we should really disable system glew, until distributions catch up

Also committed some edits in branch. I think it can be applied after bcon1 so we have plenty of time to iron out any issues.

Isn't it committed now?