[Mesa-dev] [PATCH 02/13] mesa: use integers in glReadPixels for GL_RGBA with integer types

[Brian Paul]

On 06/26/2012 05:22 PM, Jordan Justen wrote:
> On Tue, Jun 26, 2012 at 2:34 PM, Brian Paul<brianp at vmware.com>  wrote:
>> On 06/25/2012 06:34 PM, Jordan Justen wrote:
>>>
>>> Previously if a format of GL_RGBA was used, then float data would
>>> be returned, even if the type was an integer type.
>>>
>>> Signed-off-by: Jordan Justen<jordan.l.justen at intel.com>
>>> ---
>>>   src/mesa/main/readpix.c |    2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
>>> index 4058019..e3c4611 100644
>>> --- a/src/mesa/main/readpix.c
>>> +++ b/src/mesa/main/readpix.c
>>> @@ -337,7 +337,7 @@ slow_read_rgba_pixels( struct gl_context *ctx,
>>>         goto done;
>>>
>>>      for (j = 0; j<    height; j++) {
>>> -      if (_mesa_is_enum_format_integer(format)) {
>>> +      if (_mesa_is_integer_format_or_type(format, type)) {
>>>          _mesa_unpack_uint_rgba_row(rbFormat, width, map, (GLuint (*)[4])
>>> rgba);
>>>            _mesa_rebase_rgba_uint(width, (GLuint (*)[4]) rgba,
>>>                                   rb->_BaseFormat);
>>
>>
>> I'm not quite sure I follow this.
>>
>> If we call glReadPixels(format=GL_RGBA, type=GL_UNSIGNED_BYTE) it looks like
>> _mesa_is_integer_format_or_type(format, type) will evaluate to true and
>> we'll errantly take the integer path, rather than the float path.
>>
>> If one wants to read integer-valued colors, you need to pass
>> format=GL_RGBA_INTEGER.
>
> This fixed an issue with CopyTexImage and integer clamping.
>
> In meta, we use ReadPixels, and get the base format for the texture,
> which for rgb10_a2ui is GL_RGBA.
>
> Should rgb10_a2ui use GL_RGBA_INTEGER for it's base format?

In the glReadPixels call, yes, format=GL_RGBA_INTEGER.


> It doesn't look like any gl_format uses an integer base format.

That's intentional.  The base format basically indicates which color 
channels are present, not whether they're int, float, non-normalized, etc.


> Does
> this mean that CopyTexImage will always do a int to float conversion
> and then a float to int conversion if the src/dest is int based?

CopyTexImage should avoid that.


> Hmm, maybe the issue is that the float path is not clamping integers
> properly when re-packing.

The float path in what?  For glReadPixels, glTexImage, glCopyTexImage, 
etc., if the source and dest data are integer, the data should not be 
converted to float during the transfer.


> I'll investigate further, but any advise would be appreciated.
>
> Thanks for your time,
>
> -Jordan

-Brian