Re: [Mesa-dev] [PATCH] draw: (trivial) fix clamping of viewport index

2014-06-23 Thread Ilia Mirkin
On Mon, Jun 23, 2014 at 4:13 PM, Ilia Mirkin  wrote:
> On Mon, Jun 23, 2014 at 4:08 PM,   wrote:
>> From: Roland Scheidegger 
>>
>> The old logic would let all negative values go through unclamped, with
>> potentially disastrous results (probably trying to fetch viewport values
>> from random memory locations). GL has undefined rendering for vp indices
>> outside valid range but that's a bit too undefined...
>> (The logic is now the same as in llvmpipe.)
>>
>> CC: "10.1 10.2" 
>
> Reviewed-by: Ilia Mirkin 
>
> I haven't tested whether this fixes my issues (don't have the right
> patches/setup on this comp) but this seems obviously correct.

FTR, just tested it out, and it fixes the viewport case with
ARB_fragment_layer_viewport. So you can add my Tested-by as well if
you want.

>
>> ---
>>  src/gallium/auxiliary/draw/draw_private.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/auxiliary/draw/draw_private.h 
>> b/src/gallium/auxiliary/draw/draw_private.h
>> index 783c3ef..d8dc2ab 100644
>> --- a/src/gallium/auxiliary/draw/draw_private.h
>> +++ b/src/gallium/auxiliary/draw/draw_private.h
>> @@ -493,7 +493,7 @@ draw_stats_clipper_primitives(struct draw_context *draw,
>>  static INLINE unsigned
>>  draw_clamp_viewport_idx(int idx)
>>  {
>> -   return ((PIPE_MAX_VIEWPORTS > idx || idx < 0) ? idx : 0);
>> +   return ((PIPE_MAX_VIEWPORTS > idx && idx >= 0) ? idx : 0);
>>  }
>>
>>  /**
>> --
>> 1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] draw: (trivial) fix clamping of viewport index

2014-06-23 Thread Jose Fonseca
LGTM.

Jose

From: srol...@vmware.com 
Sent: 23 June 2014 21:08
To: Jose Fonseca; mesa-dev@lists.freedesktop.org; imir...@alum.mit.edu
Cc: Roland Scheidegger; 10.1 10.2
Subject: [PATCH] draw: (trivial) fix clamping of viewport index

From: Roland Scheidegger 

The old logic would let all negative values go through unclamped, with
potentially disastrous results (probably trying to fetch viewport values
from random memory locations). GL has undefined rendering for vp indices
outside valid range but that's a bit too undefined...
(The logic is now the same as in llvmpipe.)

CC: "10.1 10.2" 
---
 src/gallium/auxiliary/draw/draw_private.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/draw/draw_private.h 
b/src/gallium/auxiliary/draw/draw_private.h
index 783c3ef..d8dc2ab 100644
--- a/src/gallium/auxiliary/draw/draw_private.h
+++ b/src/gallium/auxiliary/draw/draw_private.h
@@ -493,7 +493,7 @@ draw_stats_clipper_primitives(struct draw_context *draw,
 static INLINE unsigned
 draw_clamp_viewport_idx(int idx)
 {
-   return ((PIPE_MAX_VIEWPORTS > idx || idx < 0) ? idx : 0);
+   return ((PIPE_MAX_VIEWPORTS > idx && idx >= 0) ? idx : 0);
 }

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


Re: [Mesa-dev] [PATCH] draw: (trivial) fix clamping of viewport index

2014-06-23 Thread Ilia Mirkin
On Mon, Jun 23, 2014 at 4:08 PM,   wrote:
> From: Roland Scheidegger 
>
> The old logic would let all negative values go through unclamped, with
> potentially disastrous results (probably trying to fetch viewport values
> from random memory locations). GL has undefined rendering for vp indices
> outside valid range but that's a bit too undefined...
> (The logic is now the same as in llvmpipe.)
>
> CC: "10.1 10.2" 

Reviewed-by: Ilia Mirkin 

I haven't tested whether this fixes my issues (don't have the right
patches/setup on this comp) but this seems obviously correct.

> ---
>  src/gallium/auxiliary/draw/draw_private.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/auxiliary/draw/draw_private.h 
> b/src/gallium/auxiliary/draw/draw_private.h
> index 783c3ef..d8dc2ab 100644
> --- a/src/gallium/auxiliary/draw/draw_private.h
> +++ b/src/gallium/auxiliary/draw/draw_private.h
> @@ -493,7 +493,7 @@ draw_stats_clipper_primitives(struct draw_context *draw,
>  static INLINE unsigned
>  draw_clamp_viewport_idx(int idx)
>  {
> -   return ((PIPE_MAX_VIEWPORTS > idx || idx < 0) ? idx : 0);
> +   return ((PIPE_MAX_VIEWPORTS > idx && idx >= 0) ? idx : 0);
>  }
>
>  /**
> --
> 1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] draw: (trivial) fix clamping of viewport index

2014-06-23 Thread sroland
From: Roland Scheidegger 

The old logic would let all negative values go through unclamped, with
potentially disastrous results (probably trying to fetch viewport values
from random memory locations). GL has undefined rendering for vp indices
outside valid range but that's a bit too undefined...
(The logic is now the same as in llvmpipe.)

CC: "10.1 10.2" 
---
 src/gallium/auxiliary/draw/draw_private.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/draw/draw_private.h 
b/src/gallium/auxiliary/draw/draw_private.h
index 783c3ef..d8dc2ab 100644
--- a/src/gallium/auxiliary/draw/draw_private.h
+++ b/src/gallium/auxiliary/draw/draw_private.h
@@ -493,7 +493,7 @@ draw_stats_clipper_primitives(struct draw_context *draw,
 static INLINE unsigned
 draw_clamp_viewport_idx(int idx)
 {
-   return ((PIPE_MAX_VIEWPORTS > idx || idx < 0) ? idx : 0);
+   return ((PIPE_MAX_VIEWPORTS > idx && idx >= 0) ? idx : 0);
 }
 
 /**
-- 
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev