On Fri, 2019-03-22 at 10:06 -0500, Jason Ekstrand wrote: > I'm confused. Don't we always have a NULL render target at location > 0? Is the problem really that we need the NULL render target or is > it that we can't throw away the alpha component of the RT write in > the shader?
It is the latter.
> If it's that we can't throw away the alpha component of the RT
> write, then I'd suggest a slightly different workaround which does
> just that. We can rewrite the store_output intrinsic (or store_var;
> not sure which it is at that point) so that it writes vec4(undef,
> undef, undef, alpha) to help with linking but then keep the one
> output var so it we still get the write in the shader.
>
OK, thanks for the suggestion!
Sam
>
> On Mon, Mar 4, 2019 at 4:56 AM Samuel Iglesias Gonsálvez <
> sigles...@igalia.com> wrote:
> > Still unreviewed.
> >
> > Sam
> >
> > On Thu, 2019-02-21 at 12:08 +0100, Samuel Iglesias Gonsálvez wrote:
> > > CL#3532 added a test for alpha to coverage without a color
> > > attachment.
> > > First the test draws a primitive with alpha 0 and a subpass with
> > only
> > > a depth buffer. No writes to a depth buffer are expected. Then a
> > > second draw with a color buffer and the same depth buffer is done
> > to
> > > verify the depth buffer still has the original clear values.
> > >
> > > This behavior is not explicitly forbidden by the Vulkan spec, so
> > > it seems it is allowed.
> > >
> > > When there is no color attachment for a given output, we discard
> > it
> > > so at the end we have an FS assembly like:
> > >
> > > Native code for unnamed fragment shader (null)
> > > SIMD16 shader: 1 instructions. 0 loops. 4 cycles. 0:0
> > spills:fills.
> > > Promoted 0 constants. Compacted 16 to 16 bytes (0%)
> > > START B0 (4 cycles)
> > > sendc(16) null<1>UW g120<0,1,0>F 0x90031000
> > >
> > > render MsgDesc: RT write SIMD16 LastRT Surface = 0 mlen 8 rlen 0
> > {
> > > align1 1H EOT };
> > >
> > > As g120 is not initialized, we see random writes to the depth
> > buffer
> > > due to the alphaToCoverage enablement. This commit fixes that by
> > > keeping the output and creating a null render target for it.
> > >
> > > Fixes tests:
> > >
> > > dEQP-
> > VK.pipeline.multisample.alpha_to_coverage_no_color_attachment.*
> > >
> > > Signed-off-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
> > > ---
> > > src/intel/vulkan/anv_pipeline.c | 35 +++++++++++++++++++++++++
> > ----
> > > ----
> > > 1 file changed, 27 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/anv_pipeline.c
> > > b/src/intel/vulkan/anv_pipeline.c
> > > index e2024212bd9..70a958bf3a8 100644
> > > --- a/src/intel/vulkan/anv_pipeline.c
> > > +++ b/src/intel/vulkan/anv_pipeline.c
> > > @@ -792,7 +792,9 @@ anv_pipeline_compile_gs(const struct
> > brw_compiler
> > > *compiler,
> > >
> > > static void
> > > anv_pipeline_link_fs(const struct brw_compiler *compiler,
> > > - struct anv_pipeline_stage *stage)
> > > + struct anv_pipeline_stage *stage,
> > > + const struct anv_subpass *subpass,
> > > + const VkPipelineMultisampleStateCreateInfo
> > > *ms_info)
> > > {
> > > unsigned num_rts = 0;
> > > const int max_rt = FRAG_RESULT_DATA7 - FRAG_RESULT_DATA0 + 1;
> > > @@ -843,12 +845,28 @@ anv_pipeline_link_fs(const struct
> > brw_compiler
> > > *compiler,
> > > const unsigned rt = var->data.location -
> > FRAG_RESULT_DATA0;
> > > if (rt >= MAX_RTS ||
> > > !(stage->key.wm.color_outputs_valid & (1 << rt))) {
> > > - /* Unused or out-of-bounds, throw it away */
> > > - deleted_output = true;
> > > - var->data.mode = nir_var_function_temp;
> > > - exec_node_remove(&var->node);
> > > - exec_list_push_tail(&impl->locals, &var->node);
> > > - continue;
> > > + if (rt == 0 && ms_info && ms_info-
> > >alphaToCoverageEnable &&
> > > + subpass->depth_stencil_attachment &&
> > rt_to_bindings[rt]
> > > == -1) {
> > > + /* Don't throw away the unused output, because we
> > needed
> > > it for
> > > + * calculate correctly the alpha to coverage samples
> > > when there
> > > + * is no color buffer attached at location 0.
> > > + */
> > > + rt_to_bindings[rt] = num_rts;
> > > + /* We need a null render target */
> > > + rt_bindings[rt_to_bindings[rt]] = (struct
> > > anv_pipeline_binding) {
> > > + .set = ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS,
> > > + .binding = 0,
> > > + .index = UINT32_MAX,
> > > + };
> > > + num_rts++;
> > > + } else {
> > > + /* Unused or out-of-bounds, throw it away */
> > > + deleted_output = true;
> > > + var->data.mode = nir_var_function_temp;
> > > + exec_node_remove(&var->node);
> > > + exec_list_push_tail(&impl->locals, &var->node);
> > > + continue;
> > > + }
> > > }
> > >
> > > /* Give it the new location */
> > > @@ -1075,7 +1093,8 @@ anv_pipeline_compile_graphics(struct
> > > anv_pipeline *pipeline,
> > > anv_pipeline_link_gs(compiler, &stages[s], next_stage);
> > > break;
> > > case MESA_SHADER_FRAGMENT:
> > > - anv_pipeline_link_fs(compiler, &stages[s]);
> > > + anv_pipeline_link_fs(compiler, &stages[s],
> > > + pipeline->subpass, info-
> > > >pMultisampleState);
> > > break;
> > > default:
> > > unreachable("Invalid graphics shader stage");
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: This is a digitally signed message part
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
