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

2012-01-26 Thread Brian Paul 
On Tue, Jan 24, 2012 at 10:58 PM, Anuj Phogat anuj.pho...@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.pho...@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].

Would you please add a comment about that?  Thanks.

-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2012-01-26 Thread Ian Romanick 

On 01/26/2012 06:40 AM, Brian Paul wrote:

On Tue, Jan 24, 2012 at 10:58 PM, Anuj Phogatanuj.pho...@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 Phogatanuj.pho...@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?



Would you please add a comment about that?  Thanks.


Good idea.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2012-01-26 Thread Brian Paul 
On Thu, Jan 26, 2012 at 4:55 PM, Ian Romanick i...@freedesktop.org wrote:
 On 01/26/2012 06:40 AM, Brian Paul wrote:

 On Tue, Jan 24, 2012 at 10:58 PM, Anuj Phogatanuj.pho...@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 Phogatanuj.pho...@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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2012-01-26 Thread Anuj Phogat 
On Thu, Jan 26, 2012 at 2:56 PM, Brian Paul brian.e.p...@gmail.com wrote:

 On Thu, Jan 26, 2012 at 4:55 PM, Ian Romanick i...@freedesktop.org wrote:
  On 01/26/2012 06:40 AM, Brian Paul wrote:
 
  On Tue, Jan 24, 2012 at 10:58 PM, Anuj Phogatanuj.pho...@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 Phogatanuj.pho...@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.

I can add clamp [-1, 1] . I'll send out a separate patch for that.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2012-01-24 Thread Anuj Phogat 
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.pho...@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);
-- 
1.7.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev