Fix extension detection on core contexts.

[turol]

This fixes a longstanding issue where GLEW is completely broken on core contexts because glGetString(GL_EXTENSIONS) is no longer valid. When this happens build the string manually.

Tested on 64-bit Linux.

Quick test program for SDL2: http://pastebin.com/KP13Haby

[nigels-com]

Some initial questions.

1. Can we avoid the memory leak?
2. Has this been tested in MX (multi-context) mode?
3. Has this been built and tested on Windows, Linux, Mac?

• Nigel

[turol]

1. No. There's no glewShutdown method where we could free the string.
2. No.
3. No, Linux only so far.

[nigels-com]

I do think the leak is a deal-breaker. Too bad there is no upper-bound on the extension string at compile-time. :-)

• Nigel

[nigels-com]

So as a bit of a suggestion...

Can we make a sorted list of strings which can be used for O(log n) extension checking, then cleaned up at the end of the initialization phase?

• Nigel

[turol]

Yes but it's going to be very invasive. Might require essentially rewriting the entire init code, including maybe changing the autogenerated bits. Will you do that or should I look into what it will take?

[turol]

Second try. No longer leaks memory. Not tested with MX. Not tested on anything else than Linux.

Trying to use a list would have gotten very complex since both glewGetExtension and glewContextInit call _glewSearchExtension whose interface would have required changing. Recreating the datastructure for every glewGetExtension would have been wasteful and there's no API-compatible way to store the one created in glewContextInit without memory leaks.

[turol]

What's the status on this?

[nigels-com]

No update on this. I've pretty much decided to release GLEW 1.12.0 as-is, perhaps this weekend. That clears the deck for taking on one of these bigger changes without holding things up in terms of release cycle.

[urkle]

https://github.com/urkle/glew/compare/core-gl-context <-- here is my version of this patch based on this original concept.. it's more complete and fully tested as functioning on windows, mac and linux in 2 high-profile game ports that I'm working on. (to be released in the next few months).

Also, in this current PR alloca should not be used.. if you read the man page for alloca it states

alloca() is machine and compiler dependent; its use is discouraged.
alloca() is slightly unsafe because it cannot ensure that the pointer returned points to a valid and usable block of memory.  The allocation made may exceed the bounds of the stack, or even go further into other objects in memory, and alloca() cannot determine such an error.  Avoid alloca() with large unbounded allocations.

So it's best to avoid it's use. My approach is cleaner and does not use alloca, and it works with "MX" mode as well as the hashes are able to be stored per-context. It also ensures that ALL of the base core functions are loaded in a 3.2+ core context without the need for glewExperimental=GL_TRUE.

[turol]

Edward's solution is more complete than mine however it's much more intrusive. I was aiming for minimal changes.

As for alloca, I used it to avoid a memory leak. Your solution still leaks memory though the leak is of bounded size (2 blocks of 16 bytes according to Valgrind, still reachable from global variable so not in standard report). In my first version I'd forgotten to free it on subsequent glewInits.

There's really no way to fix this without adding a glewShutdown call.

[urkle]

Unfortunately I had to do a more intrusive approach as that was the only way to get things working properly without resorting to using glewExperimental. This is all due to a core profile only returning extension identifiers after OpenGL 3.2.

And yes there is that small leak of the hash memory buffer because there isn't any glewShutdown right now.. But general use of the library doesn't entail double-initting. and if you are using multiple contexts (MX) then things are handled correctly as each is isolated into it's own non-global context handler.

Shall I create a proper PR for my branch at this point?

[turol]

Fine by me though it's up to Nigel which one he wants. If your patches are merged I'll consider these obsolete.

[nigels-com]

I appreciate the extra input on this. I was moving house over the weekend, but I ought to internet and some spare moments to allocate to this soon. Can the two blocks of leaked memory be made static for non-mx mode?

[nigels-com]

Is glGetString(GL_EXTENSIONS) even legal on a core context? Should we prefer glGetIntegerv(GL_NUM_EXTENSIONS), and fall back to the compatible method?

[turol]

glGetString(GL_EXTENSIONS) will generate an error and return NULL on a core context. The code will then try to use GL_NUM_EXTENSIONS. While this causes an error on core context I did it this way because I can be sure it will not break compatibility contexts. I think (but haven't checked) trying to get GL_NUM_EXTENSIONS on a pre-3.0 context will generate an error and so would change previously-working code to working-but-raises-an-error.

There are better ways to handle this. We could first check if we have >3.0 context but this was the least invasive way I could do it and would have required more testing than I was willing to do at that point.

[nigels-com]

Ah, I see. Thanks.

[urkle]

My branch checks to see if we have a 3.0+ context and always uses the new extension fetch mechanism.

[nigels-com]

Yeah, I'm still leaning towards a per-extension boolean flag, since after all we know the supported GLEW extensions when we're generating the source.

[urkle]

@nigels-com what do you mean by "per extension boolean flag"?

[nigels-com]

Some progress happening over on the other pull request.
#30

[nigels-com]

turol, I used some of this merge-commit over on my core-context branch to get things working with glGetStringi. Would you mind testing that branch to see if satisfies your requirements. So far I'm mostly testing that glewinfo output makes superficial sense.

[turol]

Yeah it works. It correctly detects both EXT and ARB direct_state_access when using core context where stock GLEW 1.12 doesn't.

[turol]

Also vc6 and vc10 project files have incorrect line endings. This causes trouble when changing branches on Linux. First git claims they're modified and refuses to overwrite them but checkout thinks they're ok and doesn't overwrite them. Please fix them.

[nigels-com]

Yeah, I think I hit that too. Try this

git add -A

http://stackoverflow.com/questions/11383094/unstaged-changes-left-after-git-reset-hard

[mean-ui-thread]

This header might not exist in Visual Studio. I'm on Linux so I cannot verify.

[nigels-com]

I've been setting up a Win7 laptop for development, so I'll be able to check Windows, Linux and Mac.

[nigels-com]

Time to close this pull request. There is some core support coming in the upcoming GLEW 2.0 release. Can't claim it's perfect, but it's a good step forward. Thanks to all that engaged with this issue.