On Tue, Aug 7, 2012 at 9:23 AM, Eric Anholt <e...@anholt.net> wrote:
> Anuj Phogat <anuj.pho...@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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
