Eric Anholt | 27 Jan 08:59 2012
Picon

Re: [PATCH] mesa: set clamp bit in glGetTexImage for GL_UNSIGNED_NORMALIZED

On Thu, 26 Jan 2012 13:55:37 -0800, 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
> >>  <at>  <at>  -415,7 +415,12  <at>  <at>  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?

From testing I've done before, we've got plenty of snorm issues.  I
wouldn't worry about it for now.
_______________________________________________
mesa-dev mailing list
mesa-dev <at> lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Gmane