<div dir="ltr"> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Mar 14, 2019 at 7:08 PM Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Eleni Maria Stea <<a href="mailto:estea@igalia.com" target="_blank">estea@igalia.com</a>> Added support for setting the locations when the pipeline has been created with the dynamic state bit enabled according to the Vulkan Specification section [26.5. Custom Sample Locations] for the function: 'vkCmdSetSampleLocationsEXT' The reason that we preferred to store the boolean valid inside the dynamic state struct for locations instead of using a dirty bit (ANV_CMD_DIRTY_SAMPLE_LOCATIONS for example) is that other functions can modify the value of the dirty bits causing unexpected behavior. v2: Removed all the anv* structs used with sample locations to store the locations in order for dynamic case. (see also the patch for the non-dynamic case. (Jason Ekstrand) --- src/intel/vulkan/anv_cmd_buffer.c | 17 +++++++++++++++++ src/intel/vulkan/anv_private.h | 6 ++++++ src/intel/vulkan/genX_cmd_buffer.c | 11 +++++++++++ 3 files changed, 34 insertions(+) diff --git a/src/intel/vulkan/anv_cmd_buffer.c b/src/intel/vulkan/anv_cmd_buffer.c index 1b34644a434..7f31300857b 100644 --- a/src/intel/vulkan/anv_cmd_buffer.c +++ b/src/intel/vulkan/anv_cmd_buffer.c @@ -558,6 +558,23 @@ void anv_CmdSetStencilReference( cmd_buffer->state.gfx.dirty |= ANV_CMD_DIRTY_DYNAMIC_STENCIL_REFERENCE; } +void +anv_CmdSetSampleLocationsEXT(VkCommandBuffer commandBuffer, + const VkSampleLocationsInfoEXT *pSampleLocationsInfo) +{ + ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer); + + struct anv_dynamic_state *dyn_state = &cmd_buffer->state.gfx.dynamic; + uint32_t samples = pSampleLocationsInfo->sampleLocationsPerPixel; + + assert(pSampleLocationsInfo); + dyn_state->sample_locations.samples = samples; + typed_memcpy(dyn_state->sample_locations.locations, + pSampleLocationsInfo->pSampleLocations, samples); + + dyn_state->sample_locations.valid = true; +} + static void anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer *cmd_buffer, VkPipelineBindPoint bind_point, diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index a39195733cd..94c9db14028 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -2124,6 +2124,12 @@ struct anv_dynamic_state { uint32_t front; uint32_t back; } stencil_reference; + + struct { + bool valid; + uint32_t samples; + VkSampleLocationEXT locations[MAX_SAMPLE_LOCATIONS]; + } sample_locations; }; extern const struct anv_dynamic_state default_dynamic_state; diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c index 7687507e6b7..5d2b17cf8ae 100644 --- a/src/intel/vulkan/genX_cmd_buffer.c +++ b/src/intel/vulkan/genX_cmd_buffer.c @@ -2796,6 +2796,17 @@ genX(cmd_buffer_flush_state)(struct anv_cmd_buffer *cmd_buffer) ANV_CMD_DIRTY_RENDER_TARGETS)) gen7_cmd_buffer_emit_scissor(cmd_buffer); + if (cmd_buffer->state.gfx.dynamic.sample_locations.valid) { + uint32_t samples = cmd_buffer->state.gfx.dynamic.sample_locations.samples; + const VkSampleLocationEXT *locations = + cmd_buffer->state.gfx.dynamic.sample_locations.locations; + genX(emit_multisample)(&cmd_buffer->batch, samples, locations); +#if GEN_GEN >= 8 + genX(emit_sample_pattern)(&cmd_buffer->batch, samples, locations); +#endif + cmd_buffer->state.gfx.dynamic.sample_locations.valid = false; </blockquote><div> </div><div>I'm not sure this is actually going to be correct. With dynamic state, you're required to set it before you use it. With pipeline state, it gets set every time the pipeline is bound. Effectively, the pipeline state is a big bag of dynamic state. With both of these, however, there are no defaults and you're required to bind a pipeline containing the state or explicitly set it on the command buffer before it gets used. VK_EXT_sample_locations is different though because it does have a default. So the question I'm coming around to is: When does the default get applied? The only sensible thing I can think of is at the top of the command buffer or maybe the top of the subpass. If this is the case, then we need to emit sample positions at the start of every subpass. Does the spec talk about this at all?</div><div> </div><div>--Jason </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote></div></div>