On 13 August 2012 16:46, Anuj Phogat <anuj.pho...@gmail.com> wrote:
> Render Target Write message should include source zero alpha value when
> sample-alpha-to-coverage is enabled for an FBO with multiple render
> targets.
> Source zero alpha value is used as fragment coverage for all the render
> targets.
>
> This patch makes piglit tests draw-buffers-alpha-to-coverage and
> alpha-to-coverage-no-draw-buffer-zero to pass on Sandybridge. No
> regressions are observed with piglit all.tests.
>
> V2: Revert all the changes made in emit_color_write() function to
> include src0 alpha for targets > 0. Now handling this case in a if
> block.
>
> Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com>
> ---
> src/mesa/drivers/dri/i965/brw_fs_emit.cpp | 12 ++++++++++++
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 25
> ++++++++++++++++++++++++-
> src/mesa/drivers/dri/i965/brw_wm.c | 2 ++
> src/mesa/drivers/dri/i965/brw_wm.h | 1 +
> 4 files changed, 39 insertions(+), 1 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
> index dc5f3e1..c039cab 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
> @@ -59,6 +59,18 @@ fs_visitor::generate_fb_write(fs_inst *inst)
> retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD));
> brw_set_compression_control(p, BRW_COMPRESSION_NONE);
>
> + if (inst->target > 0 &&
> + c->key.nr_color_regions > 1 &&
> + c->key.sample_alpha_to_coverage) {
> + /* Set "Source0 Alpha Present to RenderTarget" bit in message
> + * header.
> + */
> + brw_OR(p,
> + vec1(retype(brw_message_reg(inst->base_mrf),
> BRW_REGISTER_TYPE_UD)),
> + vec1(retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD)),
> + brw_imm_ud(0x1 << 11));
> + }
> +
>
if (inst->target > 0) {
> /* Set the render target index for choosing BLEND_STATE. */
> brw_MOV(p, retype(brw_vec1_reg(BRW_MESSAGE_REGISTER_FILE,
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index fefe2c7..3ede7ed 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -2014,6 +2014,7 @@ fs_visitor::emit_fb_writes()
> int nr = base_mrf;
> int reg_width = c->dispatch_width / 8;
> bool do_dual_src = this->dual_src_output.file != BAD_FILE;
> + bool src0_alpha_to_render_target = false;
>
> if (c->dispatch_width == 16 && do_dual_src) {
> fail("GL_ARB_blend_func_extended not yet supported in 16-wide.");
> @@ -2035,6 +2036,8 @@ fs_visitor::emit_fb_writes()
> }
>
> if (header_present) {
> + src0_alpha_to_render_target = c->key.nr_color_regions > 1 &&
> + c->key.sample_alpha_to_coverage;
> /* m2, m3 header */
> nr += 2;
> }
> @@ -2051,6 +2054,8 @@ fs_visitor::emit_fb_writes()
> nr += 4 * reg_width;
> if (do_dual_src)
> nr += 4;
> + if (src0_alpha_to_render_target)
> + nr += reg_width;
>
(We discussed this in person, just sending it to the list so we don't
forget)
It looks like we should update this condition so that nr doesn't get
incremented when do_dual_src is true, since the do_dual_src branch doesn't
send alpha0 values.
>
> if (c->source_depth_to_render_target) {
> if (intel->gen == 6 && c->dispatch_width == 16) {
> @@ -2122,8 +2127,26 @@ fs_visitor::emit_fb_writes()
> this->current_annotation = ralloc_asprintf(this->mem_ctx,
> "FB write target %d",
> target);
> + /* If src0_alpha_to_render_target is true, include source zero alpha
> + * data in RenderTargetWrite message for targets > 0.
> + */
> + int write_color_mrf = color_mrf;
> + if (intel->gen >= 6 &&
> + src0_alpha_to_render_target &&
> + target) {
> + fs_inst *inst;
> + fs_reg color = outputs[0];
> + color.reg_offset += 3;
> +
> + inst = emit(BRW_OPCODE_MOV,
> + fs_reg(MRF, write_color_mrf, color.type),
> + color);
> + inst->saturate = c->key.clamp_fragment_color;
> + write_color_mrf = color_mrf + reg_width;
> + }
> +
> for (unsigned i = 0; i < this->output_components[target]; i++)
> - emit_color_write(target, i, color_mrf);
> + emit_color_write(target, i, write_color_mrf);
>
> fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
> inst->target = target;
>
A few lines below this there's a statement "inst->mlen = nr - base_mrf;".
That's going to be wrong in the case where target == 0, since for target ==
0 we don't send the alpha0 values.
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c
> b/src/mesa/drivers/dri/i965/brw_wm.c
> index 5ab0547..8bf551e 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -545,6 +545,8 @@ static void brw_wm_populate_key( struct brw_context
> *brw,
>
> /* _NEW_BUFFERS */
> key->nr_color_regions = ctx->DrawBuffer->_NumColorDrawBuffers;
> + /* _NEW_MULTISAMPLE */
> + key->sample_alpha_to_coverage = ctx->Multisample.SampleAlphaToCoverage;
>
> /* CACHE_NEW_VS_PROG */
> key->vp_outputs_written = brw->vs.prog_data->outputs_written;
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.h
> b/src/mesa/drivers/dri/i965/brw_wm.h
> index b976a60..730daf3 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.h
> +++ b/src/mesa/drivers/dri/i965/brw_wm.h
> @@ -64,6 +64,7 @@ struct brw_wm_prog_key {
> GLuint stats_wm:1;
> GLuint flat_shade:1;
> GLuint nr_color_regions:5;
> + GLuint sample_alpha_to_coverage:1; /* _NEW_MULTISAMPLE */
> GLuint render_to_fbo:1;
> GLuint clamp_fragment_color:1;
> GLuint line_aa:2;
> --
> 1.7.7.6
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
