[Mesa-dev] [PATCH 02/14] mesa: add pixel_storei() helper

[Timothy Arceri]

On 28/06/17 18:29, Samuel Pitoiset wrote:
> 
> 
> On 06/28/2017 08:13 AM, Timothy Arceri wrote:
>> Will be used to add KHR_no_error support.
>> ---
>>   src/mesa/main/pixelstore.c | 103 
>> +++++++++++++++++++++++++--------------------
>>   1 file changed, 57 insertions(+), 46 deletions(-)
>>
>> diff --git a/src/mesa/main/pixelstore.c b/src/mesa/main/pixelstore.c
>> index fc81533..841ff48 100644
>> --- a/src/mesa/main/pixelstore.c
>> +++ b/src/mesa/main/pixelstore.c
>> @@ -35,177 +35,181 @@
>>   #include "mtypes.h"
>> -void GLAPIENTRY
>> -_mesa_PixelStorei( GLenum pname, GLint param )
>> +static ALWAYS_INLINE void
>> +pixel_storei(GLenum pname, GLint param, bool check_error)
> 
> Why check_error? We usually call this no_error, can you rename (and 
> invert the conditions) please?

Just because there is no instance where this no_error will not need to 
be !no_error. But I will change for consistency.

> 
>>   {
>>      /* NOTE: this call can't be compiled into the display list */
>>      GET_CURRENT_CONTEXT(ctx);
>>      switch (pname) {
>>         case GL_PACK_SWAP_BYTES:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>>            ctx->Pack.SwapBytes = param ? GL_TRUE : GL_FALSE;
>>            break;
>>         case GL_PACK_LSB_FIRST:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>>            ctx->Pack.LsbFirst = param ? GL_TRUE : GL_FALSE;
>>            break;
>>         case GL_PACK_ROW_LENGTH:
>> -         if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles3(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx) && 
>> !_mesa_is_gles3(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Pack.RowLength = param;
>>            break;
>>         case GL_PACK_IMAGE_HEIGHT:
>> -         if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles3(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx) && 
>> !_mesa_is_gles3(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Pack.ImageHeight = param;
>>            break;
>>         case GL_PACK_SKIP_PIXELS:
>> -         if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles3(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx) && 
>> !_mesa_is_gles3(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Pack.SkipPixels = param;
>>            break;
>>         case GL_PACK_SKIP_ROWS:
>> -         if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles3(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx) && 
>> !_mesa_is_gles3(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Pack.SkipRows = param;
>>            break;
>>         case GL_PACK_SKIP_IMAGES:
>> -         if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles3(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx) && 
>> !_mesa_is_gles3(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Pack.SkipImages = param;
>>            break;
>>         case GL_PACK_ALIGNMENT:
>> -         if (param!=1 && param!=2 && param!=4 && param!=8)
>> +         if (check_error && param!=1 && param!=2 && param!=4 && 
>> param!=8)
>>               goto invalid_value_error;
>>            ctx->Pack.Alignment = param;
>>            break;
>>         case GL_PACK_INVERT_MESA:
>> -         if (!_mesa_is_desktop_gl(ctx) || 
>> !ctx->Extensions.MESA_pack_invert)
>> +         if (check_error &&
>> +             (!_mesa_is_desktop_gl(ctx) || 
>> !ctx->Extensions.MESA_pack_invert))
>>               goto invalid_enum_error;
>>            ctx->Pack.Invert = param;
>>            break;
>>         case GL_PACK_COMPRESSED_BLOCK_WIDTH:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Pack.CompressedBlockWidth = param;
>>            break;
>>         case GL_PACK_COMPRESSED_BLOCK_HEIGHT:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Pack.CompressedBlockHeight = param;
>>            break;
>>         case GL_PACK_COMPRESSED_BLOCK_DEPTH:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Pack.CompressedBlockDepth = param;
>>            break;
>>         case GL_PACK_COMPRESSED_BLOCK_SIZE:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Pack.CompressedBlockSize = param;
>>            break;
>>         case GL_UNPACK_SWAP_BYTES:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>>            ctx->Unpack.SwapBytes = param ? GL_TRUE : GL_FALSE;
>>            break;
>>         case GL_UNPACK_LSB_FIRST:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>>            ctx->Unpack.LsbFirst = param ? GL_TRUE : GL_FALSE;
>>            break;
>>         case GL_UNPACK_ROW_LENGTH:
>> -         if (ctx->API == API_OPENGLES)
>> +         if (check_error && ctx->API == API_OPENGLES)
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Unpack.RowLength = param;
>>            break;
>>         case GL_UNPACK_IMAGE_HEIGHT:
>> -         if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles3(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx) && 
>> !_mesa_is_gles3(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Unpack.ImageHeight = param;
>>            break;
>>         case GL_UNPACK_SKIP_PIXELS:
>> -         if (ctx->API == API_OPENGLES)
>> +         if (check_error && ctx->API == API_OPENGLES)
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Unpack.SkipPixels = param;
>>            break;
>>         case GL_UNPACK_SKIP_ROWS:
>> -         if (ctx->API == API_OPENGLES)
>> +         if (check_error && ctx->API == API_OPENGLES)
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Unpack.SkipRows = param;
>>            break;
>>         case GL_UNPACK_SKIP_IMAGES:
>> -         if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles3(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx) && 
>> !_mesa_is_gles3(ctx))
>>               goto invalid_enum_error;
>> -         if (param < 0)
>> +         if (check_error && param < 0)
>>               goto invalid_value_error;
>>            ctx->Unpack.SkipImages = param;
>>            break;
>>         case GL_UNPACK_ALIGNMENT:
>> -         if (param!=1 && param!=2 && param!=4 && param!=8)
>> +         if (check_error && param!=1 && param!=2 && param!=4 && 
>> param!=8)
>>               goto invalid_value_error;
>>            ctx->Unpack.Alignment = param;
>>            break;
>>         case GL_UNPACK_COMPRESSED_BLOCK_WIDTH:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Unpack.CompressedBlockWidth = param;
>>            break;
>>         case GL_UNPACK_COMPRESSED_BLOCK_HEIGHT:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Unpack.CompressedBlockHeight = param;
>>            break;
>>         case GL_UNPACK_COMPRESSED_BLOCK_DEPTH:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Unpack.CompressedBlockDepth = param;
>>            break;
>>         case GL_UNPACK_COMPRESSED_BLOCK_SIZE:
>> -         if (!_mesa_is_desktop_gl(ctx))
>> +         if (check_error && !_mesa_is_desktop_gl(ctx))
>>               goto invalid_enum_error;
>> -         if (param<0)
>> +         if (check_error && param<0)
>>               goto invalid_value_error;
>>            ctx->Unpack.CompressedBlockSize = param;
>>            break;
>>         default:
>> -         goto invalid_enum_error;
>> +         if (check_error)
>> +            goto invalid_enum_error;
>> +         else
>> +            unreachable("invalid pixel store enum");
>>      }
>>      return;
>> @@ -221,6 +225,13 @@ invalid_value_error:
>>   void GLAPIENTRY
>> +_mesa_PixelStorei( GLenum pname, GLint param )
> 
> While you are it, can you remove the extra spaces around the braces?
> 
> Samuel.
> 
>> +{
>> +   pixel_storei(pname, param, true);
>> +}
>> +
>> +
>> +void GLAPIENTRY
>>   _mesa_PixelStoref( GLenum pname, GLfloat param )
>>   {
>>      _mesa_PixelStorei( pname, IROUND(param) );
>>