Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#120 Calling glGetString(GL_EXTENSION) even in Core/FC context

2.0.0
open
Nigel Stewart
API (59)
3
2015-02-19
2010-01-07
No

GLEW is calling glGetString(GL_EXTENSIONS) always for extensions even if the current context is a forward-compatible or core context. This is invalid behavior since glGetString(GL_EXTENSIONS) was deprecated in 3.0 for the Forward-Compatible context.

The best thing to do would be to instead:
1) Determine if glGetStringi is available. If so, always use glGetIntegerv(GL_NUM_EXTENSIONS, xxx) and glGetStringi(GL_EXTENSION, ) to determine if any extension exists.
2) Use glGetString if glGetStringi is not available (this should only be possible in an OpenGL implementation not supporting 3.0+).

I've seen this in 3 files : glew_init_gl.c, visualinfo.c, and mipmap.c.

I tried to make the changes myself, but the Visual Studio projects can't find many of the files after I synced.

Discussion

  • I've made a code snippet below for glew_init_gl.c. The main concern I have is how do I make sure glGetStringi is setup or, if not, set it up properly before the call?

    GLboolean glewGetExtension (const char name)
    {

    const GLubyte
    s;
    GLuint dot, n;
    GLint major, minor, num, iter;
    GLubyte p;
    GLubyte
    end;
    GLuint len = _glewStrLen((const GLubyte*)name);

    / query opengl version /
    s = glGetString(GL_VERSION);
    dot = _glewStrCLen(s, '.');
    if (dot == 0)
    return GLEW_ERROR_NO_GL_VERSION;

    major = s[dot-1]-'0';
    minor = s[dot+1]-'0';

    / If we're 3.0 or greater use glGetStringi instead.
    * since this works with both Core, Compatible, and
    * Forward-Compatible contexts
    /
    if (major > 3 && major < 9)
    {
    glGetIntegerv(GL_NUM_EXTENSIONS, &num);
    if (glGetError() == GL_NO_ERROR && num > 0)
    {
    for (iter = 0; iter < num; iter++)
    {
    p = (GLubyte)glGetStringi(GL_EXTENSION, iter);
    n = _glewStrLen(p);
    if (len == n && _glewStrSame((const GLubyte
    )name, p, n)) return GL_TRUE;
    }
    }
    }

    p = (GLubyte)glGetString(GL_EXTENSIONS);
    if (0 == p) return GL_FALSE;
    end = p + _glewStrLen(p);
    while (p < end)
    {
    n = _glewStrCLen(p, ' ');
    if (len == n && _glewStrSame((const GLubyte
    )name, p, n)) return GL_TRUE;
    p += n+1;
    }
    return GL_FALSE;
    }

     
  • Funto
    Funto
    2010-01-16

    I experienced the same problem.

    glprogrammer >> in your code snippet, I think you should not return "GLEW_ERROR_NO_GL_VERSION", as the function returns a GLboolean, and I think this is a mistake :

    / If we're 3.0 or greater use glGetStringi instead.
    since this works with both Core, Compatible, and
    Forward-Compatible contexts /
    if (major > 3 && major < 9)

    => shouldn't it be just "if(major >= 3)" ?

    I hope this will be fixed soon in GLEW...

     
  • Funto
    Funto
    2010-01-16

    Here is my modified version for glewGetExtension(), which seems to work (glprogrammer >> yours does not even compile, so I think you did not test it...) :

    static GLboolean _glewGetExtensionPriorGL3 (const char name)
    {
    GLubyte
    p;
    GLubyte end;
    GLuint len = _glewStrLen((const GLubyte
    )name);
    p = (GLubyte)glGetString(GL_EXTENSIONS);
    if (0 == p) return GL_FALSE;
    end = p + _glewStrLen(p);
    while (p < end)
    {
    GLuint n = _glewStrCLen(p, ' ');
    if (len == n && _glewStrSame((const GLubyte
    )name, p, n)) return GL_TRUE;
    p += n+1;
    }
    return GL_FALSE;
    }

    static GLboolean _glewGetExtensionGL3 (const char name)
    {
    GLubyte
    s;
    GLint num, iter;
    GLuint len = _glewStrLen((const GLubyte*)name);
    GLuint n;
    glGetIntegerv(GL_NUM_EXTENSIONS, &num);

    for(iter = 0 ; iter < num ; iter++)
    {
    s = (GLubyte)glGetStringi(GL_EXTENSIONS, iter);
    n = _glewStrLen(s);
    if(n == len && _glewStrSame((const GLubyte
    )name, s, len))
    return GL_TRUE;
    }
    return GL_FALSE;
    }

    GLboolean glewGetExtension (const char name)
    {
    const GLubyte
    s;
    GLuint dot;
    GLint major;

    / query opengl version /
    s = glGetString(GL_VERSION);
    dot = _glewStrCLen(s, '.');
    if (dot == 0)
    return GL_FALSE;

    major = s[dot-1]-'0';

    / If we're 3.0 or greater use glGetStringi instead.
    * since this works with both Core, Compatible, and
    * Forward-Compatible contexts
    /
    if (major >= 3)
    return _glewGetExtensionGL3(name);
    else
    return _glewGetExtensionPriorGL3(name);
    }

    Feel free to do what you want with this snippet.

     
  • That seems reasonable.

    Thanks.

    On a side note, I was never able to get the dev studio projects working. It always complained about missing source. Is there a forum on building I can follow?

    Thanks again

     
  • Is this going to be fixed soon? I was kind of hoping to see it in 1.5.3.

    It's a pretty big issue since some vendors throw errors when you use removed functionality in a core context.

     
  • That'd be fine if I wanted to use a different tool, but I've already started using GLEW.

    Is this something you're not planning on supporting properly in GLEW? I see the Core context as a very important feature in the future of OpenGL.

     
  • Nigel Stewart
    Nigel Stewart
    2010-05-03

    I think GLEW can and should support Core context, but I am not in a position currently to spend time implementing that. I'd be happy to help with reviewing patches, etc. The most obvious limitation of gl3w is the lack of extension support.

     
  • dzyashu
    dzyashu
    2010-10-03

    We can just test glGetStringi - it is assigned if we use OGL >= 3.x, so glewGetExtension(...) can be implemented this way:

    GLboolean glewGetExtension (const char name)
    {

    GLubyte
    p;
    GLubyte end;
    GLuint len = _glewStrLen((const GLubyte
    )name);
    GLint i;
    if (glGetStringi)
    {
    GLint num_extensions;
    glGetIntegerv(GL_NUM_EXTENSIONS, &num_extensions);
    for (i = 0; i < num_extensions; ++ i)
    {
    if (_glewStrSame((const GLubyte)name, glGetStringi(GL_EXTENSIONS, i), len)) return GL_TRUE;
    }
    }
    else
    {
    p = (GLubyte
    )glGetString(GL_EXTENSIONS);
    if (0 == p) return GL_FALSE;
    end = p + _glewStrLen(p);
    while (p < end)
    {
    GLuint n = _glewStrCLen(p, ' ');
    if (len == n && _glewStrSame((const GLubyte*)name, p, n)) return GL_TRUE;
    p += n+1;
    }
    }
    return GL_FALSE;
    }

    Certainly, list of extensions should be cached...

     
  • MG_code
    MG_code
    2010-10-05

    Here's my personal fix to the problem: http://pastebin.com/rAMvHEQ2

    It only uses glGetStringi() when glGetString() fails, and is otherwise identical to the original function.

    Aside from being as unobtrusive a change as possible, this method also means we don't query OpenGL version to find out which functions are available. It just goes for whatever works first.

    I find this to be the more parsimonious way of doing things, as it cuts out any complications arising from context and profile modes (foward-compat, core, backward compat...) now and in the foreseeable future.

     
  • Is there any specific reason why this is not yet fixed? IMHO this is a very important feature and I don't understand why neither the solution posted here nor the solution in 2960200 can be integrated in glew.
    If someone could point out what objections exist, I would spend time and effort to improve the possible solutions.
    Thank you!

     
  • Nigel Stewart
    Nigel Stewart
    2011-02-12

    No specific objection. There needs to be an effort to put a git patch together and lead the discussion on the mail list. I'd like to see this in the next release of GLEW but mostly I'm minding the store and only doing what's needed for other projects.

     
  • Maybe this is helpful then. It's a patch against the current git that implements a slightly modified version of the solution dzyashu proposed (it additionally checks whether glGetStringi returns zero and compares the string lengths first).

    From 84be5474040ae92fe145b13e34ae9f2e5f5af015 Mon Sep 17 00:00:00 2001
    From: Daniel Kirchner mokaga@lavabit.com
    Date: Sun, 13 Feb 2011 21:16:31 +0100
    Subject: [PATCH] Make glewGetExtension use glGetStringi if available.
    Ensures core profile compatibility.


    auto/src/glew_init_gl.c | 33 ++++++++++++++++++++++++++-------
    1 files changed, 26 insertions(+), 7 deletions(-)

    diff --git a/auto/src/glew_init_gl.c b/auto/src/glew_init_gl.c
    index 33bd85e..5033d63 100644
    --- a/auto/src/glew_init_gl.c
    +++ b/auto/src/glew_init_gl.c
    @@ -5,20 +5,39 @@
    * is not sufficient because extension names can be prefixes of
    * other extension names. Could use strtok() but the constant
    * string returned by glGetString might be in read-only memory.
    + * Use glGetStringi if available.
    /
    GLboolean glewGetExtension (const char
    name)
    {
    GLubyte p;
    GLubyte
    end;
    GLuint len = _glewStrLen((const GLubyte)name);
    - p = (GLubyte
    )glGetString(GL_EXTENSIONS);
    - if (0 == p) return GL_FALSE;
    - end = p + _glewStrLen(p);
    - while (p < end)
    +
    + if (glGetStringi)
    + {
    + GLint i, num_extensions;
    + glGetIntegerv (GL_NUM_EXTENSIONS, &num_extensions);
    + for (i = 0; i < num_extensions; i++)
    + {
    + p = (GLubyte) glGetStringi (GL_EXTENSIONS, i);
    + if (0 == p) return GL_FALSE;
    + if (len == _glewStrLen (p))
    + {
    + if (_glewStrSame ((const GLubyte
    )name, p, len)) return GL_TRUE;
    + }
    + }
    + }
    + else
    {
    - GLuint n = _glewStrCLen(p, ' ');
    - if (len == n && _glewStrSame((const GLubyte)name, p, n)) return GL_TRUE;
    - p += n+1;
    + p = (GLubyte
    )glGetString(GL_EXTENSIONS);
    + if (0 == p) return GL_FALSE;
    + end = p + _glewStrLen(p);
    + while (p < end)
    + {
    + GLuint n = _glewStrCLen(p, ' ');
    + if (len == n && _glewStrSame((const GLubyte*)name, p, n)) return GL_TRUE;
    + p += n+1;
    + }
    }
    return GL_FALSE;
    }
    --
    1.7.4

     
  • Hmm, having looked through glew.c / glew.h, there are 2 functions where glGetString(GL_EXTENSIONS) is used:

    glewGetExtension()
    glewContextInit()

    Each function obtains the pre-GL3.0 composite extensions string using glGetString(GL_EXTENSIONS), then calls _glewSearchExtension() to determine if the big string contains the name being checked, specifying a string start byte and end byte in stack memory..

    I am not sure quite why glewContextInit() doesnt just call glewGetExtension(). The only difference I can see is that is that glewGetExtension() calls glGetString(GL_EXTENSIONS) once and _glewSearchExtension() once, whereas glewContextInit() calls glGetString(GL_EXTENSIONS) once and _glewSearchExtension() many times.

    I guess this is slightly more efficient, at the cost of using 2 functions instead of one, but is not really much of a saving, plus it doesn't work as a model for post-GL3.0 forward-compatibility.

    I have modified my version of glew as follows.

    1/ glewGetExtension becomes just a wrapper around _glewSearchExtension, the start and end point are ignored.

    GLboolean glewGetExtension (const char name)
    {
    const GLubyte
    start;
    const GLubyte* end;
    return _glewSearchExtension(name, start, end);
    }

    2/ a section of code in glewContextInit() is commented out

    // / * query opengl extensions string * /
    // extStart = glGetString(GL_EXTENSIONS);
    // if (extStart == 0)
    // extStart = (const GLubyte*)"";
    // extEnd = extStart + _glewStrLen(extStart);

    N.B. You can actually just ignore this code and leave it in, as it does nothing important in the new scheme.

    3/ Most importantly, _glewSearchExtension is changed to do all the work, for both forward-compatible and backward-compatible profiles.

    /
    * Backward-compatible profile:
    * Search for name in the extensions string. Use of strstr()
    * is not sufficient because extension names can be prefixes of
    * other extension names. Could use strtok() but the constant
    * string returned by glGetString might be in read-only memory.

    * Forward-compatible profile:
    * Just search through the list of strings for an exact match.
    /
    static GLboolean _glewSearchExtension (const char
    name, const GLubyte start, const GLubyte end)
    {
    const GLubyte p ;
    GLuint name_len = _glewStrLen((const GLubyte
    )name);
    GLint num_extensions = 0 ;
    /
    * For the compatibility profile, glGetIntegerv( GL_NUM_EXTENSIONS )
    * should be 0, and glGetString( GL_EXTENSIONS ) should return
    * a string.


    * For core profile, glGetIntegerv( GL_NUM_EXTENSIONS ) should
    * be >= zero, and glGetString( GL_EXTENSIONS ) should not return
    * a string.
    /
    glGetIntegerv (GL_NUM_EXTENSIONS, &num_extensions) ;
    p = (const GLubyte
    )glGetString(GL_EXTENSIONS);

    if ( 0 == num_extensions && p != 0 )
    {
    // For the pre GL3.x and compatibility profile, a single long string contains
    // the extensions. if this string is available, get it and search for the named
    // extension.
    while ( p < ( p + _glewStrLen( p ) ) )
    {
    GLuint n = _glewStrCLen(p, ' ');
    if ( name_len == n && _glewStrSame((const GLubyte)name, p, name_len ))
    {
    return GL_TRUE; // Extension found.
    }
    p += n+1;
    }
    }
    else
    {
    // If the standard extension string is empty, try the GL3.x+ core profile
    // mechanism that stores the extension names as a string list. Check
    // each list name to see if it matches the requested name.
    GLint i ;
    for ( i = 0 ; i < num_extensions ; i++ )
    {
    p = (GLubyte
    ) glGetStringi (GL_EXTENSIONS, i);
    if (p != 0)
    {
    GLuint n = _glewStrLen( p );
    if ( name_len == n && _glewStrSame((const GLubyte*)name, p, name_len ))
    {
    return GL_TRUE; // Extension found
    }
    }
    }
    }
    return GL_FALSE ; // No extension found.
    }

    The above scheme is the minimal set of changes to glew.c that fixes the issue, but it leaves the code messy. A better solution would be to put all the code from _glewSearchExtension in step 3/ above into glewGetExtension(), remove _glewSearchExtension() completely, change all calls to _glewSearchExtension() to glewGetExtension(), and delete the code section in step 2/ above.

    This suggested solution is in keeping with the glew principles of not relying on the standard C lib, and relative simplicity. It would require testing to determine if it causes any speed issues that might require a cache rather than calling glGetStringi multiple times in the forward-compatible profile. On my machine, it makes no obvious difference.

    Another thing to note is that determining whether the context if forward-compatible or backward-compatible is not necessarily straightforward. In theory, if either of glGetIntegerv(GL_NUM_EXTENSIONS) or glGetStringi(GL_EXTENSIONS , i ) result in an OpenGL error, then the context is backward-compatible, not forward compatible. Likewise, if a call to glGetString(GL_EXTENSIONS) results in an OpenGL error, then the context is forward-compatible, not backward-compatible.

    This assumes correct interpretation and implementation by driver writers, whilst there is some room for ambiguity in when these calls are valid. I found that the version of the AMD Catalyst driver I am using responded to glGetString(GL_EXTENSIONS) regardless of which context type I created, whilst glGetIntegerv(GL_NUM_EXTENSIONS) returned 169 in a forward-compatible context and 0 in a backward-compatible context. The code I suggest here may not catch all possible interpretations.

    TTFN,
    Roger.

     
  • I looked at it to get rid of GL_INVALID_ENUM, and it seems you only need to replace two functions. I have included the copy you can just copy in based off of http://www.opengl.org/wiki_132/index.php?title=GlGetString&oldid=3780#glGetStringi.28GL_EXTENSIONS.2C_i.29 and AMD's Introduction to OpenGL 3.0:

    static GLboolean _glewSearchExtension (const char name,
    const GLubyte
    start, const GLubyte *end) {
    int num_extensions, i;
    glGetIntegerv(GL_NUM_EXTENSIONS, &num_extensions);

    for(i = 0; i < num_extensions; ++i) {
        if(0 == strcmp(name, (const char *)glGetStringi(GL_EXTENSIONS, i)))
            return GL_TRUE;
    }
    
    return GL_FALSE;
    

    }

    GLboolean glewGetExtension (const char* name) {
    int num_extensions, i;
    glGetIntegerv(GL_NUM_EXTENSIONS, &num_extensions);

    for(i = 0; i < num_extensions; ++i) {
        if(0 == strcmp(name, (const char *)glGetStringi(GL_EXTENSIONS, i)))
            return GL_TRUE;
    }
    
    return GL_FALSE;
    

    }

     

  • Anonymous
    2014-07-23

    Just going to say that this is really, really overdue for getting fixed.

     


Anonymous


Cancel   Add attachments