On 20 August 2012 16:59, Anuj Phogat <span dir="ltr"><<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">Render Target Write message should include source zero alpha value when<br>
sample-alpha-to-coverage is enabled for an FBO with multiple render targets.<br>
Source zero alpha value is used as fragment coverage for all the render<br>
targets.<br>
<br>
This patch makes piglit tests draw-buffers-alpha-to-coverage and<br>
alpha-to-coverage-no-draw-buffer-zero to pass on Sandybridge. No<br>
regressions are observed with piglit all.tests.<br>
<br>
V2: Revert all the changes made in emit_color_write() function to<br>
include src0 alpha for targets > 0. Now handling this case in a if<br>
block.<br>
<br>
</div>V3: Correctly calculate the instruction length for buffer zero.<br>
Properly handle the case of dual_src_blend when alpha-to-coverage<br>
is enabled.<br>
<div class="im"><br>
Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>><br>
---<br>
src/mesa/drivers/dri/i965/brw_fs_emit.cpp | 12 ++++++++++<br>
</div> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 30 ++++++++++++++++++++++++-<br>
src/mesa/drivers/dri/i965/brw_wm.c | 2 +<br>
src/mesa/drivers/dri/i965/brw_wm.h | 1 +<br>
4 files changed, 43 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp<br>
index 4564e3b..5900c0e 100644<br>
<div class="im">--- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp<br>
+++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp<br>
@@ -59,6 +59,18 @@ fs_visitor::generate_fb_write(fs_inst *inst)<br>
retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD));<br>
brw_set_compression_control(p, BRW_COMPRESSION_NONE);<br>
<br>
+ if (inst->target > 0 &&<br>
+ c->key.nr_color_regions > 1 &&<br>
+ c->key.sample_alpha_to_coverage) {<br>
+ /* Set "Source0 Alpha Present to RenderTarget" bit in message<br>
+ * header.<br>
+ */<br>
+ brw_OR(p,<br>
+ vec1(retype(brw_message_reg(inst->base_mrf), BRW_REGISTER_TYPE_UD)),<br>
+ vec1(retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD)),<br>
+ brw_imm_ud(0x1 << 11));<br>
+ }<br>
+<br>
if (inst->target > 0) {<br>
/* Set the render target index for choosing BLEND_STATE. */<br>
brw_MOV(p, retype(brw_vec1_reg(BRW_MESSAGE_REGISTER_FILE,<br>
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp<br>
</div>index 7a2f777..2309059 100644<br>
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp<br>
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp<br>
@@ -2057,6 +2057,7 @@ fs_visitor::emit_fb_writes()<br>
<div class="im"> int nr = base_mrf;<br>
int reg_width = c->dispatch_width / 8;<br>
bool do_dual_src = this->dual_src_output.file != BAD_FILE;<br>
+ bool src0_alpha_to_render_target = false;<br>
<br>
if (c->dispatch_width == 16 && do_dual_src) {<br>
fail("GL_ARB_blend_func_extended not yet supported in 16-wide.");<br>
</div>@@ -2078,6 +2079,10 @@ fs_visitor::emit_fb_writes()<br>
}<br>
<br>
if (header_present) {<br>
+ src0_alpha_to_render_target = intel->gen >= 6 &&<br>
+ !do_dual_src &&<br>
<div class="im">+ c->key.nr_color_regions > 1 &&<br>
</div>+ c->key.sample_alpha_to_coverage;<br>
<div class="im"> /* m2, m3 header */<br>
nr += 2;<br>
}<br>
</div>@@ -2094,6 +2099,8 @@ fs_visitor::emit_fb_writes()<br>
<div class="im"> nr += 4 * reg_width;<br>
if (do_dual_src)<br>
nr += 4;<br>
+ if (src0_alpha_to_render_target)<br>
+ nr += reg_width;<br>
<br>
</div><div class="im"> if (c->source_depth_to_render_target) {<br>
if (intel->gen == 6 && c->dispatch_width == 16) {<br>
</div>@@ -2165,13 +2172,32 @@ fs_visitor::emit_fb_writes()<br>
<div class="im"> this->current_annotation = ralloc_asprintf(this->mem_ctx,<br>
"FB write target %d",<br>
target);<br>
+ /* If src0_alpha_to_render_target is true, include source zero alpha<br>
+ * data in RenderTargetWrite message for targets > 0.<br>
+ */<br>
+ int write_color_mrf = color_mrf;<br>
</div>+ if (src0_alpha_to_render_target && target) {<br></blockquote><div><br>This condition is confusing to read because it makes "target" look like a boolean. It would be easier to read as "if (src0_alpha_to_render_target && target != 0)".<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">+ fs_inst *inst;<br>
+ fs_reg color = outputs[0];<br>
+ color.reg_offset += 3;<br>
+<br>
+ inst = emit(BRW_OPCODE_MOV,<br>
+ fs_reg(MRF, write_color_mrf, color.type),<br>
+ color);<br>
+ inst->saturate = c->key.clamp_fragment_color;<br>
+ write_color_mrf = color_mrf + reg_width;<br>
+ }<br>
+<br>
for (unsigned i = 0; i < this->output_components[target]; i++)<br>
- emit_color_write(target, i, color_mrf);<br>
+ emit_color_write(target, i, write_color_mrf);<br>
<br>
fs_inst *inst = emit(FS_OPCODE_FB_WRITE);<br>
inst->target = target;<br>
</div> inst->base_mrf = base_mrf;<br>
<div class="im">- inst->mlen = nr - base_mrf;<br>
</div>+ if (src0_alpha_to_render_target && !target)<br></blockquote><div><br>Similarly, this would be easier to read if we replaced "!target" with "target == 0".<br><br>With those changes, this patch is:<br>
<br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ inst->mlen = nr - base_mrf - reg_width;<br>
+ else<br>
<div class="im">+ inst->mlen = nr - base_mrf;<br>
</div> if (target == c->key.nr_color_regions - 1)<br>
inst->eot = true;<br>
inst->header_present = header_present;<br>
diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c<br>
index 323eabd..6e5163b 100644<br>
--- a/src/mesa/drivers/dri/i965/brw_wm.c<br>
+++ b/src/mesa/drivers/dri/i965/brw_wm.c<br>
@@ -633,6 +633,8 @@ static void brw_wm_populate_key( struct brw_context *brw,<br>
<div class="im"><br>
/* _NEW_BUFFERS */<br>
key->nr_color_regions = ctx->DrawBuffer->_NumColorDrawBuffers;<br>
+ /* _NEW_MULTISAMPLE */<br>
+ key->sample_alpha_to_coverage = ctx->Multisample.SampleAlphaToCoverage;<br>
<br>
/* CACHE_NEW_VS_PROG */<br>
key->vp_outputs_written = brw->vs.prog_data->outputs_written;<br>
diff --git a/src/mesa/drivers/dri/i965/brw_wm.h b/src/mesa/drivers/dri/i965/brw_wm.h<br>
</div>index 53b644f..5e4af27 100644<br>
<div class="HOEnZb"><div class="h5">--- a/src/mesa/drivers/dri/i965/brw_wm.h<br>
+++ b/src/mesa/drivers/dri/i965/brw_wm.h<br>
@@ -64,6 +64,7 @@ struct brw_wm_prog_key {<br>
GLuint stats_wm:1;<br>
GLuint flat_shade:1;<br>
GLuint nr_color_regions:5;<br>
+ GLuint sample_alpha_to_coverage:1; /* _NEW_MULTISAMPLE */<br>
GLuint render_to_fbo:1;<br>
GLuint clamp_fragment_color:1;<br>
GLuint line_aa:2;<br>
--<br>
1.7.7.6<br>
<br>
</div></div></blockquote></div><br>