<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">Seems reasonable. Only one comment which you can feel free to ignore.</div><div class="gmail_quote"> </div><div class="gmail_quote">Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div class="gmail_quote"> </div><div class="gmail_quote">On Fri, Jan 5, 2018 at 7:23 AM, Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Looks good to me too. Reviewed-by: Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.com</a>> Thanks a lot! - Lionel<div class="HOEnZb"><div class="h5"> On 05/01/18 09:19, Iago Toral wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Looks good to me, unless Jason or Lionel say otherwise: Reviewed-by: Iago Toral Quiroga <<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>> Iago On Thu, 2018-01-04 at 18:13 +0000, Alex Smith wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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 <<a href="mailto:asmith@feralinteractive.com" target="_blank">asmith@feralinteractive.com</a>> Cc: <a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank">mesa-stable@lists.freedesktop.org</a> --- 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- <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> bind_map; </blockquote> 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- <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> surface_to_descriptor[i]; </blockquote> + + 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) </blockquote></blockquote></div></div></blockquote><div> </div><div>At some point, we really should add a new #define for this and stop using UINT32_MAX. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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- <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> uses_kill) && </blockquote> - !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- <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> pMultisampleState); </blockquote> + 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- <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> pColorBlendState); </blockquote> emit_3dstate_vf_topology(pipeline); #endif emit_3dstate_vf_statistics(pipeline); </blockquote> _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote> _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div></div></blockquote></div> </div></div>