[PATCH] retrace: support depth/stencil attachment for gles

José Fonseca jose.r.fonseca at gmail.com
Thu Sep 3 14:44:49 PDT 2015


Thanks for the update.

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.

Before your patch glretrace would do to get the back buffer contents:

  glReadPixels(x = 0, y = 0, width = 300, height = 300, format = GL_RGBA ,
type = GL_UNSIGNED_BYTE, pixels)
  glGetError() = GL_NO_ERROR

After your patch glretrace does:

  glReadPixels(x = 0, y = 0, width = 300, height = 300, format = GL_RGB,
type = GL_UNSIGNED_BYTE, pixels)
  486: message: major api error 1282: GL_INVALID_OPERATION error generated.
Invalid <format> parameter for current read framebuffer
  glGetError() = GL_INVALID_OPERATION

This is basic functionality so it must not regress.

You can repro this yourself by doing

   wget
http://people.freedesktop.org/~jrfonseca/traces/egl_gles2_tri_glsl.trace
   glretrace -D 486 egl_gles2_tri_glsl.trace

(One has to use glretrace instead of eglretrace because NVIDIA drivers
usually only supports GLES context via GLX.)

I haven't tried with Mesa.  You might need to use

   eglretrace -D 486 egl_gles2_tri_glsl.trace

Also please run https://github.com/apitrace/apitrace-tests tests -- I just
beefed them up to cover this case.

Jose


On Thu, Sep 3, 2015 at 12:44 AM, Rob Clark <robdclark at gmail.com> wrote:

> 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.
>
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
> Reworked this according to José's comments the other week on the way
> out to LPC, but forgot to resend the patch..
>
>  retrace/glstate.cpp          |  3 ++
>  retrace/glstate_images.cpp   | 79
> ++++++++++++++++++++++++++++++++------------
>  retrace/glstate_internal.hpp |  1 +
>  3 files changed, 61 insertions(+), 22 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..d893b74 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) {
> @@ -143,6 +143,39 @@ probeTextureLevelSizeOES(GLenum target, GLint level,
> const GLint size[3]) {
>      return false;
>  }
>
> +static bool
> +probeTextureFormatOES(GLenum target, GLint level,
> +                      GLenum *internalFormat, GLenum *type)
> +{
> +    const struct {
> +        GLenum internalFormat;
> +        GLenum type;
> +    } info[] = {
> +        /* 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, 0 },
> +    };
> +    const GLint size[3] = {1, 1, 1};
> +
> +    for (int i = 0; info[i].internalFormat; i++) {
> +        if (probeTextureLevelSizeOES(target, level, size,
> +                                     info[i].internalFormat,
> +                                     info[i].type)) {
> +            *internalFormat = info[i].internalFormat;
> +            *type = info[i].type;
> +            return true;
> +        }
> +    }
> +
> +    return false;
> +}
> +
>
>  /**
>   * Bisect the texture size along an axis.
> @@ -150,7 +183,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 +200,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;
> @@ -185,21 +220,19 @@ getActiveTextureLevelDescOES(Context &context,
> GLenum target, GLint level, Image
>          // OpenGL ES does not support 1D textures
>          return false;
>      }
> +    GLenum internalFormat, type;
>
> -    const GLint size[3] = {1, 1, 1};
> -    if (!probeTextureLevelSizeOES(target, level, size)) {
> +    if (!probeTextureFormatOES(target, level, &internalFormat, &type))
>          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 +243,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 +409,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 +446,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 +533,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 +1040,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 +1401,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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/apitrace/attachments/20150903/87b3cff4/attachment.html>


More information about the apitrace mailing list