[PATCH] Incorrect aliasing of core/extension GLhandleARB shader functions

[Philip Taylor]

In glext_funcs.h we declare function pointers like

GLhandleARB (*pglCreateShaderObjectARB)(GLenum);

That is initialised in ogl.cpp with GetProcAddress to point to functions that GL defines with the signatures

GLhandleARB glCreateShaderObjectARB(GLenum);

(if the GL version is old but the relevant extension is supported) or

GLuint glCreateShader(GLenum);

(if the GL version is >=2.0 so we should use the core version instead of the extension).

That's fine on Windows/Linux because GLhandleARB is equivalent to GLuint, so the type of pglCreateShaderObjectARB is compatible with glCreateShader. But OS X defines GLhandleARB as void*, and on 64-bit builds that's a larger type than GLuint.

I don't think that'll cause problems in practice (the calling conventions mean glCreateShader will return a 32-bit uint in a register, which we store in a 64-bit void* and then we pass it in a register to some other shader function that expects a 32-bit uint, so there's no data loss). But it's pretty ugly and fragile.

It should be possible to implement wrapper functions that know whether they're calling glCreateShader or glCreateShaderObjectARB, and explicitly cast the return value to intptr_t (which will be safe), and same for other shader functions. But I think we should just drop extension support for GLSL in GL versions <2.0, and only support GLSL via core functions - it's not worth the effort of adding wrappers when it'll help about zero users. So we should change glext_funcs.h to remove the ARB versions of the GL 2.0 shader functions, and change the code that detects shader support to test the GL version instead of testing the extensions string.

[Emils Solmanis]

Don't have anything pre-2.0 to test on, the lowest thing I have is a HD3k on my laptop and it's 2.1. Anyway, does this look like what you asked for?

[Kieran P]

Assigning this to myconid to take a look at for the next release, seeing as he'll be deep in this stuff already :-)

[historic_bruno]

Bumping to A14 since it's not a release blocker.

[Jorma Rebane]

There is no need for this patch due to massive changes in megapatch. Since this ticket is older than a year, there simply isn't a need for it anymore.