From 16d7b6a8bb6f22ddd9252c6c0aefa84a53b65d53 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 18 Aug 2014 17:25:34 +0100 Subject: [PATCH] Don't try to capture the pointer when using PBOs for texture APIs If a PBO is bound, then the pointer argument is a relative offset from the start of the PBO, not an absolute address. Fixes b/17063439 Change-Id: I39629ce6c9bb3cb6bac5c9b0311287628306ddd4 (cherry picked from commit 0ad707f2f3d714af7d983d68330ed51dace815f5) --- .../libs/GLES_trace/src/gltrace_context.cpp | 33 ++++++++ opengl/libs/GLES_trace/src/gltrace_context.h | 8 ++ opengl/libs/GLES_trace/src/gltrace_fixup.cpp | 78 +++++++++++-------- 3 files changed, 85 insertions(+), 34 deletions(-) diff --git a/opengl/libs/GLES_trace/src/gltrace_context.cpp b/opengl/libs/GLES_trace/src/gltrace_context.cpp index 0323e8f46..d1b358677 100644 --- a/opengl/libs/GLES_trace/src/gltrace_context.cpp +++ b/opengl/libs/GLES_trace/src/gltrace_context.cpp @@ -133,6 +133,9 @@ GLTraceContext::GLTraceContext(int id, int version, GLTraceState *state, BufferedOutputStream *stream) : mId(id), mVersion(version), + mVersionMajor(0), + mVersionMinor(0), + mVersionParsed(false), mState(state), mBufferedOutputStream(stream), mElementArrayBuffers(DefaultKeyedVector(NULL)) @@ -149,10 +152,40 @@ int GLTraceContext::getVersion() { return mVersion; } +int GLTraceContext::getVersionMajor() { + if (!mVersionParsed) { + parseGlesVersion(); + mVersionParsed = true; + } + return mVersionMajor; +} + +int GLTraceContext::getVersionMinor() { + if (!mVersionParsed) { + parseGlesVersion(); + mVersionParsed = true; + } + return mVersionMinor; +} + GLTraceState *GLTraceContext::getGlobalTraceState() { return mState; } +void GLTraceContext::parseGlesVersion() { + const char* str = (const char*)hooks->gl.glGetString(GL_VERSION); + int major, minor; + if (sscanf(str, "OpenGL ES-CM %d.%d", &major, &minor) != 2) { + if (sscanf(str, "OpenGL ES %d.%d", &major, &minor) != 2) { + ALOGW("Unable to parse GL_VERSION string: \"%s\"", str); + major = 1; + minor = 0; + } + } + mVersionMajor = major; + mVersionMinor = minor; +} + void GLTraceContext::resizeFBMemory(unsigned minSize) { if (fbcontentsSize >= minSize) { return; diff --git a/opengl/libs/GLES_trace/src/gltrace_context.h b/opengl/libs/GLES_trace/src/gltrace_context.h index 4c38bbad2..38c731583 100644 --- a/opengl/libs/GLES_trace/src/gltrace_context.h +++ b/opengl/libs/GLES_trace/src/gltrace_context.h @@ -51,6 +51,9 @@ public: class GLTraceContext { int mId; /* unique context id */ int mVersion; /* GL version, e.g: egl_connection_t::GLESv2_INDEX */ + int mVersionMajor; /* GL major version. Lazily parsed in getVersionX(). */ + int mVersionMinor; /* GL minor version. Lazily parsed in getVersionX(). */ + bool mVersionParsed; /* True if major and minor versions have been parsed. */ GLTraceState *mState; /* parent GL Trace state (for per process GL Trace State Info) */ void *fbcontents; /* memory area to read framebuffer contents */ @@ -62,6 +65,9 @@ class GLTraceContext { /* list of element array buffers in use. */ DefaultKeyedVector mElementArrayBuffers; + /* Parses the GL version string returned from glGetString(GL_VERSION) to get find the major and + minor versions of the GLES API. The context must be current before calling. */ + void parseGlesVersion(); void resizeFBMemory(unsigned minSize); public: gl_hooks_t *hooks; @@ -69,6 +75,8 @@ public: GLTraceContext(int id, int version, GLTraceState *state, BufferedOutputStream *stream); int getId(); int getVersion(); + int getVersionMajor(); + int getVersionMinor(); GLTraceState *getGlobalTraceState(); void getCompressedFB(void **fb, unsigned *fbsize, unsigned *fbwidth, unsigned *fbheight, diff --git a/opengl/libs/GLES_trace/src/gltrace_fixup.cpp b/opengl/libs/GLES_trace/src/gltrace_fixup.cpp index e6d006271..be729c705 100644 --- a/opengl/libs/GLES_trace/src/gltrace_fixup.cpp +++ b/opengl/libs/GLES_trace/src/gltrace_fixup.cpp @@ -29,6 +29,33 @@ namespace android { namespace gltrace { +GLint glGetInteger(GLTraceContext *context, GLenum param) { + GLint x; + context->hooks->gl.glGetIntegerv(param, &x); + return x; +} + +GLint glGetVertexAttrib(GLTraceContext *context, GLuint index, GLenum pname) { + GLint x; + context->hooks->gl.glGetVertexAttribiv(index, pname, &x); + return x; +} + +bool isUsingPixelBuffers(GLTraceContext *context) { + if (context->getVersionMajor() < 3) { + return false; // PBOs not supported prior to GLES 3.0 + } + return glGetInteger(context, GL_PIXEL_UNPACK_BUFFER_BINDING) != 0; +} + +bool isUsingArrayBuffers(GLTraceContext *context) { + return glGetInteger(context, GL_ARRAY_BUFFER_BINDING) != 0; +} + +bool isUsingElementArrayBuffers(GLTraceContext *context) { + return glGetInteger(context, GL_ELEMENT_ARRAY_BUFFER_BINDING) != 0; +} + unsigned getBytesPerTexel(const GLenum format, const GLenum type) { /* Description from glTexImage2D spec: @@ -156,7 +183,8 @@ void fixup_addFBContents(GLTraceContext *context, GLMessage *glmsg, FBBinding fb } /** Common fixup routing for glTexImage2D & glTexSubImage2D. */ -void fixup_glTexImage(int widthIndex, int heightIndex, GLMessage *glmsg, void *dataSrc) { +void fixup_glTexImage(GLTraceContext *context, int widthIndex, int heightIndex, GLMessage *glmsg, + void *dataSrc) { GLMessage_DataType arg_width = glmsg->args(widthIndex); GLMessage_DataType arg_height = glmsg->args(heightIndex); @@ -175,7 +203,7 @@ void fixup_glTexImage(int widthIndex, int heightIndex, GLMessage *glmsg, void *d arg_data->set_type(GLMessage::DataType::BYTE); arg_data->clear_rawbytes(); - if (data != NULL) { + if (data != NULL && !isUsingPixelBuffers(context)) { arg_data->set_isarray(true); arg_data->add_rawbytes(data, bytesPerTexel * width * height); } else { @@ -185,7 +213,7 @@ void fixup_glTexImage(int widthIndex, int heightIndex, GLMessage *glmsg, void *d } -void fixup_glTexImage2D(GLMessage *glmsg, void *pointersToFixup[]) { +void fixup_glTexImage2D(GLTraceContext *context, GLMessage *glmsg, void *pointersToFixup[]) { /* void glTexImage2D(GLenum target, GLint level, GLint internalformat, @@ -198,10 +226,10 @@ void fixup_glTexImage2D(GLMessage *glmsg, void *pointersToFixup[]) { */ int widthIndex = 3; int heightIndex = 4; - fixup_glTexImage(widthIndex, heightIndex, glmsg, pointersToFixup[0]); + fixup_glTexImage(context, widthIndex, heightIndex, glmsg, pointersToFixup[0]); } -void fixup_glTexSubImage2D(GLMessage *glmsg, void *pointersToFixup[]) { +void fixup_glTexSubImage2D(GLTraceContext *context, GLMessage *glmsg, void *pointersToFixup[]) { /* void glTexSubImage2D(GLenum target, GLint level, @@ -215,10 +243,11 @@ void fixup_glTexSubImage2D(GLMessage *glmsg, void *pointersToFixup[]) { */ int widthIndex = 4; int heightIndex = 5; - fixup_glTexImage(widthIndex, heightIndex, glmsg, pointersToFixup[0]); + fixup_glTexImage(context, widthIndex, heightIndex, glmsg, pointersToFixup[0]); } -void fixup_glCompressedTexImage2D(GLMessage *glmsg, void *pointersToFixup[]) { +void fixup_glCompressedTexImage2D(GLTraceContext *context, GLMessage *glmsg, + void *pointersToFixup[]) { /* void glCompressedTexImage2D(GLenum target, GLint level, GLenum internalformat, @@ -235,7 +264,7 @@ void fixup_glCompressedTexImage2D(GLMessage *glmsg, void *pointersToFixup[]) { arg_data->set_type(GLMessage::DataType::BYTE); arg_data->clear_rawbytes(); - if (data != NULL) { + if (data != NULL && !isUsingPixelBuffers(context)) { arg_data->set_isarray(true); arg_data->add_rawbytes(data, size); } else { @@ -244,7 +273,8 @@ void fixup_glCompressedTexImage2D(GLMessage *glmsg, void *pointersToFixup[]) { } } -void fixup_glCompressedTexSubImage2D(GLMessage *glmsg, void *pointersToFixup[]) { +void fixup_glCompressedTexSubImage2D(GLTraceContext *context, GLMessage *glmsg, + void *pointersToFixup[]) { /* void glCompressedTexSubImage2D(GLenum target, GLint level, GLint xoffset, @@ -262,7 +292,7 @@ void fixup_glCompressedTexSubImage2D(GLMessage *glmsg, void *pointersToFixup[]) arg_data->set_type(GLMessage::DataType::BYTE); arg_data->clear_rawbytes(); - if (data != NULL) { + if (data != NULL && !isUsingPixelBuffers(context)) { arg_data->set_isarray(true); arg_data->add_rawbytes(data, size); } else { @@ -436,26 +466,6 @@ void fixup_glGetActiveAttribOrUniform(GLTraceContext *context, GLMessage *glmsg, arg_location->add_intvalue(location); } -GLint glGetInteger(GLTraceContext *context, GLenum param) { - GLint x; - context->hooks->gl.glGetIntegerv(param, &x); - return x; -} - -GLint glGetVertexAttrib(GLTraceContext *context, GLuint index, GLenum pname) { - GLint x; - context->hooks->gl.glGetVertexAttribiv(index, pname, &x); - return x; -} - -bool isUsingArrayBuffers(GLTraceContext *context) { - return glGetInteger(context, GL_ARRAY_BUFFER_BINDING) != 0; -} - -bool isUsingElementArrayBuffers(GLTraceContext *context) { - return glGetInteger(context, GL_ELEMENT_ARRAY_BUFFER_BINDING) != 0; -} - /** Copy @len bytes of data from @src into the @dataIndex'th argument of the message. */ void addGlBufferData(GLMessage *glmsg, int dataIndex, GLvoid *src, GLsizeiptr len) { GLMessage_DataType *arg_datap = glmsg->mutable_args(dataIndex); @@ -809,22 +819,22 @@ void fixupGLMessage(GLTraceContext *context, nsecs_t wallStart, nsecs_t wallEnd, break; case GLMessage::glTexImage2D: if (context->getGlobalTraceState()->shouldCollectTextureDataOnGlTexImage()) { - fixup_glTexImage2D(glmsg, pointersToFixup); + fixup_glTexImage2D(context, glmsg, pointersToFixup); } break; case GLMessage::glTexSubImage2D: if (context->getGlobalTraceState()->shouldCollectTextureDataOnGlTexImage()) { - fixup_glTexSubImage2D(glmsg, pointersToFixup); + fixup_glTexSubImage2D(context, glmsg, pointersToFixup); } break; case GLMessage::glCompressedTexImage2D: if (context->getGlobalTraceState()->shouldCollectTextureDataOnGlTexImage()) { - fixup_glCompressedTexImage2D(glmsg, pointersToFixup); + fixup_glCompressedTexImage2D(context, glmsg, pointersToFixup); } break; case GLMessage::glCompressedTexSubImage2D: if (context->getGlobalTraceState()->shouldCollectTextureDataOnGlTexImage()) { - fixup_glCompressedTexSubImage2D(glmsg, pointersToFixup); + fixup_glCompressedTexSubImage2D(context, glmsg, pointersToFixup); } break; case GLMessage::glShaderSource: