[Mesa-dev] [PATCH 05/11] mesa: implement _mesa_GetTextureSubImage()

Laura Ekstrand laura at jlekstrand.net
Thu Dec 18 10:07:22 PST 2014


Brian,

I dealt with a lot of the TEXTURE_CUBE_MAP stuff when I implemented
GetTextureImage (in my recent DSA textures patch series).

Ian and I have filed a bug on the spec for missing faces and mismatched
formats/sizes. Right now, I am saying if (texObj->NumLayers < 6), throw
invalid operation (not enough storage).  Moreover, I discovered Monday
(12/15) that in the spec for GenerateMipmap there is a pre-existing idea of
"cube completeness."  In section 8.14.4 of GL 4.5 spec, it defines cube
completeness as having all the same formats/sizes on the faces.  So I am
checking if the texObj is cube complete and throwing invalid operation if
it is not.

So for now, I think invalid operation is the right plan.

I agree, this is an absolute mess.

Laura

On Thu, Dec 18, 2014 at 7:32 AM, Brian Paul <brianp at vmware.com> wrote:
>
> On 12/17/2014 05:03 PM, Laura Ekstrand wrote:
>
>>
>>
>> On Sat, Dec 13, 2014 at 6:42 AM, Brian Paul <brianp at vmware.com
>> <mailto:brianp at vmware.com>> wrote:
>>
>>     One of the two new functions in GL_ARB_get_texture_sub_image.
>>     ---
>>       src/mesa/main/texgetimage.c | 305
>>     ++++++++++++++++++++++++++++++++++++++------
>>       src/mesa/main/texgetimage.h |   8 ++
>>       2 files changed, 277 insertions(+), 36 deletions(-)
>>
>>     diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
>>     index 71c25bb..e1f238b 100644
>>     --- a/src/mesa/main/texgetimage.c
>>     +++ b/src/mesa/main/texgetimage.c
>>     @@ -44,6 +44,7 @@
>>       #include "texcompress.h"
>>       #include "texgetimage.h"
>>       #include "teximage.h"
>>     +#include "texobj.h"
>>       #include "texstore.h"
>>
>>
>>     @@ -803,41 +804,36 @@ legal_getteximage_target(struct gl_context
>>     *ctx, GLenum target)
>>
>> In my work with DSA textures, I have discovered that it's more robust to
>> pass in both the texture object and target.  I originally just passed in
>> the texObj instead of the target in error checks and used
>> texObj->Target, but I failed a number of Piglit tests afterwards.  This
>> is because, for non-DSA texture objects, the target passed into the
>> function and the texObj->Target don't always match (especially for proxy
>> targets).
>>
>>       /**
>>     - * Do error checking for a glGetTexImage() call.
>>     + * Do error checking for a glGetTexImage() or
>>     glGetTextureSubImage() call.
>>        * \return GL_TRUE if any error, GL_FALSE if no errors.
>>        */
>>       static GLboolean
>>     -getteximage_error_check(struct gl_context *ctx, GLenum target,
>>     GLint level,
>>     +getteximage_error_check(struct gl_context *ctx,
>>     +                        struct gl_texture_object *texObj, GLint
>> level,
>>                               GLenum format, GLenum type, GLsizei
>>     clientMemSize,
>>     -                        GLvoid *pixels )
>>     +                        GLvoid *pixels, const char *caller)
>>       {
>>     -   struct gl_texture_object *texObj;
>>     +   GLenum target = texObj->Target;
>>          struct gl_texture_image *texImage;
>>          const GLint maxLevels = _mesa_max_texture_levels(ctx, target);
>>     -   const GLuint dimensions = (target == GL_TEXTURE_3D) ? 3 : 2;
>>          GLenum baseFormat, err;
>>
>>     -   if (!legal_getteximage_target(ctx, target)) {
>>     -      _mesa_error(ctx, GL_INVALID_ENUM,
>>     "glGetTexImage(target=0x%x)", target);
>>     -      return GL_TRUE;
>>     -   }
>>     -
>>          assert(maxLevels != 0);
>>          if (level < 0 || level >= maxLevels) {
>>     -      _mesa_error( ctx, GL_INVALID_VALUE, "glGetTexImage(level)" );
>>     +      _mesa_error(ctx, GL_INVALID_VALUE, "%s(level)", caller);
>>             return GL_TRUE;
>>          }
>>
>>          err = _mesa_error_check_format_and_type(ctx, format, type);
>>          if (err != GL_NO_ERROR) {
>>     -      _mesa_error(ctx, err, "glGetTexImage(format/type)");
>>     +      _mesa_error(ctx, err, "%s(format/type)", caller);
>>             return GL_TRUE;
>>          }
>>
>>          texObj = _mesa_get_current_tex_object(ctx, target);
>>
>>          if (!texObj) {
>>     -      _mesa_error(ctx, GL_INVALID_ENUM, "glGetTexImage(target)");
>>     +      _mesa_error(ctx, GL_INVALID_ENUM, "%s(target)", caller);
>>             return GL_TRUE;
>>          }
>>
>>     @@ -854,64 +850,79 @@ getteximage_error_check(struct gl_context
>>     *ctx, GLenum target, GLint level,
>>           */
>>          if (_mesa_is_color_format(format)
>>              && !_mesa_is_color_format(baseFormat)) {
>>     -      _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(format
>>     mismatch)");
>>     +      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(format mismatch)",
>>     caller);
>>             return GL_TRUE;
>>          }
>>          else if (_mesa_is_depth_format(format)
>>                   && !_mesa_is_depth_format(baseFormat)
>>                   && !_mesa_is_depthstencil_format(baseFormat)) {
>>     -      _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(format
>>     mismatch)");
>>     +      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(format mismatch)",
>>     caller);
>>             return GL_TRUE;
>>          }
>>          else if (_mesa_is_stencil_format(format)
>>                   && !ctx->Extensions.ARB_texture_stencil8) {
>>     -      _mesa_error(ctx, GL_INVALID_ENUM,
>>     "glGetTexImage(format=GL_STENCIL_INDEX)");
>>     +      _mesa_error(ctx, GL_INVALID_ENUM,
>>     "%s(format=GL_STENCIL_INDEX)", caller);
>>             return GL_TRUE;
>>          }
>>          else if (_mesa_is_ycbcr_format(format)
>>                   && !_mesa_is_ycbcr_format(baseFormat)) {
>>     -      _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(format
>>     mismatch)");
>>     +      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(format mismatch)",
>>     caller);
>>             return GL_TRUE;
>>          }
>>          else if (_mesa_is_depthstencil_format(format)
>>                   && !_mesa_is_depthstencil_format(baseFormat)) {
>>     -      _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(format
>>     mismatch)");
>>     +      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(format mismatch)",
>>     caller);
>>             return GL_TRUE;
>>          }
>>          else if (_mesa_is_enum_format_integer(format) !=
>>                   _mesa_is_format_integer(texImage->TexFormat)) {
>>     -      _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(format
>>     mismatch)");
>>     +      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(format mismatch)",
>>     caller);
>>             return GL_TRUE;
>>          }
>>
>>     -   if (!_mesa_validate_pbo_access(dimensions, &ctx->Pack,
>>     texImage->Width,
>>     -                                  texImage->Height, texImage->Depth,
>>     -                                  format, type, clientMemSize,
>>     pixels)) {
>>     +   return GL_FALSE;
>>     +}
>>     +
>>     +
>>     +/**
>>     + * Do error checking related to the PBO and image size.
>>     + */
>>     +static bool
>>     +pbo_error_check(struct gl_context *ctx, GLenum target,
>>     +                GLenum format, GLenum type,
>>     +                GLsizei width, GLsizei height, GLsizei depth,
>>     +                GLsizei bufferSize, void *pixels,
>>     +                const char *caller)
>>     +{
>>     +   const GLuint dimensions = (target == GL_TEXTURE_3D) ? 3 : 2;
>>     +
>>     +   if (!_mesa_validate_pbo_access(dimensions, &ctx->Pack,
>>     +                                  width, height, depth,
>>     +                                  format, type, bufferSize, pixels))
>> {
>>             if (_mesa_is_bufferobj(ctx->Pack.BufferObj)) {
>>                _mesa_error(ctx, GL_INVALID_OPERATION,
>>     -                     "glGetTexImage(out of bounds PBO access)");
>>     +                     "%s(out of bounds PBO access)", caller);
>>             } else {
>>                _mesa_error(ctx, GL_INVALID_OPERATION,
>>     -                     "glGetnTexImageARB(out of bounds access:"
>>     -                     " bufSize (%d) is too small)", clientMemSize);
>>     +                     "%s(out of bounds access: bufSize (%d) is too
>>     small)",
>>     +                     caller, bufferSize);
>>             }
>>     -      return GL_TRUE;
>>     +      return true;
>>          }
>>
>>          if (_mesa_is_bufferobj(ctx->Pack.BufferObj)) {
>>             /* PBO should not be mapped */
>>             if (_mesa_check_disallowed_mapping(ctx->Pack.BufferObj)) {
>>                _mesa_error(ctx, GL_INVALID_OPERATION,
>>     -                     "glGetTexImage(PBO is mapped)");
>>     -         return GL_TRUE;
>>     +                     "%s(PBO is mapped)", caller);
>>     +         return true;
>>             }
>>          }
>>
>>     -   return GL_FALSE;
>>     +   return false;
>>       }
>>
>>
>>     -
>>       /**
>>        * Get texture image.  Called by glGetTexImage.
>>        *
>>     @@ -932,21 +943,36 @@ _mesa_GetnTexImageARB( GLenum target, GLint
>>     level, GLenum format,
>>
>>          FLUSH_VERTICES(ctx, 0);
>>
>>     -   if (getteximage_error_check(ctx, target, level, format, type,
>>     -                               bufSize, pixels)) {
>>     +   if (!legal_getteximage_target(ctx, target)) {
>>     +      _mesa_error(ctx, GL_INVALID_ENUM,
>>     "glGetTexImage(target=0x%x)", target);
>>             return;
>>          }
>>
>>     -   if (!_mesa_is_bufferobj(ctx->Pack.BufferObj) && !pixels) {
>>     -      /* not an error, do nothing */
>>     +   texObj = _mesa_get_current_tex_object(ctx, target);
>>     +
>>     +   if (getteximage_error_check(ctx, texObj, level, format, type,
>>     +                               bufSize, pixels, "glGet[n]TexImage"))
>> {
>>             return;
>>          }
>>
>>     -   texObj = _mesa_get_current_tex_object(ctx, target);
>>          texImage = _mesa_select_tex_image(ctx, texObj, target, level);
>>     +   if (!texImage) {
>>     +      return;  /* no texture image */
>>     +   }
>>
>>     -   if (_mesa_is_zero_size_texture(texImage))
>>     +   if (pbo_error_check(ctx, target, format, type,
>>     +                       texImage->Width, texImage->Height,
>>     texImage->Depth,
>>     +                       bufSize, pixels, "glGet[n]TexImage")) {
>>             return;
>>     +   }
>>     +
>>     +   if (_mesa_is_zero_size_texture(texImage)) {
>>     +      return;  /* nothing to get */
>>     +   }
>>     +
>>     +   if (!_mesa_is_bufferobj(ctx->Pack.BufferObj) && !pixels) {
>>     +      return;  /* not an error, do nothing */
>>     +   }
>>
>>          if (MESA_VERBOSE & (VERBOSE_API | VERBOSE_TEXTURE)) {
>>             _mesa_debug(ctx, "glGetTexImage(tex %u) format = %s, w=%d,
>>     h=%d,"
>>     @@ -1110,3 +1136,210 @@ _mesa_GetCompressedTexImage(GLenum target,
>>     GLint level, GLvoid *img)
>>       {
>>          _mesa_GetnCompressedTexImageARB(target, level, INT_MAX, img);
>>       }
>>
>>
>> Note:  When this gets merged with my DSA work, the target should maybe
>> get passed into this function.  This is because DSA and traditional
>> functions share a backend function, and that backend function will have
>> to resemble your GetTextureSubImage function (thus calling
>> dimensions_error_check).  As aforementioned, not passing the target
>> could cause trouble.
>>
>>     +
>>     +
>>     +
>>     +/**
>>     + * Error-check the offset and size arguments to
>>     + * glGet[Compressed]TextureSubImage().
>>     + * \return true if error, false if no error.
>>     + */
>>     +static bool
>>     +dimensions_error_check(struct gl_context *ctx,
>>     +                       struct gl_texture_image *texImage,
>>     +                       GLint xoffset, GLint yoffset, GLint zoffset,
>>     +                       GLsizei width, GLsizei height, GLsizei depth)
>>     +{
>>     +   const GLenum target = texImage->TexObject->Target;
>>     +
>>     +   switch (target) {
>>     +   case GL_TEXTURE_1D:
>>     +      if (yoffset != 0) {
>>     +         _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                     "glGetTextureSubImage(1D, yoffset = %d)\n",
>>     yoffset);
>>     +         return true;
>>     +      }
>>     +      if (height != 1) {
>>     +         _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                     "glGetTextureSubImage(1D, height = %d)\n",
>>     height);
>>     +         return true;
>>     +      }
>>     +      /* fall-through */
>>     +   case GL_TEXTURE_1D_ARRAY:
>>     +   case GL_TEXTURE_2D:
>>     +   case GL_TEXTURE_RECTANGLE:
>>     +      if (zoffset != 0) {
>>     +         _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                     "glGetTextureSubImage(zoffset = %d)\n",
>> zoffset);
>>     +         return true;
>>     +      }
>>     +      if (depth != 1) {
>>     +         _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                     "glGetTextureSubImage(depth = %d)\n", depth);
>>     +         return true;
>>     +      }
>>     +      break;
>>     +   default:
>>     +      ; /* nothing */
>>     +   }
>>     +
>>     +   if (xoffset < 0) {
>>     +      _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                  "glGetTextureSubImage(xoffset = %d)\n", xoffset);
>>     +      return true;
>>     +   }
>>     +
>>     +   if (yoffset < 0) {
>>     +      _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                  "glGetTextureSubImage(yoffset = %d)\n", yoffset);
>>     +      return true;
>>     +   }
>>     +
>>     +   if (zoffset < 0) {
>>     +      _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                  "glGetTextureSubImage(zoffset = %d)\n", zoffset);
>>     +      return true;
>>     +   }
>>     +
>>
>> Why didn't you put xoffset %d + width %d > image_width %u ?
>>
>
> IMHO, the error message has enough info to convey the problem.  If you
> feel strongly that this needs to be fixed, there's a bunch of similar
> instances (with more terse messages) in teximage.c that should be fixed as
> well.
>
>
>
>
>>     +   if (xoffset + width > texImage->Width) {
>>     +      _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                  "glGetTextureSubImage(xoffset %d + width %d >
>> %u)\n",
>>     +                  xoffset, width, texImage->Width);
>>     +      return true;
>>     +   }
>>     +
>>     +   if (yoffset + height > texImage->Height) {
>>     +      _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                  "glGetTextureSubImage(yoffset %d + height %d >
>>     %u)\n",
>>     +                  yoffset, height, texImage->Height);
>>     +      return true;
>>     +   }
>>     +
>>     +   if (zoffset + depth > texImage->Depth) {
>>     +      _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                  "glGetTextureSubImage(zoffset %d + depth %d >
>> %u)\n",
>>     +                  zoffset, depth, texImage->Depth);
>>     +      return true;
>>     +   }
>>     +
>>     +   /* Extra checks for compressed textures */
>>     +   {
>>     +      GLuint bw, bh;
>>     +      _mesa_get_format_block_size(texImage->TexFormat, &bw, &bh);
>>
>>     +      if (bw > 1 || bh > 1) {
>>     +         /* offset must be multiple of block size */
>>
>> This should be an INVALID_VALUE error according to the GL 4.5 spec.
>>
>
> Fixed in v2.
>
>
>
>>     +         if (xoffset % bw != 0) {
>>     +            _mesa_error(ctx, GL_INVALID_OPERATION,
>>     +                        "glGetTextureSubImage(xoffset = %d)",
>> xoffset);
>>     +            return true;
>>     +         }
>>     +         if (target != GL_TEXTURE_1D && target !=
>>     GL_TEXTURE_1D_ARRAY) {
>>
>> This should be an INVALID_VALUE error according to the GL 4.5 spec.
>>
>
> Fixed in v2.
>
>
>  Also, shouldn't it be yoffset % bh != 0 ?  You have bw.
>>
>
> Fixed in v2.
>
>
>
>>     +            if (yoffset % bw != 0) {
>>     +               _mesa_error(ctx, GL_INVALID_OPERATION,
>>     +                           "glGetTextureSubImage(yoffset = %d)",
>>     yoffset);
>>     +               return true;
>>     +            }
>>     +         }
>>
>> Why is there no similar zoffset check?  Is it because Mesa doesn't have
>> a concept of block depth?
>>
>
> Correct.
>
>
>
>>     +
>>     +         /* The size must be a multiple of bw x bh, or we must be
>>     using a
>>     +          * offset+size that exactly hits the edge of the image.
>>     +          */
>>
>> Note: xoffset + width != (GLint) texImage->Width is more permissive than
>> the spec document, which says (xoffset != 0) || (width != (GLint)
>> texImage->Width).
>>
>
> I believe there was discussion about this a few years ago and Mesa's
> behavior matches nvidia's driver.  The same logic is used in the glTexImage
> code.  It allows sub-regions of compressed textures to be read/written on
> block boundaries.
>
>
>  Again, the spec document says INVALID_VALUE.
>>
>
> Fixed.
>
>
>
>>     +         if ((width % bw != 0) &&
>>     +             (xoffset + width != (GLint) texImage->Width)) {
>>     +            _mesa_error(ctx, GL_INVALID_OPERATION,
>>     +                        "glGetTextureSubImage(width = %d)", width);
>>     +            return true;
>>     +         }
>>     +
>>
>> Again, the spec document says INVALID_VALUE.
>>
>
> Fixed.
>
>
>
>>     +         if ((height % bh != 0) &&
>>     +             (yoffset + height != (GLint) texImage->Height)) {
>>     +            _mesa_error(ctx, GL_INVALID_OPERATION,
>>     +                        "glGetTextureSubImage(height = %d)", height);
>>     +            return true;
>>     +         }
>>     +      }
>>     +   }
>>
>> No depth check?
>>
>
> No.  We have no 3D compression formats.
>
>
>
>>     +
>>     +   if (width == 0 || height == 0 || depth == 0) {
>>     +      /* Not an error, but nothing to do.  Return 'true' so that the
>>     +       * caller simply returns.
>>     +       */
>>     +      return true;
>>     +   }
>>     +
>>     +   return false;
>>     +}
>>     +
>>     +
>>     +
>>     +void GLAPIENTRY
>>     +_mesa_GetTextureSubImage(GLuint texture, GLint level,
>>     +                         GLint xoffset, GLint yoffset, GLint zoffset,
>>     +                         GLsizei width, GLsizei height, GLsizei
>> depth,
>>     +                         GLenum format, GLenum type, GLsizei bufSize,
>>     +                         void *pixels)
>>     +{
>>     +   struct gl_texture_object *texObj;
>>     +   struct gl_texture_image *texImage;
>>     +   GLenum target;
>>     +
>>     +   GET_CURRENT_CONTEXT(ctx);
>>     +
>>     +   texObj = _mesa_lookup_texture(ctx, texture);
>>     +   if (!texObj) {
>>
>>
>> I think this error might be a mistake in the spec.  According to
>> https://www.opengl.org/registry/specs/ARB/get_texture_sub_image.txt
>> <https://urldefense.proofpoint.com/v2/url?u=https-
>> 3A__www.opengl.org_registry_specs_ARB_get-5Ftexture-5Fsub-
>> 5Fimage.txt&d=AwMFaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=
>> T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=WWWcDPCoYwfOyILpH0kA7-k-
>> qpSPTsIs1qycAmB5GNA&s=ZhWz0mRK6xg9KPHqEvdXdXdSYxLyX81SGDpO0VoV-nw&e=>,
>> it should be INVALID_OPERATION in the case where the user passed in an
>> unknown texture name.  Oddly enough, the OpenGL 4.5 core spec
>> (30.10.2014) has INVALID_VALUE (consistent with what you have here) in
>> section 8.11 Texture Queries.  But that is very strange because all the
>> DSA functions consistently throw INVALID_OPERATION for this error (such
>> as GetTextureImage and TextureSubImage*D, etc.) in the OpenGL 4.5 spec.
>> I'm not sure why they wouldn't agree.
>>
>> In my DSA patch series, I have a convenience function for throwing
>> INVALID_OPERATION for an unknown texture name because this occurs
>> absolutely everywhere (_mesa_lookup_texture_err).
>>
>
> Actually, a later patch in the series changed this to INVALID_OPERATION
> (and fixed the error string).  I'll move that bit into this patch.
>
> INVALID_OPERATION seems to be the right thing.
>
>
>
>>     +      _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                  "glGetTextureSubImage(invalid texture ID %u\n",
>>     texture);
>>     +      return;
>>     +   }
>>     +
>>     +   /* common error checking */
>>     +   if (getteximage_error_check(ctx, texObj, level, format, type,
>>     bufSize,
>>     +                               pixels, "glGetTextureSubImage")) {
>>     +      return;
>>     +   }
>>     +
>>
>>
>> You forgot to call legal_getteximage_target.
>>
>
> I don't think that's needed since it's not possible for texObj->Target to
> be an invalid value at this point.
>
>
>
>>     +   target = texObj->Target;
>>
>>
>> This doesn't seem like it will return more than one face of the cube
>> map.  In fact, it will throw INVALID_OPERATION if the user tries to do
>> so (depth != 1).  But the GL 4.5 spec says the user should be able to
>> get as many faces as he or she wants from GetTextureSubImage (up to all
>> six).
>>
>
> Ugh, something got lost from my early versions of the
> GL_ARB_get_texture_sub_image spec to what's in the final spec, and the 4.5
> spec.
>
> The problem is OpenGL doesn't disallow specifying different dimensions for
> each of the cube map faces.  For example, it's perfectly legal to
> create a cube map texture like this:
>
> glTexImage2D(POSITIVE_X, width=32, height=32)
> glTexImage2D(NEGATIVE_X, width=16, height=16)
> glTexImage2D(POSITIVE_Y, width=64, height=64)
> etc.
>
> You can even have different internal formats per face!
>
> The texture will be flagged as 'incomplete' and you can't render with it.
> But you can create such a texture and you can query the face dimensions
> with glGetTexLevelParameter() and get back the per-face sizes.  Plus,
> glGetTexImage operates per-face.  (I should probably whip up a piglit test
> to verify this).
>
> So, what should happen with glGetTextureSubImage() if we try to get two or
> more mismatched-size faces at once?  I can't find any spec language about
> this.  Maybe just raise GL_INVALID_OPERATION??
>
> My first few drafts of the GL_ARB_get_texture_sub_image spec had a target
> parameter which only allowed GL_TEXTURE_CUBE_MAP_POSITIVE/NEGATIVE_X/Y/Z
> so that this situation would be handled consistently.
>
> I didn't see that this got changed in the spec so I implemented what I
> originally intended (only allow getting one face/slice at a time).
>
>
> BTW, it would be nice in Mesa if we could treat GL_TEXTURE_CUBE_MAP
> textures more like GL_TEXTURE_CUBE_MAP_ARRAY textures.  That is, the faces
> would just be treated as slices in a depth=6 array.
>
> Ideally, gl_texture_object::Image[face][level] would become
> gl_texture_object::Image[level].  But we can't really do that since
> according to the spec, we need to keep track of possibly different
> dimensions and formats for each face.
>
> This is a mess.  What do you think about the GL_INVALID_OPERATION idea?
>
>
> In any case, I'm on vacation after Friday so I probably won't have time to
> go further on this until the new year.
>
> -Brian
>
>
>
>  In my GetTextureImage patch, I just call the backend
>> _mesa_get_texture_image multiple times.  When they are merged, the
>> backend will be _mesa_get_texture_sub_image and GetTextureSubImage can
>> call that multiple times in the same way.
>>
>>     +   if (target == GL_TEXTURE_CUBE_MAP) {
>>     +      /* convert z to face/target */
>>     +      if (zoffset >= 6) {
>>     +         _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                     "glGetTextureSubImage(cube, zoffset = %d\n",
>>     zoffset);
>>     +         return;
>>     +      }
>>     +      if (depth != 1) {
>>     +         _mesa_error(ctx, GL_INVALID_VALUE,
>>     +                     "glGetTextureSubImage(cube, depth = %d\n",
>> depth);
>>     +         return;
>>     +      }
>>     +      target = GL_TEXTURE_CUBE_MAP_POSITIVE_X + zoffset;
>>     +   }
>>     +
>>     +   texImage = _mesa_select_tex_image(ctx, texObj, target, level);
>>     +
>>     +   /* check dimensions */
>>     +   if (dimensions_error_check(ctx, texImage, xoffset, yoffset,
>> zoffset,
>>     +                              width, height, depth)) {
>>     +      return;
>>     +   }
>>     +
>>     +   if (pbo_error_check(ctx, target, format, type,
>>     +                       width, height, depth,
>>     +                       bufSize, pixels, "glGetTextureSubImage")) {
>>     +      return;
>>     +   }
>>     +
>>     +   if (_mesa_is_zero_size_texture(texImage)) {
>>     +      return;  /* nothing to get */
>>     +   }
>>     +
>>     +   if (!_mesa_is_bufferobj(ctx->Pack.BufferObj) && pixels == NULL) {
>>     +      return;  /* not an error, do nothing */
>>     +   }
>>     +
>>     +   _mesa_lock_texture(ctx, texObj);
>>     +   {
>>     +      ctx->Driver.GetTexSubImage(ctx, xoffset, yoffset, zoffset,
>>     width, height,
>>     +                                 depth, format, type, pixels,
>>     texImage);
>>     +   }
>>     +   _mesa_unlock_texture(ctx, texObj);
>>     +}
>>     diff --git a/src/mesa/main/texgetimage.h b/src/mesa/main/texgetimage.h
>>     index c9af5b9..40f152c 100644
>>     --- a/src/mesa/main/texgetimage.h
>>     +++ b/src/mesa/main/texgetimage.h
>>     @@ -65,4 +65,12 @@ extern void GLAPIENTRY
>>       _mesa_GetnCompressedTexImageARB(GLenum target, GLint level,
>>     GLsizei bufSize,
>>                                       GLvoid *img);
>>
>>     +extern void GLAPIENTRY
>>     +_mesa_GetTextureSubImage(GLuint texture, GLint level,
>>     +                         GLint xoffset, GLint yoffset, GLint zoffset,
>>     +                         GLsizei width, GLsizei height, GLsizei
>> depth,
>>     +                         GLenum format, GLenum type, GLsizei bufSize,
>>     +                         void *pixels);
>>     +
>>     +
>>       #endif /* TEXGETIMAGE_H */
>>     --
>>     1.9.1
>>
>>     _______________________________________________
>>     mesa-dev mailing list
>>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org
>> >
>>     http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>     <https://urldefense.proofpoint.com/v2/url?u=http-
>> 3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AwMFaQ&c=
>> Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=
>> T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=WWWcDPCoYwfOyILpH0kA7-k-
>> qpSPTsIs1qycAmB5GNA&s=GrUeebJuM5x75DOASs1ezQdYEv5z61wojf4LHxr7Rvc&e=>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141218/3bafef0e/attachment-0001.html>


More information about the mesa-dev mailing list