<div dir="ltr"><div><div>I fixed this and pushed the change.<br><br>BTW, on your first first version you said:<br><br>   "TODO: this still seems to fall over w/ z24_s8, somehow ends up<br>attempting to glReadPixels() with GL_FLOAT type.  Still trying to<br>untangle all the logic here... gles really doesn't make it easy."<br><br>I understand why and how to fix this.  See the following comment on the code if you're interested:</div><div><br>           /* FIXME: NV_read_depth_stencil states that type must match the<br>             * internal format, whereas we always request GL_FLOAT, as that's<br>             * what we want.  To fix this we should probe the adequate type<br>             * here, and then manually convert the pixels to float after<br>             * glReadPixels */<br><br>Jose</div><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 3, 2015 at 10:44 PM, José Fonseca <span dir="ltr"><<a href="mailto:jose.r.fonseca@gmail.com" target="_blank">jose.r.fonseca@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><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" target="_blank">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" target="_blank">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></blockquote></div></div></div></div></div>