On 07/24/2015 10:05 AM, Ilia Mirkin wrote:
On Fri, Jul 24, 2015 at 9:55 AM, Brian Paul <bri...@vmware.com> wrote:
The commit subject line doesn't seem to match the code.
It matches the code if you read the whole function... I think. Right
now if there's no image, it'll succeed, whereas after this change,
it'll return INVALID_OPERATION.
Hmm, both with and without your patch the piglit
arb_get_texture_sub_image-errors test passes. It tests getting of a
non-existent image.
Note that "no image" can be interpreted in two ways:
1. the gl_texture_object::Image[face][level] pointer is NULL
2. the gl_texture_object::Image[face][level] image exists but
width,height,depth are zero.
In my piglit test, the former case generates INVALID_OPERATION.
I'm adding some additional piglit tests to check the latter case. Your
patch does catch the case of width = 0 and xoffset+width>texwidth which
we didn't catch before.
I guess I'd simply name your patch "mesa: fix error checking for getting
zero-sized texture images".
-Brian
Happy to use a different subject line, let me know what you had in mind.
The code looks good though.
Reviewed-by: Brian Paul <bri...@vmware.com>
On 07/23/2015 06:40 PM, Ilia Mirkin wrote:
Commit 17f714836 (mesa: rearrange texture error checking order) moved
the width/height/depth == 0 allowance before checking if the image was
there. This was in part due to depth having to be == 1 for 2D images and
width having to be == 1 for 1D images. Instead relax the height/depth
checks to also accept 0 as valid.
With this change,
bin/arb_direct_state_access-get-textures
starts passing again.
Fixes: 17f714836 (mesa: rearrange texture error checking order)
Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu>
---
src/mesa/main/texgetimage.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
index cdbd618..c0ccce3 100644
--- a/src/mesa/main/texgetimage.c
+++ b/src/mesa/main/texgetimage.c
@@ -928,13 +928,6 @@ dimensions_error_check(struct gl_context *ctx,
const struct gl_texture_image *texImage;
int i;
- if (width == 0 || height == 0 || depth == 0) {
- /* Not an error, but nothing to do. Return 'true' so that the
- * caller simply returns.
- */
- return true;
- }
-
if (xoffset < 0) {
_mesa_error(ctx, GL_INVALID_VALUE, "%s(xoffset = %d)", caller,
xoffset);
return true;
@@ -973,7 +966,7 @@ dimensions_error_check(struct gl_context *ctx,
"%s(1D, yoffset = %d)", caller, yoffset);
return true;
}
- if (height != 1) {
+ if (height > 1) {
_mesa_error(ctx, GL_INVALID_VALUE,
"%s(1D, height = %d)", caller, height);
return true;
@@ -987,7 +980,7 @@ dimensions_error_check(struct gl_context *ctx,
"%s(zoffset = %d)", caller, zoffset);
return true;
}
- if (depth != 1) {
+ if (depth > 1) {
_mesa_error(ctx, GL_INVALID_VALUE,
"%s(depth = %d)", caller, depth);
return true;
@@ -1086,6 +1079,13 @@ dimensions_error_check(struct gl_context *ctx,
}
}
Reviewed-by: Brian Paul <bri...@vmware.com>
+ 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;
}
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
