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

[Anuj Phogat]

On Tue, Aug 7, 2012 at 9:23 AM, Eric Anholt <eric at anholt.net> wrote:
> Anuj Phogat <anuj.phogat at gmail.com> writes:
>
>> 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.
>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> index fefe2c7..7fc28ac 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> @@ -1930,14 +1930,24 @@ fs_visitor::emit_color_write(int target, int index, int first_color_mrf)
>>  {
>>     int reg_width = c->dispatch_width / 8;
>>     fs_inst *inst;
>> -   fs_reg color = outputs[target];
>> +   fs_reg color;
>> +   bool src0_alpha_to_render_target = target > 0 &&
>> +                                   c->key.nr_color_regions > 1 &&
>> +                                   c->key.sample_alpha_to_coverage;
>> +
>> +   color = (src0_alpha_to_render_target && !index) ?
>> +         outputs[0] :
>> +         outputs[target];
>>     fs_reg mrf;
>>
>>     /* If there's no color data to be written, skip it. */
>>     if (color.file == BAD_FILE)
>>        return;
>>
>> -   color.reg_offset += index;
>> +   if (src0_alpha_to_render_target)
>> +      color.reg_offset += !index ? 3 : index - 1;
>> +   else
>> +      color.reg_offset += index;
>
> Ew, this is really awful.
>
> How about instead..,
>
>> -      for (unsigned i = 0; i < this->output_components[target]; i++)
>> -      emit_color_write(target, i, color_mrf);
>> +      /* If src0_alpha_to_render_target is true, include source zero alpha
>> +       * data in RenderTargetWrite message for targets > 0.
>> +       */
>> +      output_components = (target && src0_alpha_to_render_target) ?
>> +                       (this->output_components[target] + 1) :
>> +                       this->output_components[target];
>>
>> -      fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
>> +      for (unsigned i = 0; i < output_components; i++)
>> +         emit_color_write(target, i, color_mrf);
>
> Replace all of this change with:
>
>       if (src0_alpha_to_render_target) {
>          emit_color_write(0, 3, color_mrf);
Without any modifications in emit_color_write() function, this will
write src0 alpha in wrong register: color_mrf + 3 * reg_width.
We want the value to be written at color_mrf.

>          color_mrf += reg_width);
This will permanently modify the initial value of color_mrf which
should stay same for all the render targets. I verified that
draw-buffers-alpha-to-coverage fails with above changes.

I will send out a new patch which removes all the changes you
didn't like in emit_color_write() function.
>       }
>       for (unsigned i = 0; i < this->output_components[target]; i++)
>          emit_color_write(target, i, color_mrf);
>
>> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
>> index 5ab0547..210b078 100644
>> --- a/src/mesa/drivers/dri/i965/brw_wm.c
>> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
>> @@ -546,6 +546,8 @@ static void brw_wm_populate_key( struct brw_context *brw,
>>     /* _NEW_BUFFERS */
>>     key->nr_color_regions = ctx->DrawBuffer->_NumColorDrawBuffers;
>
> Needs
>       /* _NEW_MULTISAMPLE */
>> +   key->sample_alpha_to_coverage = ctx->Multisample.SampleAlphaToCoverage;
>
> and corresponding addition to the state struct below.