[Mesa-dev] [PATCH 7/8] i965/fs: Add support for TXD with shadow comparisons on gen4-6.

[Kenneth Graunke]

On 06/15/2011 08:27 AM, Eric Anholt wrote:
> On Wed, 15 Jun 2011 01:24:54 -0700, Kenneth Graunke<kenneth at whitecape.org>  wrote:
>> Gen4-6 don't have a sample_d_c message, so we have to do a regular
>> sample_d and emit instructions to manually perform the comparison.
>>
>> This requires a state dependent recompile whenever ctx->Depth.Func
>> changes.  do_wm_prog looks for a compiled program in the cache based off
>> of brw_wm_prog_key, and if it doesn't find one, recompiles.  So we
>> simply need to add the depth comparison function to the key; the
>> _NEW_DEPTH dirty bit was already in-place.
>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> index 03687ce..67344bd 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> @@ -744,7 +744,7 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>>      }
>>      mlen += ir->coordinate->type->vector_elements * reg_width;
>>
>> -   if (ir->shadow_comparitor) {
>> +   if (ir->shadow_comparitor&&  ir->op != ir_txd) {
>>         mlen = MAX2(mlen, header_present + 4 * reg_width);
>>
>>         this->result = reg_undef;
>> @@ -934,6 +934,21 @@ fs_visitor::visit(ir_texture *ir)
>>      int sampler = _mesa_get_sampler_uniform_value(ir->sampler, prog,&fp->Base);
>>      sampler = fp->Base.SamplerUnits[sampler];
>>
>> +   /* Pre-Ivybridge doesn't have a sample_d_c message, so shadow compares
>> +    * for textureGrad/TXD need to be emulated with instructions.
>> +    */
>> +   bool hw_compare_supported = ir->op != ir_txd || intel->gen>  7;
>> +   if (ir->shadow_comparitor&&  !hw_compare_supported) {
>> +      /* Mark that this program is only valid for the current glDepthFunc */
>> +      c->key.depth_compare_func = ctx->Depth.Func;
>
> The compiler is only called if the key wasn't found in the cache, so you
> can't set the key inside the compiler :)

You're right, of course.  Unfortunately, I really want to!  Only shaders 
using shadow2DGradARB need to be recompiled when glDepthFunc changes, so 
I really want to leave it as 0 for most shaders.  Otherwise the common 
case will have completely pointless recompiles.

Unfortunately, I only know whether or not I need it at compile time...

This -almost- works...it just means we'll recompile shaders using 
shadow2DGradARB every time, since they'll never be found in the cache. 
Stupid.  I'll reply with a patch which works around this.

>> +
>> +      /* No need to even sample for GL_ALWAYS or GL_NEVER...bail early */
>> +      if (ctx->Depth.Func == GL_ALWAYS)
>> +	 return swizzle_shadow_result(ir, fs_reg(1.0f), sampler);
>> +      else if (ctx->Depth.Func == GL_NEVER)
>> +	 return swizzle_shadow_result(ir, fs_reg(0.0f), sampler);
>> +   }
>
> Generally I use "c->key." values to make it obvious that state dependent
> recompiles are in place, instead of "ctx->".

Good idea.  Updated.

>> diff --git a/src/mesa/drivers/dri/i965/brw_wm.h b/src/mesa/drivers/dri/i965/brw_wm.h
>> index e244b55..0b284b6 100644
>> --- a/src/mesa/drivers/dri/i965/brw_wm.h
>> +++ b/src/mesa/drivers/dri/i965/brw_wm.h
>> @@ -67,6 +67,7 @@ struct brw_wm_prog_key {
>>      GLuint alpha_test:1;
>>      GLuint clamp_fragment_color:1;
>>      GLuint line_aa:2;
>> +   uint8_t depth_compare_func; /* 0 if irrelevant, GL_LESS etc. otherwise */
>
> Wow, those do happen to fit in 8 bits.  Might comment that explicitly --
> leaving out bits when storing GLenums has been a common bug and seeing
> things like this make me paranoid and go check :)

:) Makes sense; commented.  I figured we really want to pack the key.