[!] Wrong code convention used for some GL-functions in CCGLProgram.h

cocos2d-x C++
#1

Hi.
I have noticed that if one of my shaders can’t be compiled my application just crashes without any detailed messages. My investigation showed that the crash takes place in CCGLProgram::logForOpenGLObject during the invocation of infoFunc. Further investigation discovered that on the Windows platform (which I use for development and testing) glGetShaderiv uses __stdcall call convention, while GLInfoFunction uses default __cdecl. Moreover, it seems that with Windows’ version of glew there is no need to get the pointer to those GL-functions, thus instead of

return this->logForOpenGLObject(m_uVertShader, (GLInfoFunction)&glGetShaderiv, (GLLogFunction)&glGetShaderInfoLog);

it should be

return this->logForOpenGLObject(m_uVertShader, (GLInfoFunction)glGetShaderiv, (GLLogFunction)glGetShaderInfoLog);

I’d wish to make the fix by myself, but I have no Mac and iOS for testing the fix for cross-platform compatibility.

#2

Hi dot squid,
That makes sense.
Could you upload the shader?
I will be glad to help you test it on MAC and iOS after applying your patch.

#3

There is no need to have a special shader. Just make some stupid syntax errors in any built-in shader, thus it fails to compile. If cocos2d-x shows an error report in the console and terminates the app correctly, then everything goes fine on the platfrom you’re testing.

I attached the patch to this message, but it contains the ‘fix’ only for Windows platfrom. But I expect it should be enough and other platforms do not require any changes.

Thanks.

#4

Thank you. Issue #2846 was created.
BTW, redmine issue system was opened for everyone. If you find a bug, you could create a new issue there.
http://www.cocos2d-x.org/projects/native/issues/new

#5

correct.
If the shader can’t be compiled or linked, the game should abort(), but it must report where the error is.

We did something similar for cocos2d-iphone:
https://github.com/cocos2d/cocos2d-iphone/blob/develop-v2/cocos2d/CCGLProgram.m#L172

#6

And I think we should only check for the error on DEBUG mode.
It is expensive (CPU-wise) to do it all the time.

Some tips from Apple:
https://developer.apple.com/library/ios/documentation/3DDrawing/Conceptual/OpenGLES_ProgrammingGuide/BestPracticesforShaders/BestPracticesforShaders.html#//apple_ref/doc/uid/TP40008793-CH7-SW3

#7

OMG!
Is it so hard to fix this issue? If you did not notice, it’s still present and it migrated to v3.0. The app still crashes / throws an exception if it can’t load the shader. And in fact, the code that causes this issue is for debugging. That’s ridiculous.
You have the patch with the fix. It requires to change only a couple of lines of code to implement it in v3.0.

1 Like
#8

@dotsquid this is fixed in Cocos2d-x 3.6. If someone uses older cocos2d-x and on shader compile cocos crashes (when the shader has a syntax error) here is the fix. In CCGLProgram.cpp change the function from:

std::string GLProgram::logForOpenGLObject(GLuint object, GLInfoFunction infoFunc, GLLogFunction logFunc) const
{
    std::string ret;
    GLint logLength = 0, charsWritten = 0;

    infoFunc(object, GL_INFO_LOG_LENGTH, &logLength);
    if (logLength < 1)
        return "";

    char *logBytes = (char*)malloc(logLength);
    logFunc(object, logLength, &charsWritten, logBytes);

    ret = logBytes;

    free(logBytes);
    return ret;
}

to

std::string GLProgram::logForOpenGLObject(GLuint object, GLInfoFunction infoFunc, GLLogFunction logFunc) const
{
    std::string ret;
    GLint logLength = 0, charsWritten = 0;

    glGetShaderiv(object, GL_INFO_LOG_LENGTH, &logLength);
    if (logLength < 1)
        return "";

    char *logBytes = (char*)malloc(logLength);
    glGetShaderInfoLog(object, logLength, &charsWritten, logBytes);

    ret = logBytes;

    free(logBytes);
    return ret;
}

See the code here: https://github.com/cocos2d/cocos2d-x/blob/cocos2d-x-3.6/cocos/renderer/CCGLProgram.cpp