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

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/15/2011 11:00 AM, Kenneth Graunke wrote:
> 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.

We could always add a flag in the gl_shader_program that the program
uses TXD.  You could force c->key.depth_compare_func to 0 whenever that
flag is zero.  Or we could put a similar flag in the brw_fs structure
and do a preprocessing step to set it.  Right?

>>> +
>>> +      /* 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.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk34/DQACgkQX1gOwKyEAw9KsACgk/L9OPEOdduhPeaYTyFyEK4U
L04Ani+R/agfr/dxurd0+poca4EP1Uru
=b63p
-----END PGP SIGNATURE-----