[Mesa-dev] [PATCH] i965/msaa: Add sample-alpha-to-coverage support for multiple render targets

Paul Berry stereotype441 at gmail.com
Wed Aug 22 15:02:46 PDT 2012


On 20 August 2012 16:59, Anuj Phogat <anuj.phogat at 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.
>
> V3: Correctly calculate the instruction length for buffer zero.
> Properly handle the case of dual_src_blend when alpha-to-coverage
> is enabled.
>
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> ---
>  src/mesa/drivers/dri/i965/brw_fs_emit.cpp    |   12 ++++++++++
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   30
> ++++++++++++++++++++++++-
>  src/mesa/drivers/dri/i965/brw_wm.c           |    2 +
>  src/mesa/drivers/dri/i965/brw_wm.h           |    1 +
>  4 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
> index 4564e3b..5900c0e 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 7a2f777..2309059 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -2057,6 +2057,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.");
> @@ -2078,6 +2079,10 @@ fs_visitor::emit_fb_writes()
>     }
>
>     if (header_present) {
> +      src0_alpha_to_render_target = intel->gen >= 6 &&
> +                                   !do_dual_src &&
> +                                   c->key.nr_color_regions > 1 &&
> +                                   c->key.sample_alpha_to_coverage;
>        /* m2, m3 header */
>        nr += 2;
>     }
> @@ -2094,6 +2099,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;
>
>     if (c->source_depth_to_render_target) {
>        if (intel->gen == 6 && c->dispatch_width == 16) {
> @@ -2165,13 +2172,32 @@ 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 (src0_alpha_to_render_target && target) {
>

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)".


> +         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;
>        inst->base_mrf = base_mrf;
> -      inst->mlen = nr - base_mrf;
> +      if (src0_alpha_to_render_target && !target)
>

Similarly, this would be easier to read if we replaced "!target" with
"target == 0".

With those changes, this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>


> +         inst->mlen = nr - base_mrf - reg_width;
> +      else
> +         inst->mlen = nr - base_mrf;
>        if (target == c->key.nr_color_regions - 1)
>          inst->eot = true;
>        inst->header_present = header_present;
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c
> b/src/mesa/drivers/dri/i965/brw_wm.c
> index 323eabd..6e5163b 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -633,6 +633,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 53b644f..5e4af27 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120822/071ea6ec/attachment.html>


More information about the mesa-dev mailing list