subject:"\\\\\\\[Mesa\\\\\\\-dev\\\\\\\] \\\\\\\[PATCH 3\\\\\\\/5\\\\\\\] i965\\\\\\\/fs\\\\\\\: Add support for textureGather\\\\\\\(.., comp\\\\\\\)"

[Mesa-dev] [PATCH 3/5] i965/fs: Add support for textureGather(.., comp)

2013-10-05 Thread Chris Forbes

- For HSW: Select the channel based on the component selected (swizzle
  is done in HW)
- For IVB: Select the channel based on the swizzle state for the
  component selected. Only apply the RG32F w/a if we actually want
  green -- we're about to flag it regardless of swizzle state.

Signed-off-by: Chris Forbes chr...@ijw.co.nz
---
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 6141009..5508cdc 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -1475,7 +1475,8 @@ fs_visitor::visit(ir_texture *ir)
   /* When tg4 is used with the degenerate ZERO/ONE swizzles, don't bother
* emitting anything other than setting up the constant result.
*/
-  int swiz = GET_SWZ(c-key.tex.swizzles[sampler], 0);
+  ir_constant *chan = ir-lod_info.component-as_constant();
+  int swiz = GET_SWZ(c-key.tex.swizzles[sampler], chan-value.i[0]);
   if (swiz == SWIZZLE_ZERO || swiz == SWIZZLE_ONE) {
 
  fs_reg res = fs_reg(this, glsl_type::vec4_type);
@@ -1594,14 +1595,16 @@ fs_visitor::visit(ir_texture *ir)
 uint32_t
 fs_visitor::gather_channel(ir_texture *ir, int sampler)
 {
-   int swiz = GET_SWZ(c-key.tex.swizzles[sampler], 0 /* red */);
-   if (c-key.tex.gather_channel_quirk_mask  (1sampler))
-  return 2;   /* gather4 sampler is broken for green channel on RG32F --
-   * we must ask for blue instead.
-   */
+   ir_constant *chan = ir-lod_info.component-as_constant();
+   int swiz = GET_SWZ(c-key.tex.swizzles[sampler], chan-value.i[0] /* red 
*/);
switch (swiz) {
   case SWIZZLE_X: return 0;
-  case SWIZZLE_Y: return 1;
+  case SWIZZLE_Y:
+ if (c-key.tex.gather_channel_quirk_mask  (1sampler))
+return 2;   /* gather4 sampler is broken for green channel on 
RG32F --
+ * we must ask for blue instead.
+ */
+ return 1;
   case SWIZZLE_Z: return 2;
   case SWIZZLE_W: return 3;
   default:
-- 
1.8.4

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

Re: [Mesa-dev] [PATCH 3/5] i965/fs: Add support for textureGather(.., comp)

2013-10-05 Thread Kenneth Graunke

On 10/05/2013 03:38 AM, Chris Forbes wrote:
 - For HSW: Select the channel based on the component selected (swizzle
   is done in HW)
 - For IVB: Select the channel based on the swizzle state for the
   component selected. Only apply the RG32F w/a if we actually want
   green -- we're about to flag it regardless of swizzle state.
 
 Signed-off-by: Chris Forbes chr...@ijw.co.nz
 ---
  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 17 ++---
  1 file changed, 10 insertions(+), 7 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 index 6141009..5508cdc 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 @@ -1475,7 +1475,8 @@ fs_visitor::visit(ir_texture *ir)
/* When tg4 is used with the degenerate ZERO/ONE swizzles, don't bother
 * emitting anything other than setting up the constant result.
 */
 -  int swiz = GET_SWZ(c-key.tex.swizzles[sampler], 0);
 +  ir_constant *chan = ir-lod_info.component-as_constant();
 +  int swiz = GET_SWZ(c-key.tex.swizzles[sampler], chan-value.i[0]);

Cool!  On my first pass through the spec I hadn't realized that the
channel parameter was required to be a constant integer expression, and
you indeed enforced that by using ir_var_const_in.  Makes life here so
much easier.

There's only one problem: right now, nothing enforces the constraint
that the component parameter must be 0, 1, 2, or 3.  Although it isn't
explicitly stated in the spec, I believe specifying another value should
be a compile error.  I think this has to get handled at the ast_to_hir
step, and can probably be an ugly special case for now.

It looks like currently, the bitshifts in GET_SWZ will probably return
0, resulting in the red channel being selected.  So there isn't an out
of bounds access problem...just undefined results.  A compile error is
definitely more friendly, though.


if (swiz == SWIZZLE_ZERO || swiz == SWIZZLE_ONE) {
  
   fs_reg res = fs_reg(this, glsl_type::vec4_type);
 @@ -1594,14 +1595,16 @@ fs_visitor::visit(ir_texture *ir)
  uint32_t
  fs_visitor::gather_channel(ir_texture *ir, int sampler)
  {
 -   int swiz = GET_SWZ(c-key.tex.swizzles[sampler], 0 /* red */);
 -   if (c-key.tex.gather_channel_quirk_mask  (1sampler))
 -  return 2;   /* gather4 sampler is broken for green channel on RG32F --
 -   * we must ask for blue instead.
 -   */
 +   ir_constant *chan = ir-lod_info.component-as_constant();
 +   int swiz = GET_SWZ(c-key.tex.swizzles[sampler], chan-value.i[0] /* red 
 */);

The /* red */ comment here doesn't make sense anymore...I'd drop it.

 switch (swiz) {
case SWIZZLE_X: return 0;
 -  case SWIZZLE_Y: return 1;
 +  case SWIZZLE_Y:
 + if (c-key.tex.gather_channel_quirk_mask  (1sampler))
 +return 2;   /* gather4 sampler is broken for green channel on 
 RG32F --
 + * we must ask for blue instead.
 + */
 + return 1;

I would like to see the comment above the block, rather than off to the
side like this.  Ditto for the next patch.

case SWIZZLE_Z: return 2;
case SWIZZLE_W: return 3;
default:
 

Patches 3-4 are:
Reviewed-by: Kenneth Graunke kenn...@whitecape.org

(since my comment about bounds checking 'comp' would go in a different
patch anyway)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/5] i965/fs: Add support for textureGather(.., comp)

2013-10-05 Thread Chris Forbes

I'm not sure about the compile-time error, or how to hack it in -- at
ast_to_hir time, function calls haven't been inlined yet, so I think
this gets really nasty?

Will address it in a follow-up patch though as you suggest.

-- Chris

On Sun, Oct 6, 2013 at 7:06 AM, Kenneth Graunke kenn...@whitecape.org wrote:
 On 10/05/2013 03:38 AM, Chris Forbes wrote:
 - For HSW: Select the channel based on the component selected (swizzle
   is done in HW)
 - For IVB: Select the channel based on the swizzle state for the
   component selected. Only apply the RG32F w/a if we actually want
   green -- we're about to flag it regardless of swizzle state.

 Signed-off-by: Chris Forbes chr...@ijw.co.nz
 ---
  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 17 ++---
  1 file changed, 10 insertions(+), 7 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 index 6141009..5508cdc 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 @@ -1475,7 +1475,8 @@ fs_visitor::visit(ir_texture *ir)
/* When tg4 is used with the degenerate ZERO/ONE swizzles, don't 
 bother
 * emitting anything other than setting up the constant result.
 */
 -  int swiz = GET_SWZ(c-key.tex.swizzles[sampler], 0);
 +  ir_constant *chan = ir-lod_info.component-as_constant();
 +  int swiz = GET_SWZ(c-key.tex.swizzles[sampler], chan-value.i[0]);

 Cool!  On my first pass through the spec I hadn't realized that the
 channel parameter was required to be a constant integer expression, and
 you indeed enforced that by using ir_var_const_in.  Makes life here so
 much easier.

 There's only one problem: right now, nothing enforces the constraint
 that the component parameter must be 0, 1, 2, or 3.  Although it isn't
 explicitly stated in the spec, I believe specifying another value should
 be a compile error.  I think this has to get handled at the ast_to_hir
 step, and can probably be an ugly special case for now.

 It looks like currently, the bitshifts in GET_SWZ will probably return
 0, resulting in the red channel being selected.  So there isn't an out
 of bounds access problem...just undefined results.  A compile error is
 definitely more friendly, though.


if (swiz == SWIZZLE_ZERO || swiz == SWIZZLE_ONE) {

   fs_reg res = fs_reg(this, glsl_type::vec4_type);
 @@ -1594,14 +1595,16 @@ fs_visitor::visit(ir_texture *ir)
  uint32_t
  fs_visitor::gather_channel(ir_texture *ir, int sampler)
  {
 -   int swiz = GET_SWZ(c-key.tex.swizzles[sampler], 0 /* red */);
 -   if (c-key.tex.gather_channel_quirk_mask  (1sampler))
 -  return 2;   /* gather4 sampler is broken for green channel on RG32F --
 -   * we must ask for blue instead.
 -   */
 +   ir_constant *chan = ir-lod_info.component-as_constant();
 +   int swiz = GET_SWZ(c-key.tex.swizzles[sampler], chan-value.i[0] /* red 
 */);

 The /* red */ comment here doesn't make sense anymore...I'd drop it.

 switch (swiz) {
case SWIZZLE_X: return 0;
 -  case SWIZZLE_Y: return 1;
 +  case SWIZZLE_Y:
 + if (c-key.tex.gather_channel_quirk_mask  (1sampler))
 +return 2;   /* gather4 sampler is broken for green channel on 
 RG32F --
 + * we must ask for blue instead.
 + */
 + return 1;

 I would like to see the comment above the block, rather than off to the
 side like this.  Ditto for the next patch.

case SWIZZLE_Z: return 2;
case SWIZZLE_W: return 3;
default:


 Patches 3-4 are:
 Reviewed-by: Kenneth Graunke kenn...@whitecape.org

 (since my comment about bounds checking 'comp' would go in a different
 patch anyway)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 3/5] i965/fs: Add support for textureGather(.., comp)

Re: [Mesa-dev] [PATCH 3/5] i965/fs: Add support for textureGather(.., comp)

Re: [Mesa-dev] [PATCH 3/5] i965/fs: Add support for textureGather(.., comp)

3 matches

Site Navigation

Mail list logo

Footer information