From 89301eaf214f0da74c5266e7a548899d5f491b50 Mon Sep 17 00:00:00 2001 From: Alistair Strachan Date: Fri, 22 May 2015 14:10:09 -0700 Subject: [PATCH] Fix EGL shim extension injection for GL ES 3 drivers. The Android EGL shim injects GL_EXT_debug_marker into the ES driver EXTENSIONS string for the OpenGL ES 1.x and 2.0/3.0/3.1 drivers if the extension is not already provided. This feature is used by GLES_trace. In Open GL ES 3.0 it became possible to query an indexed version of the EXTENSIONS string via GetStringi(). NUM_EXTENSIONS Gets were also added to the specification (taken from Open GL). If the shim does not have to inject the extension, then there is no problem, as glGetString() and glGetStringi() / NUM_EXTENSIONS will be consistent. However, if the Android EGL shim injects the extension, NUM_EXTENSIONS and GetStringi() will report one less extension than is really available. Consistency between these methods is tested by the dEQP framework with the dEQP-GLES3.functional.state_query.string.extensions test. If the driver does not provide GL_EXT_debug_marker, this test fails. This change wraps all of the affected entry points so that the wrapped driver extensions are never visible directly to dEQP, eliminating the inconsistency. --- opengl/libs/EGL/egl.cpp | 38 +++++++++++++++++ opengl/libs/EGL/egl_object.cpp | 12 ++++++ opengl/libs/EGL/egl_object.h | 2 + opengl/libs/GLES2/gl2.cpp | 78 ++++++++++++++++++++++++++++++++-- opengl/libs/GLES2/gl2_api.in | 10 ++--- opengl/libs/egl_impl.h | 3 ++ opengl/tools/glgen2/glgen.py | 28 +++++++++--- 7 files changed, 155 insertions(+), 16 deletions(-) diff --git a/opengl/libs/EGL/egl.cpp b/opengl/libs/EGL/egl.cpp index 7c70fa03a..4e0e5bc22 100644 --- a/opengl/libs/EGL/egl.cpp +++ b/opengl/libs/EGL/egl.cpp @@ -292,6 +292,44 @@ const GLubyte * egl_get_string_for_current_context(GLenum name) { return (const GLubyte *)c->gl_extensions.string(); } +const GLubyte * egl_get_string_for_current_context(GLenum name, GLuint index) { + // NOTE: returning NULL here will fall-back to the default + // implementation. + + EGLContext context = egl_tls_t::getContext(); + if (context == EGL_NO_CONTEXT) + return NULL; + + egl_context_t const * const c = get_context(context); + if (c == NULL) // this should never happen, by construction + return NULL; + + if (name != GL_EXTENSIONS) + return NULL; + + // if index is out of bounds, assume it will be in the default + // implementation too, so we don't have to generate a GL error here + if (index >= c->tokenized_gl_extensions.size()) + return NULL; + + return (const GLubyte *)c->tokenized_gl_extensions.itemAt(index).string(); +} + +GLint egl_get_num_extensions_for_current_context() { + // NOTE: returning -1 here will fall-back to the default + // implementation. + + EGLContext context = egl_tls_t::getContext(); + if (context == EGL_NO_CONTEXT) + return -1; + + egl_context_t const * const c = get_context(context); + if (c == NULL) // this should never happen, by construction + return -1; + + return (GLint)c->tokenized_gl_extensions.size(); +} + // ---------------------------------------------------------------------------- // this mutex protects: diff --git a/opengl/libs/EGL/egl_object.cpp b/opengl/libs/EGL/egl_object.cpp index d3ee76d4d..d511940e3 100644 --- a/opengl/libs/EGL/egl_object.cpp +++ b/opengl/libs/EGL/egl_object.cpp @@ -113,6 +113,18 @@ void egl_context_t::onMakeCurrent(EGLSurface draw, EGLSurface read) { temp.append(gl_extensions); gl_extensions.setTo(temp); } + + // tokenize the supported extensions for the glGetStringi() wrapper + exts = gl_extensions.string(); + while (1) { + const char *end = strchr(exts, ' '); + if (end == NULL) { + tokenized_gl_extensions.push(String8(exts)); + break; + } + tokenized_gl_extensions.push(String8(exts, end - exts)); + exts = end + 1; + } } } diff --git a/opengl/libs/EGL/egl_object.h b/opengl/libs/EGL/egl_object.h index 518fdec21..f5a9f587b 100644 --- a/opengl/libs/EGL/egl_object.h +++ b/opengl/libs/EGL/egl_object.h @@ -27,6 +27,7 @@ #include #include +#include #include @@ -159,6 +160,7 @@ public: egl_connection_t const* cnx; int version; String8 gl_extensions; + Vector tokenized_gl_extensions; }; // ---------------------------------------------------------------------------- diff --git a/opengl/libs/GLES2/gl2.cpp b/opengl/libs/GLES2/gl2.cpp index d5dc0128e..6034a8edc 100644 --- a/opengl/libs/GLES2/gl2.cpp +++ b/opengl/libs/GLES2/gl2.cpp @@ -205,13 +205,22 @@ extern "C" { #undef CALL_GL_API_RETURN /* - * glGetString() is special because we expose some extensions in the wrapper + * glGetString() and glGetStringi() are special because we expose some + * extensions in the wrapper. Also, wrapping glGetXXX() is required because + * the value returned for GL_NUM_EXTENSIONS may have been altered by the + * injection of the additional extensions. */ -extern "C" const GLubyte * __glGetString(GLenum name); +extern "C" { + const GLubyte * __glGetString(GLenum name); + const GLubyte * __glGetStringi(GLenum name, GLuint index); + void __glGetBooleanv(GLenum pname, GLboolean * data); + void __glGetFloatv(GLenum pname, GLfloat * data); + void __glGetIntegerv(GLenum pname, GLint * data); + void __glGetInteger64v(GLenum pname, GLint64 * data); +} -const GLubyte * glGetString(GLenum name) -{ +const GLubyte * glGetString(GLenum name) { const GLubyte * ret = egl_get_string_for_current_context(name); if (ret == NULL) { gl_hooks_t::gl_t const * const _c = &getGlThreadSpecific()->gl; @@ -219,3 +228,64 @@ const GLubyte * glGetString(GLenum name) } return ret; } + +const GLubyte * glGetStringi(GLenum name, GLuint index) { + const GLubyte * ret = egl_get_string_for_current_context(name, index); + if (ret == NULL) { + gl_hooks_t::gl_t const * const _c = &getGlThreadSpecific()->gl; + if(_c) ret = _c->glGetStringi(name, index); + } + return ret; +} + +void glGetBooleanv(GLenum pname, GLboolean * data) { + if (pname == GL_NUM_EXTENSIONS) { + int num_exts = egl_get_num_extensions_for_current_context(); + if (num_exts >= 0) { + *data = num_exts > 0 ? GL_TRUE : GL_FALSE; + return; + } + } + + gl_hooks_t::gl_t const * const _c = &getGlThreadSpecific()->gl; + if (_c) _c->glGetBooleanv(pname, data); +} + +void glGetFloatv(GLenum pname, GLfloat * data) { + if (pname == GL_NUM_EXTENSIONS) { + int num_exts = egl_get_num_extensions_for_current_context(); + if (num_exts >= 0) { + *data = (GLfloat)num_exts; + return; + } + } + + gl_hooks_t::gl_t const * const _c = &getGlThreadSpecific()->gl; + if (_c) _c->glGetFloatv(pname, data); +} + +void glGetIntegerv(GLenum pname, GLint * data) { + if (pname == GL_NUM_EXTENSIONS) { + int num_exts = egl_get_num_extensions_for_current_context(); + if (num_exts >= 0) { + *data = (GLint)num_exts; + return; + } + } + + gl_hooks_t::gl_t const * const _c = &getGlThreadSpecific()->gl; + if (_c) _c->glGetIntegerv(pname, data); +} + +void glGetInteger64v(GLenum pname, GLint64 * data) { + if (pname == GL_NUM_EXTENSIONS) { + int num_exts = egl_get_num_extensions_for_current_context(); + if (num_exts >= 0) { + *data = (GLint64)num_exts; + return; + } + } + + gl_hooks_t::gl_t const * const _c = &getGlThreadSpecific()->gl; + if (_c) _c->glGetInteger64v(pname, data); +} diff --git a/opengl/libs/GLES2/gl2_api.in b/opengl/libs/GLES2/gl2_api.in index 836396051..09d8b0006 100644 --- a/opengl/libs/GLES2/gl2_api.in +++ b/opengl/libs/GLES2/gl2_api.in @@ -172,7 +172,7 @@ void API_ENTRY(glGetAttachedShaders)(GLuint program, GLsizei maxCount, GLsizei * GLint API_ENTRY(glGetAttribLocation)(GLuint program, const GLchar * name) { CALL_GL_API_RETURN(glGetAttribLocation, program, name); } -void API_ENTRY(glGetBooleanv)(GLenum pname, GLboolean * data) { +void API_ENTRY(__glGetBooleanv)(GLenum pname, GLboolean * data) { CALL_GL_API(glGetBooleanv, pname, data); } void API_ENTRY(glGetBufferParameteriv)(GLenum target, GLenum pname, GLint * params) { @@ -181,13 +181,13 @@ void API_ENTRY(glGetBufferParameteriv)(GLenum target, GLenum pname, GLint * para GLenum API_ENTRY(glGetError)(void) { CALL_GL_API_RETURN(glGetError); } -void API_ENTRY(glGetFloatv)(GLenum pname, GLfloat * data) { +void API_ENTRY(__glGetFloatv)(GLenum pname, GLfloat * data) { CALL_GL_API(glGetFloatv, pname, data); } void API_ENTRY(glGetFramebufferAttachmentParameteriv)(GLenum target, GLenum attachment, GLenum pname, GLint * params) { CALL_GL_API(glGetFramebufferAttachmentParameteriv, target, attachment, pname, params); } -void API_ENTRY(glGetIntegerv)(GLenum pname, GLint * data) { +void API_ENTRY(__glGetIntegerv)(GLenum pname, GLint * data) { CALL_GL_API(glGetIntegerv, pname, data); } void API_ENTRY(glGetProgramiv)(GLuint program, GLenum pname, GLint * params) { @@ -604,7 +604,7 @@ void API_ENTRY(glClearBufferfv)(GLenum buffer, GLint drawbuffer, const GLfloat * void API_ENTRY(glClearBufferfi)(GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil) { CALL_GL_API(glClearBufferfi, buffer, drawbuffer, depth, stencil); } -const GLubyte * API_ENTRY(glGetStringi)(GLenum name, GLuint index) { +const GLubyte * API_ENTRY(__glGetStringi)(GLenum name, GLuint index) { CALL_GL_API_RETURN(glGetStringi, name, index); } void API_ENTRY(glCopyBufferSubData)(GLenum readTarget, GLenum writeTarget, GLintptr readOffset, GLintptr writeOffset, GLsizeiptr size) { @@ -649,7 +649,7 @@ GLenum API_ENTRY(glClientWaitSync)(GLsync sync, GLbitfield flags, GLuint64 timeo void API_ENTRY(glWaitSync)(GLsync sync, GLbitfield flags, GLuint64 timeout) { CALL_GL_API(glWaitSync, sync, flags, timeout); } -void API_ENTRY(glGetInteger64v)(GLenum pname, GLint64 * data) { +void API_ENTRY(__glGetInteger64v)(GLenum pname, GLint64 * data) { CALL_GL_API(glGetInteger64v, pname, data); } void API_ENTRY(glGetSynciv)(GLsync sync, GLenum pname, GLsizei bufSize, GLsizei * length, GLint * values) { diff --git a/opengl/libs/egl_impl.h b/opengl/libs/egl_impl.h index cb0e908d3..c0990ece3 100644 --- a/opengl/libs/egl_impl.h +++ b/opengl/libs/egl_impl.h @@ -30,6 +30,9 @@ namespace android { // ---------------------------------------------------------------------------- EGLAPI const GLubyte * egl_get_string_for_current_context(GLenum name); +EGLAPI const GLubyte * egl_get_string_for_current_context(GLenum name, + GLuint index); +EGLAPI GLint egl_get_num_extensions_for_current_context(); // ---------------------------------------------------------------------------- }; // namespace android diff --git a/opengl/tools/glgen2/glgen.py b/opengl/tools/glgen2/glgen.py index ed6b451f7..9b30fd130 100755 --- a/opengl/tools/glgen2/glgen.py +++ b/opengl/tools/glgen2/glgen.py @@ -86,11 +86,25 @@ def fmtTypeNameList(params): return ', '.join(['"%s", %s' % (p[0], p[1]) for p in params]) -def overrideSymbolName(sym): - # The wrapper intercepts glGetString and (sometimes) calls the generated - # __glGetString thunk which dispatches to the driver's glGetString - if sym == 'glGetString': - return '__glGetString' +def overrideSymbolName(sym, apiname): + # The wrapper intercepts various glGet and glGetString functions and + # (sometimes) calls the generated thunk which dispatches to the + # driver's implementation + wrapped_get_syms = { + 'gles1' : [ + 'glGetString' + ], + 'gles2' : [ + 'glGetString', + 'glGetStringi', + 'glGetBooleanv', + 'glGetFloatv', + 'glGetIntegerv', + 'glGetInteger64v', + ], + } + if sym in wrapped_get_syms.get(apiname): + return '__' + sym else: return sym @@ -115,8 +129,8 @@ class TrampolineGen(reg.OutputGenerator): print('%s API_ENTRY(%s)(%s) {\n' ' %s(%s%s%s);\n' '}' - % (rtype, overrideSymbolName(fname), fmtParams(params), - call, fname, + % (rtype, overrideSymbolName(fname, self.genOpts.apiname), + fmtParams(params), call, fname, ', ' if len(params) > 0 else '', fmtArgs(params)), file=self.outFile)