Bug 143964 - OpenGLShims appears to have a dead store if GLES2
Summary: OpenGLShims appears to have a dead store if GLES2
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-20 14:08 PDT by Joseph Pecoraro
Modified: 2024-01-17 01:10 PST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-04-20 14:08:28 PDT
* SUMMARY
Bug 95556 / r127874 introduced some GLES2 handling. It looks like it would cause a dead store. The code should be cleaned up in some way, or there is an actual bug.

Currently the code looks like:
https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/OpenGLShims.cpp#L64

    static void* lookupOpenGLFunctionAddress(const char* functionName, bool* success = 0)
    {
        ...
        fullFunctionName = functionName;
        fullFunctionName.append("EXT");
        target = getProcAddress(fullFunctionName.utf8().data());

    #if defined(GL_ES_VERSION_2_0)
        fullFunctionName = functionName;
        fullFunctionName.append("ANGLE");
        target = getProcAddress(fullFunctionName.utf8().data());
        if (target)
            return target;

        fullFunctionName = functionName;
        fullFunctionName.append("APPLE");
        target = getProcAddress(fullFunctionName.utf8().data());
    #endif

        // A null address is still a failure case.
        if (!target && success)
            *success = false;

        return target;
    }

Note that the target assignment for "EXT" will just immediately get overwritten by the "ANGLE"/"APPLE" case if GL_ES_VERSION_2_0.

So either there is a bug and the "EXT" case should be getting used, or there is not a bug and this is unnecessary work and the "EXT" case should just be an #else.
Comment 1 Joseph Pecoraro 2015-04-20 14:11:05 PDT
My guess is there should be an:

    if (target)
        return target;

But I don't know how to test.