[Mesa-dev] [PATCH] anv/pipeline: do not disable depth writes if depth testing is disabled

[Iago Toral]

On Mon, 2017-04-03 at 08:55 -0700, Nanley Chery wrote:
> On Mon, Apr 03, 2017 at 08:02:54AM +0200, Iago Toral wrote:
> > 
> > Can anyone review this one?
> > 
> > On Wed, 2017-03-29 at 08:58 +0200, Iago Toral Quiroga wrote:
> > > 
> > > Writing and testing are two different things and they can be set
> > > separately
> > > by the application. If an application wants to record depth data
> > > without
> > > caring for the depth test, it can enable depth test and set the
> > > depth
> > > compare function to VK_COMPARE_OP_ALWAYS or it can simply disable
> > > depth testing altogether. Some CTS tests do the latter.
> > > 
> Doesn't disabling the depth test prevent any updates to the depth
> buffer?
> 
> Section 25.10 Depth Test from the Vulkan spec says:
> 
>    When disabled, the depth comparison and subsequent possible
> updates
>    to the value of the depth component of the depth/stencil
> attachment
>    are bypassed and the fragment is passed to the next operation.
> 
> -Nanley

Right, the spec text is quite clear. To be honest, it didn't occur to
me that the API would allow to set these two things separately and then
the spec would say that one of them is actually ignored depending on
the setting for the other... anyway, I filed an issue to Khronos so
they can confirm that this is the intended behavior and hopefully
improve the documentation of VkPipelineDepthStencilStateCreateInfo to
make this clear there.

Thanks for catching this and sorry for the noise.

Iago

> > 
> > > 
> > > Fixes all multisample tests with depth-only formats in:
> > > dEQP-VK.renderpass.multisample.*
> > > ---
> > >  src/intel/vulkan/genX_pipeline.c | 4 ----
> > >  1 file changed, 4 deletions(-)
> > > 
> > > diff --git a/src/intel/vulkan/genX_pipeline.c
> > > b/src/intel/vulkan/genX_pipeline.c
> > > index 85a9e4f..dc393cb 100644
> > > --- a/src/intel/vulkan/genX_pipeline.c
> > > +++ b/src/intel/vulkan/genX_pipeline.c
> > > @@ -728,10 +728,6 @@
> > > sanitize_ds_state(VkPipelineDepthStencilStateCreateInfo *state,
> > >  {
> > >     *stencilWriteEnable = state->stencilTestEnable;
> > >  
> > > -   /* If the depth test is disabled, we won't be writing
> > > anything.
> > > */
> > > -   if (!state->depthTestEnable)
> > > -      state->depthWriteEnable = false;
> > > -
> > >     /* The Vulkan spec requires that if either depth or stencil
> > > is
> > > not present,
> > >      * the pipeline is to act as if the test silently passes.
> > >      */
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev