On Tue, 2015-06-16 at 11:15 -0700, Anuj Phogat wrote:
> This utility function is used in a later patch.
>
> Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com>
> Cc: <mesa-sta...@lists.freedesktop.org>
> ---
> src/mesa/main/readpix.c | 32 ++++++++++++++++++--------------
> src/mesa/main/readpix.h | 4 ++++
> 2 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> index a9416ef..1038983 100644
> --- a/src/mesa/main/readpix.c
> +++ b/src/mesa/main/readpix.c
> @@ -114,6 +114,22 @@ _mesa_get_readpixels_transfer_ops(const struct
> gl_context *ctx,
> return transferOps;
> }
>
> +bool
> +_mesa_need_signed_unsigned_int_conversion(mesa_format rbFormat,
> + GLenum format, GLenum type)
> +{
> + const GLenum srcType = _mesa_get_format_datatype(rbFormat);
> + return (srcType == GL_INT &&
> + _mesa_is_enum_format_integer(format) &&
> + (type == GL_UNSIGNED_INT ||
> + type == GL_UNSIGNED_SHORT ||
> + type == GL_UNSIGNED_BYTE)) ||
> + (srcType == GL_UNSIGNED_INT &&
> + _mesa_is_enum_format_integer(format) &&
> + (type == GL_INT ||
> + type == GL_SHORT ||
> + type == GL_BYTE));
> +}
I think it is better if you assign the result of
_mesa_is_enum_format_integer(format) to a temporary instead of calling
it twice in the condition, just like you do with srcType.
> /**
> * Return true if memcpy cannot be used for ReadPixels.
> @@ -130,7 +146,6 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
> *ctx, GLenum format,
> {
> struct gl_renderbuffer *rb =
> _mesa_get_read_renderbuffer_for_format(ctx, format);
> - GLenum srcType;
>
> assert(rb);
>
> @@ -157,20 +172,9 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
> *ctx, GLenum format,
>
> /* Conversion between signed and unsigned integers needs masking
> * (it isn't just memcpy). */
> - srcType = _mesa_get_format_datatype(rb->Format);
> -
> - if ((srcType == GL_INT &&
> - _mesa_is_enum_format_integer(format) &&
> - (type == GL_UNSIGNED_INT ||
> - type == GL_UNSIGNED_SHORT ||
> - type == GL_UNSIGNED_BYTE)) ||
> - (srcType == GL_UNSIGNED_INT &&
> - _mesa_is_enum_format_integer(format) &&
> - (type == GL_INT ||
> - type == GL_SHORT ||
> - type == GL_BYTE))) {
> + if (_mesa_need_signed_unsigned_int_conversion(rb->Format, format,
> + type))
> return GL_TRUE;
> - }
You need to rebase your patch, this code does not exist any more. I
moved it to Gallium where I am not sure that your change is what they
want. You should probably just skip this part.
With these changes,
Reviewed-by: Iago Toral Quiroga <ito...@igalia.com>
> /* And finally, see if there are any transfer ops. */
> return _mesa_get_readpixels_transfer_ops(ctx, rb->Format, format, type,
> diff --git a/src/mesa/main/readpix.h b/src/mesa/main/readpix.h
> index f894036..a93e263 100644
> --- a/src/mesa/main/readpix.h
> +++ b/src/mesa/main/readpix.h
> @@ -46,6 +46,10 @@ _mesa_get_readpixels_transfer_ops(const struct gl_context
> *ctx,
> GLenum format, GLenum type,
> GLboolean uses_blit);
>
> +extern bool
> +_mesa_need_signed_unsigned_int_conversion(mesa_format rbFormat,
> + GLenum format, GLenum type);
> +
> extern void
> _mesa_readpixels(struct gl_context *ctx,
> GLint x, GLint y, GLsizei width, GLsizei height,
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
