<div dir="ltr"><div><div><div><div><div><div>I had actually made the change to gen8_upload_blend_state, but after<br></div>reading through the gen8 PRM a few times I decided to back it out.<br></div>It does seem that the initial gen8 BLEND_STATE DWord can disable alpha test.<br></div>Of course, new hardware features may not always behave as described.<br></div>In that case allowances will need to be made.<br></div>Or, we could always fill in the extra eight bytes for the first BLEND_STATE entry.<br></div>Given that the structure is 64 byte aligned, that 'extra' data is almost always free.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 2, 2015 at 1:45 AM, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wednesday, July 01, 2015 10:16:28 AM Mike Stroyan wrote:<br>
> When there are no color buffer render targets, gen6 and gen7 still<br>
> use the first BLEND_STATE element to determine alpha test.<br>
> gen6_upload_blend_state was allocating zero elements when<br>
> ctx->Color.AlphaEnabled was false.<br>
> That left _3DSTATE_CC_STATE_POINTERS or _3DSTATE_BLEND_STATE_POINTERS<br>
> pointing to random data from some previous brw_state_batch().<br>
> That sometimes suppressed depth rendering when those bits<br>
> happened to mean COMPAREFUNC_NEVER.<br>
> This produced flickering shadows for dota2 reborn.<br>
> ---<br>
>  src/mesa/drivers/dri/i965/gen6_cc.c | 2 +-<br>
>  1 file changed, 1 insertion(+), 1 deletion(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/gen6_cc.c b/src/mesa/drivers/dri/i965/gen6_cc.c<br>
> index 2bfa271..2b76e24 100644<br>
> --- a/src/mesa/drivers/dri/i965/gen6_cc.c<br>
> +++ b/src/mesa/drivers/dri/i965/gen6_cc.c<br>
> @@ -51,7 +51,7 @@ gen6_upload_blend_state(struct brw_context *brw)<br>
>      * with render target 0, which will reference BLEND_STATE[0] for<br>
>      * alpha test enable.<br>
>      */<br>
> -   if (nr_draw_buffers == 0 && ctx->Color.AlphaEnabled)<br>
> +   if (nr_draw_buffers == 0)<br>
>        nr_draw_buffers = 1;<br>
><br>
>     size = sizeof(*blend) * nr_draw_buffers;<br>
><br>
<br>
</div></div>Great catch!<br>
<br>
Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
<br>
And pushed:<br>
   9d408a4..fe2b748  master -> master<br>
<br>
I think we ought to change gen8_blend_state.c as well, but I'm not quite<br>
sure what change to make.  Either we should make the same change you did<br>
here, or delete the whole "We need at least 1 BLEND_STATE written"<br>
block.<br>
<br>
On Gen8+, it looks like the alpha test and other functions that might<br>
discard pixels are all in the shared/common DWord, and the per-color<br>
target DWord pairs look relatively harmless.  I suppose the null RT<br>
would still refer to BLEND_STATE[0]...so it might still be worth<br>
emitting one.  Any thoughts?<br>
</blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><br> Mike Stroyan - Software Architect<br> LunarG, Inc.  - The Graphics Experts<br> Cell:  (970) 219-7905<br> Email: Mike@LunarG.com<br> Website: <a href="http://www.lunarg.com" target="_blank">http://www.lunarg.com</a></div>
</div>