REGRESSION(r181571): Build broken when USE(OPENGL_ES_2) is defined.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Carlos Alberto Lopez Perez]

At least on platform GTK, r181571 <http://trac.webkit.org/r181571> broke the build when defining -DENABLE_GLES2=ON.

$CC [...] -o Source/WebCore/CMakeFiles/WebCore.dir/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp.o -c ../../Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp
../../Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:1365:5: error: use of undeclared identifier 'glGenVertexArrays'
    glGenVertexArrays(1, &array);
    ^
../../Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:1379:5: error: use of undeclared identifier 'glDeleteVertexArrays'
    glDeleteVertexArrays(1, &array);
    ^
../../Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:1392:12: error: use of undeclared identifier 'glIsVertexArray'; did you mean 'isVertexArray'?
    return glIsVertexArray(array);
           ^~~~~~~~~~~~~~~
           isVertexArray
../../Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:1385:32: note: 'isVertexArray' declared here
GC3Dboolean GraphicsContext3D::isVertexArray(Platform3DObject array)
                               ^
../../Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:1403:5: error: use of undeclared identifier 'glBindVertexArray'; did you mean 'bindVertexArray'?
    glBindVertexArray(array);
    ^~~~~~~~~~~~~~~~~
    bindVertexArray
../../Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:1399:25: note: 'bindVertexArray' declared here
void GraphicsContext3D::bindVertexArray(Platform3DObject array)
                        ^
4 errors generated.
ninja: build stopped: subcommand failed.


The issue seems to be that Vertex Array Objects (glGenVertexArrays, etc) are in GLESv3 but not in GLESv2.

[Carlos Alberto Lopez Perez]

Created attachment 253382 [details]
Patch

[WebKit Commit Bot]

Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE

[Carlos Alberto Lopez Perez]

Comment on attachment 253382 [details]
Patch

All EWS are happy, so asking for review now.

[Alex Christensen]

Comment on attachment 253382 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=253382&action=review

> Source/ThirdParty/ANGLE/include/GLES2/gl2softlinking.h:-164
> -SOFT_LINK(libGLESv2, glBindVertexArray, void, GL_APIENTRY, (GLuint array), (array));
> -SOFT_LINK(libGLESv2, glDeleteVertexArrays, void, GL_APIENTRY, (GLsizei n, const GLuint* arrays), (n, arrays));
> -SOFT_LINK(libGLESv2, glGenVertexArrays, void, GL_APIENTRY, (GLsizei n, const GLuint* arrays), (n, arrays));
> -SOFT_LINK(libGLESv2, glIsVertexArray, GLboolean, GL_APIENTRY, (GLuint array), (array));

Great!  This is the proper response to my complaint in https://bugs.webkit.org/show_bug.cgi?id=126944

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:1364
> +#if !USE(OPENGL_ES_2) && (PLATFORM(GTK) || PLATFORM(EFL) || PLATFORM(WIN) || PLATFORM(IOS))

If we're not using OpenGLESv2, then we don't want WebGL2 functionality to be available at all.  This is a good compile fix, but I think we need to make it a step further and make this whole function not available instead of just returning 0.

[Carlos Alberto Lopez Perez]

(In reply to comment #4)
> 
> > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:1364
> > +#if !USE(OPENGL_ES_2) && (PLATFORM(GTK) || PLATFORM(EFL) || PLATFORM(WIN) || PLATFORM(IOS))
> 
> If we're not using OpenGLESv2, then we don't want WebGL2 functionality to be
> available at all.  This is a good compile fix, but I think we need to make
> it a step further and make this whole function not available instead of just
> returning 0.

My understanding is that WebGL2 should not be enabled/compiled when using OpenGLESv2, because OpenGLESv2 lacks some features that are needed for WebGL2 (like Vertex Array Objects).

Maybe bug 141178 is a good candidate to fix this in a proper way?.

In the meantime I would like to land at least this build fix.

BTW, Thanks for the review ;)

[WebKit Commit Bot]

Comment on attachment 253382 [details]
Patch

Clearing flags on attachment: 253382

Committed r184605: <http://trac.webkit.org/changeset/184605>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.