From 27fb522faac031e04bf980fdfad5a2243a4c4889 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 15 Apr 2011 12:58:17 -0700 Subject: [PATCH] mesa: Use _mesa_get_format_bytes to refactor out the RB get_pointer_* This is a squash of the following two commits: mesa: Use _mesa_get_format_bytes to refactor out the RB get_pointer_* Reviewed-by: Brian Paul (cherry picked from commit 6ab9889a2704304a45b4da5b28840af08f6f42c5) mesa: Fix return type of _mesa_get_format_bytes() (#37351) Despite that negative values aren't sensible here, making this unsigned is dangerous. Consider get_pointer_generic, which computes a value of the form: void *base + (int x * int stride + int y) * unsigned bpp The usual arithmetic conversions will coerce the (x*stride + y) subexpression to unsigned. Since stride can be negative, this is disastrous. Fixes at least the following piglit tests on Ironlake: fbo/fbo-blit-d24s8 spec/ARB_depth_texture/fbo-clear-formats spec/EXT_packed_depth_stencil/fbo-clear-formats NOTE: This is a candidate for the 7.10 branch. Reviewed-by: Chad Versace Signed-off-by: Adam Jackson (cherry picked from commit e8b1c6d6f55f5be3bef25084fdd8b6127517e137) --- src/mesa/main/formats.c | 4 +- src/mesa/main/formats.h | 2 +- src/mesa/main/renderbuffer.c | 81 ++++++------------------------------ 3 files changed, 17 insertions(+), 70 deletions(-) diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c index ce072e9f402..77a29c5ee0a 100644 --- a/src/mesa/main/formats.c +++ b/src/mesa/main/formats.c @@ -873,8 +873,10 @@ _mesa_get_format_name(gl_format format) * Return bytes needed to store a block of pixels in the given format. * Normally, a block is 1x1 (a single pixel). But for compressed formats * a block may be 4x4 or 8x4, etc. + * + * Note: not GLuint, so as not to coerce math to unsigned. cf. fdo #37351 */ -GLuint +GLint _mesa_get_format_bytes(gl_format format) { const struct gl_format_info *info = _mesa_get_format_info(format); diff --git a/src/mesa/main/formats.h b/src/mesa/main/formats.h index 997229bf9f4..3b92074022e 100644 --- a/src/mesa/main/formats.h +++ b/src/mesa/main/formats.h @@ -178,7 +178,7 @@ typedef enum extern const char * _mesa_get_format_name(gl_format format); -extern GLuint +extern GLint _mesa_get_format_bytes(gl_format format); extern GLint diff --git a/src/mesa/main/renderbuffer.c b/src/mesa/main/renderbuffer.c index 8c1b5d58769..e3d8cdc1db5 100644 --- a/src/mesa/main/renderbuffer.c +++ b/src/mesa/main/renderbuffer.c @@ -53,25 +53,22 @@ * Routines for get/put values in common buffer formats follow. */ +static void * +get_pointer_generic(struct gl_context *ctx, struct gl_renderbuffer *rb, + GLint x, GLint y) +{ + if (!rb->Data) + return NULL; + + return (rb->Data + + (y * rb->RowStride + x) * _mesa_get_format_bytes(rb->Format)); +} + /********************************************************************** * Functions for buffers of 1 X GLubyte values. * Typically stencil. */ -static void * -get_pointer_ubyte(struct gl_context *ctx, struct gl_renderbuffer *rb, - GLint x, GLint y) -{ - if (!rb->Data) - return NULL; - ASSERT(rb->DataType == GL_UNSIGNED_BYTE); - /* Can't assert rb->Format since these funcs may be used for serveral - * different formats (GL_ALPHA8, GL_STENCIL_INDEX8, etc). - */ - return (GLubyte *) rb->Data + y * rb->RowStride + x; -} - - static void get_row_ubyte(struct gl_context *ctx, struct gl_renderbuffer *rb, GLuint count, GLint x, GLint y, void *values) @@ -180,18 +177,6 @@ put_mono_values_ubyte(struct gl_context *ctx, struct gl_renderbuffer *rb, GLuint * Typically depth/Z. */ -static void * -get_pointer_ushort(struct gl_context *ctx, struct gl_renderbuffer *rb, - GLint x, GLint y) -{ - if (!rb->Data) - return NULL; - ASSERT(rb->DataType == GL_UNSIGNED_SHORT); - ASSERT(rb->Width > 0); - return (GLushort *) rb->Data + y * rb->RowStride + x; -} - - static void get_row_ushort(struct gl_context *ctx, struct gl_renderbuffer *rb, GLuint count, GLint x, GLint y, void *values) @@ -309,18 +294,6 @@ put_mono_values_ushort(struct gl_context *ctx, struct gl_renderbuffer *rb, * Typically depth/Z or color index. */ -static void * -get_pointer_uint(struct gl_context *ctx, struct gl_renderbuffer *rb, - GLint x, GLint y) -{ - if (!rb->Data) - return NULL; - ASSERT(rb->DataType == GL_UNSIGNED_INT || - rb->DataType == GL_UNSIGNED_INT_24_8_EXT); - return (GLuint *) rb->Data + y * rb->RowStride + x; -} - - static void get_row_uint(struct gl_context *ctx, struct gl_renderbuffer *rb, GLuint count, GLint x, GLint y, void *values) @@ -605,18 +578,6 @@ put_mono_values_ubyte3(struct gl_context *ctx, struct gl_renderbuffer *rb, * Typically color buffers. */ -static void * -get_pointer_ubyte4(struct gl_context *ctx, struct gl_renderbuffer *rb, - GLint x, GLint y) -{ - if (!rb->Data) - return NULL; - ASSERT(rb->DataType == GL_UNSIGNED_BYTE); - ASSERT(rb->Format == MESA_FORMAT_RGBA8888); - return (GLubyte *) rb->Data + 4 * (y * rb->RowStride + x); -} - - static void get_row_ubyte4(struct gl_context *ctx, struct gl_renderbuffer *rb, GLuint count, GLint x, GLint y, void *values) @@ -765,17 +726,6 @@ put_mono_values_ubyte4(struct gl_context *ctx, struct gl_renderbuffer *rb, * Typically accum buffer. */ -static void * -get_pointer_ushort4(struct gl_context *ctx, struct gl_renderbuffer *rb, - GLint x, GLint y) -{ - if (!rb->Data) - return NULL; - ASSERT(rb->DataType == GL_UNSIGNED_SHORT || rb->DataType == GL_SHORT); - return (GLushort *) rb->Data + 4 * (y * rb->RowStride + x); -} - - static void get_row_ushort4(struct gl_context *ctx, struct gl_renderbuffer *rb, GLuint count, GLint x, GLint y, void *values) @@ -934,6 +884,8 @@ put_mono_values_ushort4(struct gl_context *ctx, struct gl_renderbuffer *rb, void _mesa_set_renderbuffer_accessors(struct gl_renderbuffer *rb) { + rb->GetPointer = get_pointer_generic; + switch (rb->Format) { case MESA_FORMAT_RGB888: rb->DataType = GL_UNSIGNED_BYTE; @@ -949,7 +901,6 @@ _mesa_set_renderbuffer_accessors(struct gl_renderbuffer *rb) case MESA_FORMAT_RGBA8888: rb->DataType = GL_UNSIGNED_BYTE; - rb->GetPointer = get_pointer_ubyte4; rb->GetRow = get_row_ubyte4; rb->GetValues = get_values_ubyte4; rb->PutRow = put_row_ubyte4; @@ -961,7 +912,6 @@ _mesa_set_renderbuffer_accessors(struct gl_renderbuffer *rb) case MESA_FORMAT_SIGNED_RGBA_16: rb->DataType = GL_SHORT; - rb->GetPointer = get_pointer_ushort4; rb->GetRow = get_row_ushort4; rb->GetValues = get_values_ushort4; rb->PutRow = put_row_ushort4; @@ -974,7 +924,6 @@ _mesa_set_renderbuffer_accessors(struct gl_renderbuffer *rb) #if 0 case MESA_FORMAT_A8: rb->DataType = GL_UNSIGNED_BYTE; - rb->GetPointer = get_pointer_alpha8; rb->GetRow = get_row_alpha8; rb->GetValues = get_values_alpha8; rb->PutRow = put_row_alpha8; @@ -987,7 +936,6 @@ _mesa_set_renderbuffer_accessors(struct gl_renderbuffer *rb) case MESA_FORMAT_S8: rb->DataType = GL_UNSIGNED_BYTE; - rb->GetPointer = get_pointer_ubyte; rb->GetRow = get_row_ubyte; rb->GetValues = get_values_ubyte; rb->PutRow = put_row_ubyte; @@ -999,7 +947,6 @@ _mesa_set_renderbuffer_accessors(struct gl_renderbuffer *rb) case MESA_FORMAT_Z16: rb->DataType = GL_UNSIGNED_SHORT; - rb->GetPointer = get_pointer_ushort; rb->GetRow = get_row_ushort; rb->GetValues = get_values_ushort; rb->PutRow = put_row_ushort; @@ -1013,7 +960,6 @@ _mesa_set_renderbuffer_accessors(struct gl_renderbuffer *rb) case MESA_FORMAT_X8_Z24: case MESA_FORMAT_Z24_X8: rb->DataType = GL_UNSIGNED_INT; - rb->GetPointer = get_pointer_uint; rb->GetRow = get_row_uint; rb->GetValues = get_values_uint; rb->PutRow = put_row_uint; @@ -1026,7 +972,6 @@ _mesa_set_renderbuffer_accessors(struct gl_renderbuffer *rb) case MESA_FORMAT_Z24_S8: case MESA_FORMAT_S8_Z24: rb->DataType = GL_UNSIGNED_INT_24_8_EXT; - rb->GetPointer = get_pointer_uint; rb->GetRow = get_row_uint; rb->GetValues = get_values_uint; rb->PutRow = put_row_uint;