[Mesa-dev] [PATCH] mesa: set clamp bit in glGetTexImage for GL_UNSIGNED_NORMALIZED

[Brian Paul]

On Thu, Jan 26, 2012 at 4:55 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 01/26/2012 06:40 AM, Brian Paul wrote:
>>
>> On Tue, Jan 24, 2012 at 10:58 PM, Anuj Phogat<anuj.phogat at gmail.com>
>>  wrote:
>>>
>>> Color clamping should be enabled in glGetTexImage if texture dataType is
>>> GL_UNSIGNED_NORMALIZED and format is GL_LUMINANCE or GL_LUMINANCE_ALPHA
>>>
>>> Fixes 2 Intel oglconform test cases: pxconv-gettex and pxtrans-gettex
>>> https://bugs.freedesktop.org/show_bug.cgi?id=40864
>>>
>>> NOTE: This is a candidate for the 8.0 branch
>>>
>>> Signed-off-by: Anuj Phogat<anuj.phogat at gmail.com>
>>> ---
>>>  src/mesa/main/texgetimage.c |    7 ++++++-
>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
>>> index 8c85c1e..f89b868 100644
>>> --- a/src/mesa/main/texgetimage.c
>>> +++ b/src/mesa/main/texgetimage.c
>>> @@ -415,7 +415,12 @@ get_tex_rgba(struct gl_context *ctx, GLuint
>>> dimensions,
>>>          transferOps |= IMAGE_CLAMP_BIT;
>>>       }
>>>    }
>>> -
>>> +   else if ((format == GL_LUMINANCE ||
>>> +            format == GL_LUMINANCE_ALPHA)&&
>>> +            dataType == GL_UNSIGNED_NORMALIZED) {
>>> +      transferOps |= IMAGE_CLAMP_BIT;
>>> +   }
>>> +
>>>    if (_mesa_is_format_compressed(texImage->TexFormat)) {
>>>       get_tex_rgba_compressed(ctx, dimensions, format, type,
>>>                               pixels, texImage, transferOps);
>>> --
>>
>>
>> This looks OK.  However, we need to add more comments here.  I had to
>> do some digging to recall why luminance and lum/alpha are special.
>> The reason is if the source image is RGB, the returned luminance is
>> R+G+B and we need to clamp the sum to [0,1].
>
>
> When Anuj and I discussed this patch, I had another concern, but I'm not
> sure it matters.  If dataType is GL_SIGNED_NORMALIZED, it seems like the
> value should clamp to [-1,1].  We don't currently have a way to do that.
>  Does that seem right to you?

I would expect so too.  I'll put it on my to-do list, but it might be a while.

-Brian