Looks good to me, unless Jason or Lionel say otherwise:
Reviewed-by: Iago Toral Quiroga <ito...@igalia.com>
Iago
On Thu, 2018-01-04 at 18:13 +0000, Alex Smith wrote:
> If we have a color attachment, but its writes are masked, this would
> have still returned true. This is inconsistent with how
> HasWriteableRT
> in 3DSTATE_PS_BLEND is set, which does take the mask into account.
>
> This could lead to PixelShaderHasUAV not being set in
> 3DSTATE_PS_EXTRA
> if the fragment shader does use UAVs, meaning the fragment shader may
> not be invoked because HasWriteableRT is false. Specifically, this
> was
> seen to occur when the shader also enables early fragment tests: the
> fragment shader was not invoked despite passing depth/stencil.
>
> Fix by taking the color write mask into account in this function.
> This
> is consistent with how things are done on i965.
>
> Signed-off-by: Alex Smith <asm...@feralinteractive.com>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
> src/intel/vulkan/genX_pipeline.c | 27 ++++++++++++++++++---------
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/src/intel/vulkan/genX_pipeline.c
> b/src/intel/vulkan/genX_pipeline.c
> index 0ae9ead587..cfc3bea426 100644
> --- a/src/intel/vulkan/genX_pipeline.c
> +++ b/src/intel/vulkan/genX_pipeline.c
> @@ -1330,7 +1330,8 @@ emit_3dstate_gs(struct anv_pipeline *pipeline)
> }
>
> static bool
> -has_color_buffer_write_enabled(const struct anv_pipeline *pipeline)
> +has_color_buffer_write_enabled(const struct anv_pipeline *pipeline,
> + const
> VkPipelineColorBlendStateCreateInfo *blend)
> {
> const struct anv_shader_bin *shader_bin =
> pipeline->shaders[MESA_SHADER_FRAGMENT];
> @@ -1339,10 +1340,15 @@ has_color_buffer_write_enabled(const struct
> anv_pipeline *pipeline)
>
> const struct anv_pipeline_bind_map *bind_map = &shader_bin-
> >bind_map;
> for (int i = 0; i < bind_map->surface_count; i++) {
> - if (bind_map->surface_to_descriptor[i].set !=
> - ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS)
> + struct anv_pipeline_binding *binding = &bind_map-
> >surface_to_descriptor[i];
> +
> + if (binding->set != ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS)
> continue;
> - if (bind_map->surface_to_descriptor[i].index != UINT32_MAX)
> +
> + const VkPipelineColorBlendAttachmentState *a =
> + &blend->pAttachments[binding->index];
> +
> + if (binding->index != UINT32_MAX && a->colorWriteMask != 0)
> return true;
> }
>
> @@ -1351,6 +1357,7 @@ has_color_buffer_write_enabled(const struct
> anv_pipeline *pipeline)
>
> static void
> emit_3dstate_wm(struct anv_pipeline *pipeline, struct anv_subpass
> *subpass,
> + const VkPipelineColorBlendStateCreateInfo *blend,
> const VkPipelineMultisampleStateCreateInfo
> *multisample)
> {
> const struct brw_wm_prog_data *wm_prog_data =
> get_wm_prog_data(pipeline);
> @@ -1395,7 +1402,7 @@ emit_3dstate_wm(struct anv_pipeline *pipeline,
> struct anv_subpass *subpass,
> if (wm.PixelShaderComputedDepthMode != PSCDEPTH_OFF ||
> wm_prog_data->has_side_effects ||
> wm.PixelShaderKillsPixel ||
> - has_color_buffer_write_enabled(pipeline))
> + has_color_buffer_write_enabled(pipeline, blend))
> wm.ThreadDispatchEnable = true;
>
> if (samples > 1) {
> @@ -1520,7 +1527,8 @@ emit_3dstate_ps(struct anv_pipeline *pipeline,
> #if GEN_GEN >= 8
> static void
> emit_3dstate_ps_extra(struct anv_pipeline *pipeline,
> - struct anv_subpass *subpass)
> + struct anv_subpass *subpass,
> + const VkPipelineColorBlendStateCreateInfo
> *blend)
> {
> const struct brw_wm_prog_data *wm_prog_data =
> get_wm_prog_data(pipeline);
>
> @@ -1575,7 +1583,7 @@ emit_3dstate_ps_extra(struct anv_pipeline
> *pipeline,
> * attachments, we need to force-enable here.
> */
> if ((wm_prog_data->has_side_effects || wm_prog_data-
> >uses_kill) &&
> - !has_color_buffer_write_enabled(pipeline))
> + !has_color_buffer_write_enabled(pipeline, blend))
> ps.PixelShaderHasUAV = true;
>
> #if GEN_GEN >= 9
> @@ -1705,10 +1713,11 @@ genX(graphics_pipeline_create)(
> emit_3dstate_hs_te_ds(pipeline, pCreateInfo->pTessellationState);
> emit_3dstate_gs(pipeline);
> emit_3dstate_sbe(pipeline);
> - emit_3dstate_wm(pipeline, subpass, pCreateInfo-
> >pMultisampleState);
> + emit_3dstate_wm(pipeline, subpass, pCreateInfo->pColorBlendState,
> + pCreateInfo->pMultisampleState);
> emit_3dstate_ps(pipeline, pCreateInfo->pColorBlendState);
> #if GEN_GEN >= 8
> - emit_3dstate_ps_extra(pipeline, subpass);
> + emit_3dstate_ps_extra(pipeline, subpass, pCreateInfo-
> >pColorBlendState);
> emit_3dstate_vf_topology(pipeline);
> #endif
> emit_3dstate_vf_statistics(pipeline);
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
