On Mon, 2018-04-02 at 09:31 -0700, Jason Ekstrand wrote:
>
> On April 2, 2018 04:04:49 Iago Toral Quiroga <ito...@igalia.com>
> wrote:
>
> From the SPIR-V spec, OpTypeImage:
>
> "Depth is whether or not this image is a depth image. (Note that
> whether or not depth comparisons are actually done is a property of
> the sampling opcode, not of this type declaration.)"
>
> OpImageSample{Proj}Dref{Explicit,Implicit}Lod sampling opcodes always
> do
> a depth comparison, regardless of the Depth property of the image
> type,
> and as stated in the spec quote, this should be honored. For us, it
> means
> that we always have to handle them as shadow sampling opcodes.
>
> Fixes crashes in:
> dEQP-
> VK.spirv_assembly.instruction.graphics.image_sampler.depth_property.*
> ---
> src/compiler/spirv/spirv_to_nir.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> index 72ab426e80..f968c4d387 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -1908,7 +1908,22 @@ vtn_handle_texture(struct vtn_builder *b,
> SpvOp opcode,
> const struct glsl_type *image_type = sampled.type->type;
> const enum glsl_sampler_dim sampler_dim =
> glsl_get_sampler_dim(image_type);
> const bool is_array = glsl_sampler_type_is_array(image_type);
> - const bool is_shadow = glsl_sampler_type_is_shadow(image_type);
> +
> + bool is_shadow;
> + switch (opcode) {
> + /* These opcodes are always depth-comparitors regardless of the
> depth
> + * property of the image
> + */
> + case SpvOpImageSampleDrefImplicitLod:
> + case SpvOpImageSampleProjDrefImplicitLod:
> + case SpvOpImageSampleDrefExplicitLod:
> + case SpvOpImageSampleProjDrefExplicitLod:
> + is_shadow = true;
> + break;
> + default:
> + is_shadow = glsl_sampler_type_is_shadow(image_type);
>
> Given the sure quote above, should this just be false?
Yes, I think you're right. I have just tested a shadow mapping demo I
have around and verified that GLSLang produces these opcodes for shadow
samplers, so I think it should be safe to do this change. I'll run it
through Jenkins and send a new patch if that doesn't find anything
suspicious.
> Also, don't we
> already have a switch for the Dref source that we can use instead of
> making
> a new one?
Yes, we can use other switches to do this, although we will need to use
a fallthrough, but I guess that's okay.
>
> + break;
> + }
>
> /* Figure out the base texture operation */
> nir_texop texop;
> --
> 2.14.1
>
>
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
