<div dir="ltr"><div>Thanks for the update.</div><div><br></div>Before submiting I did basic check with NVIDIA proprietary drivers (which supports NV_read_depth_stencil), and your patch causes reading the back buffer (ie, no framebuffer bound) to regress.<br><div><br></div><div>Before your patch glretrace would do to get the back buffer contents:</div><div><br></div><div> glReadPixels(x = 0, y = 0, width = 300, height = 300, format = GL_RGBA , type = GL_UNSIGNED_BYTE, pixels)<br></div><div><div> glGetError() = GL_NO_ERROR</div></div><div><br></div><div>After your patch glretrace does:</div><div><br></div><div><div><div> glReadPixels(x = 0, y = 0, width = 300, height = 300, format = GL_RGB, type = GL_UNSIGNED_BYTE, pixels)</div><div> 486: message: major api error 1282: GL_INVALID_OPERATION error generated. Invalid <format> parameter for current read framebuffer</div><div> glGetError() = GL_INVALID_OPERATION<br></div></div><div><br></div><div>This is basic functionality so it must not regress.</div><div><br></div><div>You can repro this yourself by doing</div><div><br></div><div> wget <a href="http://people.freedesktop.org/~jrfonseca/traces/egl_gles2_tri_glsl.trace">http://people.freedesktop.org/~jrfonseca/traces/egl_gles2_tri_glsl.trace</a></div><div> glretrace -D 486 egl_gles2_tri_glsl.trace</div><div><br></div><div>(One has to use glretrace instead of eglretrace because NVIDIA drivers usually only supports GLES context via GLX.)</div><div><br></div><div>I haven't tried with Mesa. You might need to use</div><div><br></div><div> eglretrace -D 486 egl_gles2_tri_glsl.trace</div><div><br></div><div>Also please run <a href="https://github.com/apitrace/apitrace-tests">https://github.com/apitrace/apitrace-tests</a> tests -- I just beefed them up to cover this case.</div><div><br></div><div>Jose</div><div><div><div><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 3, 2015 at 12:44 AM, Rob Clark <span dir="ltr"><<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><span>If we have the NV_read_depth_stencil extension, we can actually read<br>
depth/stencil. Takes a bit of gymnastics to actually figure out that<br>
it is a depth/stencil buffer in the first place.<br>
<br>
</span><span>Signed-off-by: Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>><br>
---<br>
</span>Reworked this according to José's comments the other week on the way<br>
out to LPC, but forgot to resend the patch..<br>
<br>
retrace/glstate.cpp | 3 ++<br>
retrace/glstate_images.cpp | 79 ++++++++++++++++++++++++++++++++------------<br>
retrace/glstate_internal.hpp | 1 +<br>
3 files changed, 61 insertions(+), 22 deletions(-)<br>
<span><br>
diff --git a/retrace/glstate.cpp b/retrace/glstate.cpp<br>
index 3d3c026..4386e08 100644<br>
--- a/retrace/glstate.cpp<br>
+++ b/retrace/glstate.cpp<br>
@@ -59,6 +59,9 @@ Context::Context(void) {<br>
ARB_get_program_binary = ext.has("GL_ARB_get_program_binary");<br>
KHR_debug = !ES && ext.has("GL_KHR_debug");<br>
EXT_debug_label = ext.has("GL_EXT_debug_label");<br>
+<br>
+ if (ES)<br>
+ NV_read_depth_stencil = ext.has("GL_NV_read_depth_stencil");<br>
}<br>
<br>
PixelPackState::PixelPackState(const Context &context) {<br>
diff --git a/retrace/glstate_images.cpp b/retrace/glstate_images.cpp<br>
</span>index c111efa..d893b74 100644<br>
<span>--- a/retrace/glstate_images.cpp<br>
+++ b/retrace/glstate_images.cpp<br>
@@ -102,11 +102,11 @@ struct ImageDesc<br>
* call.<br>
*/<br>
static bool<br>
-probeTextureLevelSizeOES(GLenum target, GLint level, const GLint size[3]) {<br>
+probeTextureLevelSizeOES(GLenum target, GLint level, const GLint size[3],<br>
+ GLenum internalFormat, GLenum type)<br>
+{<br>
flushErrors();<br>
<br>
- GLenum internalFormat = GL_RGBA;<br>
- GLenum type = GL_UNSIGNED_BYTE;<br>
GLint dummy = 0;<br>
<br>
switch (target) {<br>
</span>@@ -143,6 +143,39 @@ probeTextureLevelSizeOES(GLenum target, GLint level, const GLint size[3]) {<br>
return false;<br>
}<br>
<br>
+static bool<br>
+probeTextureFormatOES(GLenum target, GLint level,<br>
+ GLenum *internalFormat, GLenum *type)<br>
+{<br>
+ const struct {<br>
+ GLenum internalFormat;<br>
+ GLenum type;<br>
+ } info[] = {<br>
<span>+ /* internalFormat */ /* type */<br>
+ { GL_RGBA, GL_UNSIGNED_BYTE },<br>
</span>+ { GL_DEPTH_STENCIL, GL_FLOAT_32_UNSIGNED_INT_24_8_REV },<br>
+ { GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8 },<br>
+ { GL_DEPTH_COMPONENT, GL_FLOAT },<br>
+ { GL_DEPTH_COMPONENT, GL_UNSIGNED_INT },<br>
+ { GL_STENCIL_INDEX, GL_UNSIGNED_BYTE },<br>
+ /* others? */<br>
+ { 0, 0 },<br>
+ };<br>
<span>+ const GLint size[3] = {1, 1, 1};<br>
+<br>
</span>+ for (int i = 0; info[i].internalFormat; i++) {<br>
<span>+ if (probeTextureLevelSizeOES(target, level, size,<br>
</span>+ info[i].internalFormat,<br>
+ info[i].type)) {<br>
+ *internalFormat = info[i].internalFormat;<br>
+ *type = info[i].type;<br>
+ return true;<br>
+ }<br>
+ }<br>
+<br>
+ return false;<br>
+}<br>
+<br>
<br>
/**<br>
* Bisect the texture size along an axis.<br>
@@ -150,7 +183,9 @@ probeTextureLevelSizeOES(GLenum target, GLint level, const GLint size[3]) {<br>
<span> * It is assumed that the texture exists.<br>
*/<br>
static GLint<br>
-bisectTextureLevelSizeOES(GLenum target, GLint level, GLint axis, GLint max) {<br>
+bisectTextureLevelSizeOES(GLenum target, GLint level, GLint axis, GLint max,<br>
+ GLenum internalFormat, GLenum type)<br>
+{<br>
GLint size[3] = {0, 0, 0};<br>
<br>
assert(axis < 3);<br>
</span>@@ -165,7 +200,7 @@ bisectTextureLevelSizeOES(GLenum target, GLint level, GLint axis, GLint max) {<br>
<span><br>
size[axis] = test;<br>
<br>
- if (probeTextureLevelSizeOES(target, level, size)) {<br>
+ if (probeTextureLevelSizeOES(target, level, size, internalFormat, type)) {<br>
min = test;<br>
} else {<br>
max = test;<br>
</span>@@ -185,21 +220,19 @@ getActiveTextureLevelDescOES(Context &context, GLenum target, GLint level, Image<br>
// OpenGL ES does not support 1D textures<br>
return false;<br>
}<br>
+ GLenum internalFormat, type;<br>
<span><br>
- const GLint size[3] = {1, 1, 1};<br>
- if (!probeTextureLevelSizeOES(target, level, size)) {<br>
</span>+ if (!probeTextureFormatOES(target, level, &internalFormat, &type))<br>
return false;<br>
- }<br>
<span><br>
- // XXX: mere guess<br>
- desc.internalFormat = GL_RGBA;<br>
+ desc.internalFormat = internalFormat;<br>
<br>
GLint maxSize = 0;<br>
switch (target) {<br>
case GL_TEXTURE_2D:<br>
glGetIntegerv(GL_MAX_TEXTURE_SIZE, &maxSize);<br>
- desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize);<br>
- desc.height = bisectTextureLevelSizeOES(target, level, 1, maxSize);<br>
+ desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize, internalFormat, type);<br>
+ desc.height = bisectTextureLevelSizeOES(target, level, 1, maxSize, internalFormat, type);<br>
desc.depth = 1;<br>
break;<br>
case GL_TEXTURE_CUBE_MAP:<br>
</span>@@ -210,15 +243,15 @@ getActiveTextureLevelDescOES(Context &context, GLenum target, GLint level, Image<br>
<span> case GL_TEXTURE_CUBE_MAP_POSITIVE_Z:<br>
case GL_TEXTURE_CUBE_MAP_NEGATIVE_Z:<br>
glGetIntegerv(GL_MAX_CUBE_MAP_TEXTURE_SIZE, &maxSize);<br>
- desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize);<br>
+ desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize, internalFormat, type);<br>
desc.height = desc.width;<br>
desc.depth = 1;<br>
break;<br>
case GL_TEXTURE_3D_OES:<br>
glGetIntegerv(GL_MAX_3D_TEXTURE_SIZE_OES, &maxSize);<br>
- desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize);<br>
- desc.width = bisectTextureLevelSizeOES(target, level, 1, maxSize);<br>
- desc.depth = bisectTextureLevelSizeOES(target, level, 2, maxSize);<br>
+ desc.width = bisectTextureLevelSizeOES(target, level, 0, maxSize, internalFormat, type);<br>
+ desc.width = bisectTextureLevelSizeOES(target, level, 1, maxSize, internalFormat, type);<br>
+ desc.depth = bisectTextureLevelSizeOES(target, level, 2, maxSize, internalFormat, type);<br>
break;<br>
default:<br>
return false;<br>
</span>@@ -376,7 +409,8 @@ getTextureBinding(GLenum target)<br>
<span> * texture to a framebuffer.<br>
*/<br>
static inline void<br>
-getTexImageOES(GLenum target, GLint level, ImageDesc &desc, GLubyte *pixels)<br>
+getTexImageOES(GLenum target, GLint level, GLenum format, GLenum type,<br>
+ ImageDesc &desc, GLubyte *pixels)<br>
{<br>
memset(pixels, 0x80, desc.height * desc.width * 4);<br>
<br>
</span>@@ -412,12 +446,13 @@ getTexImageOES(GLenum target, GLint level, ImageDesc &desc, GLubyte *pixels)<br>
<span> if (status != GL_FRAMEBUFFER_COMPLETE) {<br>
std::cerr << __FUNCTION__ << ": " << enumToString(status) << "\n";<br>
}<br>
- glReadPixels(0, 0, desc.width, desc.height, GL_RGBA, GL_UNSIGNED_BYTE, pixels);<br>
+<br>
+ glReadPixels(0, 0, desc.width, desc.height, format, type, pixels);<br>
break;<br>
case GL_TEXTURE_3D_OES:<br>
for (int i = 0; i < desc.depth; i++) {<br>
glFramebufferTexture3D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_3D, texture, level, i);<br>
- glReadPixels(0, 0, desc.width, desc.height, GL_RGBA, GL_UNSIGNED_BYTE, pixels + 4 * i * desc.width * desc.height);<br>
+ glReadPixels(0, 0, desc.width, desc.height, format, type, pixels + 4 * i * desc.width * desc.height);<br>
}<br>
break;<br>
}<br>
</span>@@ -498,7 +533,7 @@ dumpActiveTextureLevel(StateWriter &writer, Context &context,<br>
<span> }<br>
} else {<br>
if (context.ES) {<br>
- getTexImageOES(target, level, desc, image->pixels);<br>
+ getTexImageOES(target, level, format, type, desc, image->pixels);<br>
} else {<br>
glGetTexImage(target, level, format, type, image->pixels);<br>
}<br>
</span>@@ -1005,7 +1040,7 @@ dumpReadBufferImage(StateWriter &writer,<br>
<span> }<br>
<br>
// On GLES glReadPixels only supports GL_RGBA and GL_UNSIGNED_BYTE combination<br>
- if (context.ES) {<br>
+ if (context.ES && !context.NV_read_depth_stencil) {<br>
format = GL_RGBA;<br>
type = GL_UNSIGNED_BYTE;<br>
}<br>
</span>@@ -1366,7 +1401,7 @@ dumpFramebufferAttachments(StateWriter &writer, Context &context, GLenum target)<br>
<div><div><br>
glReadBuffer(read_buffer);<br>
<br>
- if (!context.ES) {<br>
+ if ((!context.ES) || context.NV_read_depth_stencil) {<br>
dumpFramebufferAttachment(writer, context, target, GL_DEPTH_ATTACHMENT);<br>
dumpFramebufferAttachment(writer, context, target, GL_STENCIL_ATTACHMENT);<br>
}<br>
diff --git a/retrace/glstate_internal.hpp b/retrace/glstate_internal.hpp<br>
index 6f9086e..c180890 100644<br>
--- a/retrace/glstate_internal.hpp<br>
+++ b/retrace/glstate_internal.hpp<br>
@@ -49,6 +49,7 @@ struct Context<br>
unsigned ARB_get_program_binary:1;<br>
unsigned KHR_debug:1;<br>
unsigned EXT_debug_label:1;<br>
+ unsigned NV_read_depth_stencil:1; /* ES only */<br>
<br>
Context(void);<br>
};<br>
--<br>
2.4.3<br>
<br>
</div></div></blockquote></div><br></div></div></div></div></div></div></div>