[Mesa-dev] [PATCH 5/6] i965/fs: Implement textureSize (TXS) on Gen5+.

[Kenneth Graunke]

On 08/23/2011 11:18 AM, Eric Anholt wrote:
> On Mon, 22 Aug 2011 16:26:23 -0700, Kenneth Graunke <kenneth at whitecape.org> wrote:
>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> ---
>>  src/mesa/drivers/dri/i965/brw_defines.h      |    2 +
>>  src/mesa/drivers/dri/i965/brw_fs.cpp         |    1 +
>>  src/mesa/drivers/dri/i965/brw_fs.h           |    3 +-
>>  src/mesa/drivers/dri/i965/brw_fs_emit.cpp    |    4 +++
>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   28 +++++++++++++++++++------
>>  src/mesa/program/ir_to_mesa.cpp              |    7 ++++-
>>  6 files changed, 35 insertions(+), 10 deletions(-)

Ah crap.  After receiving Ian and Dave's review, I -just- pushed
these...right before receiving your email.  Sorry...should've waited...

>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> index 792799d..3551e3d 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> @@ -751,6 +751,8 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>>     int base_mrf = 2;
>>     int reg_width = c->dispatch_width / 8;
>>     bool header_present = false;
>> +   const int vector_elements =
>> +      ir->coordinate ? ir->coordinate->type->vector_elements : 0;
>>  
>>     if (ir->offset) {
>>        /* The offsets set up by the ir_texture visitor are in the
>> @@ -761,7 +763,7 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>>        base_mrf--;
>>     }
>>  
> 
> Whoah, called the accept() method of the null pointer for the coordinate
> and things didn't blow up?

No: fs_visitor::visit(ir_texture *) checks if ir->coordinate != NULL
before calling accept() on it.  This hunk just avoids a NULL-pointer
dereference on ir->coordinate->type->vector_elements and, by setting the
dimensionality of the coordinate to 0, avoids the loops that MOV the
coordinate in.

>> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
>> index b222005..e7609df 100644
>> --- a/src/mesa/program/ir_to_mesa.cpp
>> +++ b/src/mesa/program/ir_to_mesa.cpp
>> @@ -2104,7 +2104,10 @@ ir_to_mesa_visitor::visit(ir_texture *ir)
>>     ir_to_mesa_instruction *inst = NULL;
>>     prog_opcode opcode = OPCODE_NOP;
>>  
>> -   ir->coordinate->accept(this);
>> +   if (ir->op == ir_txs)
>> +      this->result = src_reg_for_float(0.0);
>> +   else
>> +      ir->coordinate->accept(this);
>>  
>>     /* Put our coords in a temp.  We'll need to modify them for shadow,
>>      * projection, or LOD, so the only case we'd use it as is is if
>> @@ -2128,6 +2131,7 @@ ir_to_mesa_visitor::visit(ir_texture *ir)
>>  
>>     switch (ir->op) {
>>     case ir_tex:
>> +   case ir_txs:
>>        opcode = OPCODE_TEX;
>>        break;
>>     case ir_txb:
>> @@ -2148,7 +2152,6 @@ ir_to_mesa_visitor::visit(ir_texture *ir)
>>        dy = this->result;
>>        break;
>>     case ir_txf:
>> -   case ir_txs:
>>        assert(!"GLSL 1.30 features unsupported");
>>        break;
>>     }
> 
> The ir_to_mesa hunks probably deserve a separate commit to explain
> what's going on there.
> 
> The other patches are:
> 
> Reviewed-by: Eric Anholt <eric at anholt.net>