[Mesa-dev] [PATCH 05/11] mesa: implement _mesa_GetTextureSubImage()
Laura Ekstrand
laura at jlekstrand.net
Thu Dec 18 11:47:52 PST 2014
After sending this review, I think it would be best (to avoid a gigantic
merge snarl) for me to send another patch where I manually merge this with
my patch "[PATCH 29/41] main: Added entry point for glGetTextureImage."
Laura
On Thu, Dec 18, 2014 at 10:07 AM, Laura Ekstrand <laura at jlekstrand.net>
wrote:
>
> 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/d2adb4a4/attachment-0001.html>
More information about the mesa-dev
mailing list