[Mesa-dev] [PATCH] i965: Add comment to explain the weird-looking shadow compares.

2014-04-12 Thread Chris Forbes 
This always looks crazy when I stumble across it, until I remember
what the hardware is doing. Describing it ought to short-circuit
that process next time :)

Signed-off-by: Chris Forbes chr...@ijw.co.nz
---
 src/mesa/drivers/dri/i965/intel_state.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_state.c 
b/src/mesa/drivers/dri/i965/intel_state.c
index bfc35d8..1597db6 100644
--- a/src/mesa/drivers/dri/i965/intel_state.c
+++ b/src/mesa/drivers/dri/i965/intel_state.c
@@ -40,6 +40,17 @@
 int
 intel_translate_shadow_compare_func(GLenum func)
 {
+   /* GL specifies the result of shadow comparisons as:
+* 1 if   ref op texel,
+* 0 otherwise.
+*
+* The hardware does:
+* 0 if texel op ref,
+* 1 otherwise.
+*
+* So, these look a bit strange because there's both a negation
+* and swapping of the arguments involved.
+*/
switch (func) {
case GL_NEVER:
return BRW_COMPAREFUNCTION_ALWAYS;
@@ -50,11 +61,11 @@ intel_translate_shadow_compare_func(GLenum func)
case GL_GREATER:
return BRW_COMPAREFUNCTION_GEQUAL;
case GL_GEQUAL:
-  return BRW_COMPAREFUNCTION_GREATER;
+   return BRW_COMPAREFUNCTION_GREATER;
case GL_NOTEQUAL:
-  return BRW_COMPAREFUNCTION_EQUAL;
+   return BRW_COMPAREFUNCTION_EQUAL;
case GL_EQUAL:
-  return BRW_COMPAREFUNCTION_NOTEQUAL;
+   return BRW_COMPAREFUNCTION_NOTEQUAL;
case GL_ALWAYS:
return BRW_COMPAREFUNCTION_NEVER;
}
-- 
1.9.2

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

Re: [Mesa-dev] [PATCH] i965: Add comment to explain the weird-looking shadow compares.

2014-04-12 Thread Kenneth Graunke 
On 04/12/2014 03:34 AM, Chris Forbes wrote:
 This always looks crazy when I stumble across it, until I remember
 what the hardware is doing. Describing it ought to short-circuit
 that process next time :)
 
 Signed-off-by: Chris Forbes chr...@ijw.co.nz
 ---
  src/mesa/drivers/dri/i965/intel_state.c | 17 ++---
  1 file changed, 14 insertions(+), 3 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i965/intel_state.c 
 b/src/mesa/drivers/dri/i965/intel_state.c
 index bfc35d8..1597db6 100644
 --- a/src/mesa/drivers/dri/i965/intel_state.c
 +++ b/src/mesa/drivers/dri/i965/intel_state.c
 @@ -40,6 +40,17 @@
  int
  intel_translate_shadow_compare_func(GLenum func)
  {
 +   /* GL specifies the result of shadow comparisons as:
 +* 1 if   ref op texel,
 +* 0 otherwise.
 +*
 +* The hardware does:
 +* 0 if texel op ref,
 +* 1 otherwise.
 +*
 +* So, these look a bit strange because there's both a negation
 +* and swapping of the arguments involved.
 +*/

Thanks for adding this!  I've had to do this a few times as well.

Reviewed-by: Kenneth Graunke kenn...@whitecape.org

 switch (func) {
 case GL_NEVER:
 return BRW_COMPAREFUNCTION_ALWAYS;
 @@ -50,11 +61,11 @@ intel_translate_shadow_compare_func(GLenum func)
 case GL_GREATER:
 return BRW_COMPAREFUNCTION_GEQUAL;
 case GL_GEQUAL:
 -  return BRW_COMPAREFUNCTION_GREATER;
 +   return BRW_COMPAREFUNCTION_GREATER;

Although your patch makes the 6-7 space indent consistent...we really
want to use 2*3=6, not 7. :)  If you wouldn't mind switching that before
pushing...

 case GL_NOTEQUAL:
 -  return BRW_COMPAREFUNCTION_EQUAL;
 +   return BRW_COMPAREFUNCTION_EQUAL;
 case GL_EQUAL:
 -  return BRW_COMPAREFUNCTION_NOTEQUAL;
 +   return BRW_COMPAREFUNCTION_NOTEQUAL;
 case GL_ALWAYS:
 return BRW_COMPAREFUNCTION_NEVER;
 }
 




signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Add comment to explain the weird-looking shadow compares.

2014-04-12 Thread Chris Forbes 
Eek -- yes, I'll fix that!

On Sun, Apr 13, 2014 at 8:04 AM, Kenneth Graunke kenn...@whitecape.org wrote:
 On 04/12/2014 03:34 AM, Chris Forbes wrote:
 This always looks crazy when I stumble across it, until I remember
 what the hardware is doing. Describing it ought to short-circuit
 that process next time :)

 Signed-off-by: Chris Forbes chr...@ijw.co.nz
 ---
  src/mesa/drivers/dri/i965/intel_state.c | 17 ++---
  1 file changed, 14 insertions(+), 3 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/intel_state.c 
 b/src/mesa/drivers/dri/i965/intel_state.c
 index bfc35d8..1597db6 100644
 --- a/src/mesa/drivers/dri/i965/intel_state.c
 +++ b/src/mesa/drivers/dri/i965/intel_state.c
 @@ -40,6 +40,17 @@
  int
  intel_translate_shadow_compare_func(GLenum func)
  {
 +   /* GL specifies the result of shadow comparisons as:
 +* 1 if   ref op texel,
 +* 0 otherwise.
 +*
 +* The hardware does:
 +* 0 if texel op ref,
 +* 1 otherwise.
 +*
 +* So, these look a bit strange because there's both a negation
 +* and swapping of the arguments involved.
 +*/

 Thanks for adding this!  I've had to do this a few times as well.

 Reviewed-by: Kenneth Graunke kenn...@whitecape.org

 switch (func) {
 case GL_NEVER:
 return BRW_COMPAREFUNCTION_ALWAYS;
 @@ -50,11 +61,11 @@ intel_translate_shadow_compare_func(GLenum func)
 case GL_GREATER:
 return BRW_COMPAREFUNCTION_GEQUAL;
 case GL_GEQUAL:
 -  return BRW_COMPAREFUNCTION_GREATER;
 +   return BRW_COMPAREFUNCTION_GREATER;

 Although your patch makes the 6-7 space indent consistent...we really
 want to use 2*3=6, not 7. :)  If you wouldn't mind switching that before
 pushing...

 case GL_NOTEQUAL:
 -  return BRW_COMPAREFUNCTION_EQUAL;
 +   return BRW_COMPAREFUNCTION_EQUAL;
 case GL_EQUAL:
 -  return BRW_COMPAREFUNCTION_NOTEQUAL;
 +   return BRW_COMPAREFUNCTION_NOTEQUAL;
 case GL_ALWAYS:
 return BRW_COMPAREFUNCTION_NEVER;
 }



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