<div dir="ltr"><div>Looks good AFAICS. Just cosmetic requests.</div><div> </div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Aug 10, 2015 at 11:14 PM, Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">If we have the NV_read_depth_stencil extension, we can actually read depth/stencil. Takes a bit of gymnastics to actually figure out that it is a depth/stencil buffer in the first place. TODO: this still seems to fall over w/ z24_s8, somehow ends up attempting to glReadPixels() with GL_FLOAT type. Still trying to untangle all the logic here... gles really doesn't make it easy. Signed-off-by: Rob Clark <<a href="mailto:robdclark@gmail.com">robdclark@gmail.com</a>> --- Re-sending this and following patch to apitrace list (already sent to mesa list). The corresponding mesa patches to implement the extensions used were sent earlier today. retrace/glstate.cpp | 3 ++ retrace/glstate_images.cpp | 69 ++++++++++++++++++++++++++++++-------------- retrace/glstate_internal.hpp | 1 + 3 files changed, 52 insertions(+), 21 deletions(-) diff --git a/retrace/glstate.cpp b/retrace/glstate.cpp index 3d3c026..4386e08 100644 --- a/retrace/glstate.cpp +++ b/retrace/glstate.cpp @@ -59,6 +59,9 @@ Context::Context(void) { ARB_get_program_binary = ext.has("GL_ARB_get_program_binary"); KHR_debug = !ES && ext.has("GL_KHR_debug"); EXT_debug_label = ext.has("GL_EXT_debug_label"); + + if (ES) + NV_read_depth_stencil = ext.has("GL_NV_read_depth_stencil"); } PixelPackState::PixelPackState(const Context &context) { diff --git a/retrace/glstate_images.cpp b/retrace/glstate_images.cpp index c111efa..90ae3e0 100644 --- a/retrace/glstate_images.cpp +++ b/retrace/glstate_images.cpp @@ -102,11 +102,11 @@ struct ImageDesc * call. */ static bool -probeTextureLevelSizeOES(GLenum target, GLint level, const GLint size[3]) { +probeTextureLevelSizeOES(GLenum target, GLint level, const GLint size[3], + GLenum internalFormat, GLenum type) +{ flushErrors(); - GLenum internalFormat = GL_RGBA; - GLenum type = GL_UNSIGNED_BYTE; GLint dummy = 0; switch (target) { @@ -150,7 +150,9 @@ probeTextureLevelSizeOES(GLenum target, GLint level, const GLint size[3]) { * It is assumed that the texture exists. */ static GLint -bisectTextureLevelSizeOES(GLenum target, GLint level, GLint axis, GLint max) { +bisectTextureLevelSizeOES(GLenum target, GLint level, GLint axis, GLint max, + GLenum internalFormat, GLenum type) +{ GLint size[3] = {0, 0, 0}; assert(axis < 3); @@ -165,7 +167,7 @@ bisectTextureLevelSizeOES(GLenum target, GLint level, GLint axis, GLint max) { size[axis] = test; - if (probeTextureLevelSizeOES(target, level, size)) { + if (probeTextureLevelSizeOES(target, level, size, internalFormat, type)) { min = test; } else { max = test; @@ -186,20 +188,43 @@ getActiveTextureLevelDescOES(Context &context, GLenum target, GLint level, Image return false; } - const GLint size[3] = {1, 1, 1}; - if (!probeTextureLevelSizeOES(target, level, size)) { + GLenum formatAndTypes[] = { + /* internalFormat */ /* type */ + GL_RGBA, GL_UNSIGNED_BYTE, + GL_DEPTH_STENCIL, GL_FLOAT_32_UNSIGNED_INT_24_8_REV, + GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8, + GL_DEPTH_COMPONENT, GL_FLOAT, + GL_DEPTH_COMPONENT, GL_UNSIGNED_INT, + GL_STENCIL_INDEX, GL_UNSIGNED_BYTE, + /* others? */ + 0, + }; </blockquote><div> </div><div>Please use a structure here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + const GLint size[3] = {1, 1, 1}; + GLenum internalFormat = GL_NONE; + GLenum type; + + for (int i = 0; formatAndTypes[i]; i += 2) { + if (probeTextureLevelSizeOES(target, level, size, + formatAndTypes[i], + formatAndTypes[i+1])) { + internalFormat = formatAndTypes[i]; + type = formatAndTypes[i+1]; + break; + } + } + </blockquote><div> </div><div>Please put this new code in a new probeTextureFormatOES function.</div><div> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + if (internalFormat == GL_NONE) { return false; } - // XXX: mere guess - desc.internalFormat = GL_RGBA; + desc.internalFormat = internalFormat; GLint maxSize = 0; switch (target) { case GL_TEXTURE_2D: glGetIntegerv(GL_MAX_TEXTURE_SIZE, &maxSize); - desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize); - desc.height = bisectTextureLevelSizeOES(target, level, 1, maxSize); + desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize, internalFormat, type); + desc.height = bisectTextureLevelSizeOES(target, level, 1, maxSize, internalFormat, type); desc.depth = 1; break; case GL_TEXTURE_CUBE_MAP: @@ -210,15 +235,15 @@ getActiveTextureLevelDescOES(Context &context, GLenum target, GLint level, Image case GL_TEXTURE_CUBE_MAP_POSITIVE_Z: case GL_TEXTURE_CUBE_MAP_NEGATIVE_Z: glGetIntegerv(GL_MAX_CUBE_MAP_TEXTURE_SIZE, &maxSize); - desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize); + desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize, internalFormat, type); desc.height = desc.width; desc.depth = 1; break; case GL_TEXTURE_3D_OES: glGetIntegerv(GL_MAX_3D_TEXTURE_SIZE_OES, &maxSize); - desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize); - desc.width = bisectTextureLevelSizeOES(target, level, 1, maxSize); - desc.depth = bisectTextureLevelSizeOES(target, level, 2, maxSize); + desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize, internalFormat, type); + desc.width = bisectTextureLevelSizeOES(target, level, 1, maxSize, internalFormat, type); + desc.depth = bisectTextureLevelSizeOES(target, level, 2, maxSize, internalFormat, type); break; default: return false; @@ -376,7 +401,8 @@ getTextureBinding(GLenum target) * texture to a framebuffer. */ static inline void -getTexImageOES(GLenum target, GLint level, ImageDesc &desc, GLubyte *pixels) +getTexImageOES(GLenum target, GLint level, GLenum format, GLenum type, + ImageDesc &desc, GLubyte *pixels) { memset(pixels, 0x80, desc.height * desc.width * 4); @@ -412,12 +438,13 @@ getTexImageOES(GLenum target, GLint level, ImageDesc &desc, GLubyte *pixels) if (status != GL_FRAMEBUFFER_COMPLETE) { std::cerr << __FUNCTION__ << ": " << enumToString(status) << "\n"; } - glReadPixels(0, 0, desc.width, desc.height, GL_RGBA, GL_UNSIGNED_BYTE, pixels); + + glReadPixels(0, 0, desc.width, desc.height, format, type, pixels); break; case GL_TEXTURE_3D_OES: for (int i = 0; i < desc.depth; i++) { glFramebufferTexture3D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_3D, texture, level, i); - glReadPixels(0, 0, desc.width, desc.height, GL_RGBA, GL_UNSIGNED_BYTE, pixels + 4 * i * desc.width * desc.height); + glReadPixels(0, 0, desc.width, desc.height, format, type, pixels + 4 * i * desc.width * desc.height); } break; } @@ -498,7 +525,7 @@ dumpActiveTextureLevel(StateWriter &writer, Context &context, } } else { if (context.ES) { - getTexImageOES(target, level, desc, image->pixels); + getTexImageOES(target, level, format, type, desc, image->pixels); } else { glGetTexImage(target, level, format, type, image->pixels); } @@ -1005,7 +1032,7 @@ dumpReadBufferImage(StateWriter &writer, } // On GLES glReadPixels only supports GL_RGBA and GL_UNSIGNED_BYTE combination - if (context.ES) { + if (context.ES && !context.NV_read_depth_stencil) { format = GL_RGBA; type = GL_UNSIGNED_BYTE; } @@ -1366,7 +1393,7 @@ dumpFramebufferAttachments(StateWriter &writer, Context &context, GLenum target) glReadBuffer(read_buffer); - if (!context.ES) { + if ((!context.ES) || context.NV_read_depth_stencil) { dumpFramebufferAttachment(writer, context, target, GL_DEPTH_ATTACHMENT); dumpFramebufferAttachment(writer, context, target, GL_STENCIL_ATTACHMENT); } diff --git a/retrace/glstate_internal.hpp b/retrace/glstate_internal.hpp index 6f9086e..c180890 100644 --- a/retrace/glstate_internal.hpp +++ b/retrace/glstate_internal.hpp @@ -49,6 +49,7 @@ struct Context unsigned ARB_get_program_binary:1; unsigned KHR_debug:1; unsigned EXT_debug_label:1; + unsigned NV_read_depth_stencil:1; /* ES only */ Context(void); }; -- 2.4.3 </blockquote></div></div><div class="gmail_extra"> </div><div class="gmail_extra">Jose</div></div>