[PATCH v4 3/4] wined3d: Implement Fetch4 GLSL shader generation

[Stefan Dösinger]

Hi,

Am 05.02.19 um 01:19 schrieb Daniel Ansorregui:

> +    if(sample_flags & WINED3D_GLSL_SAMPLE_GATHER)
Formatting. "if(" -> "if ("

> +    /* Fetch4 overwrites the other texture flags */
> +    if (priv->cur_ps_args->fetch4 & (1u << resource_idx))
> +    {
> +        sample_flags |= WINED3D_GLSL_SAMPLE_GATHER;
> +        mask = 0;
> +    }
I am probably missing something, but the sample flags logical OR doesn't
quite match up with the comment. Is the comment outdated from previous
Intel-only behavior?

>      if (shader_version < WINED3D_SHADER_VERSION(2,0)) swizzle = WINED3DSP_NOSWIZZLE;
> +    else if (sample_flags & WINED3D_GLSL_SAMPLE_GATHER) swizzle = WINED3DSP_FETCH4_SWIZZLE;
>      else swizzle = ins->src[1].swizzle;
I have some doubts here. This would mean 1.x pixel shaders have a fetch4
swizzle that is different from fixed function and shader model 2.0+.
Shader model 2.0 also allows specifying a post-lookup swizzle on the
sampler (texld r0, s0.zwxz, v3 for sample will put the texture's blue
channel into .x and .w, the alpha into .y and red in .z). I suspect you
need to merge WINED3DSP_FETCH4_SWIZZLE with ins->src[1].swizzle. I think
we should have a function for that, but right now I can only find
shader_glsl_swizzle_get_component.

It is quite possible that the sampler swizzle is ignored by windows
drivers if fetch4, considering how many other texture fetch properties
are ignored.

The same applies to the other functions you modify of course, like
shader_glsl_texldd.

> +    if(priv->cur_ps_args && priv->cur_ps_args->fetch4 & (1u << sampler_idx))
> +    {
> +        flags = WINED3D_GLSL_SAMPLE_GATHER;
> +    }
Formatting

> +    if (flags == WINED3D_GLSL_SAMPLE_GATHER){
Formatting

> +        swizzle = WINED3DSP_FETCH4_SWIZZLE;
> +        shader_glsl_gen_sample_code(ins, sampler_idx, &sample_function, swizzle, flags,
> +                NULL, NULL, NULL, NULL, "%s", coord_param.param_str);
> +        shader_glsl_release_sample_function(ins->ctx, &sample_function);
> +        return;
> +    }
> +
> +    shader_glsl_get_sample_function(ins->ctx, sampler_idx, sampler_idx, WINED3D_GLSL_SAMPLE_LOD, &sample_function);
> +    shader_glsl_add_src_param(ins, &ins->src[0], sample_function.coord_mask, &coord_param);
> +    shader_glsl_add_src_param(ins, &ins->src[0], WINED3DSP_WRITEMASK_3, &lod_param);
> +    shader_glsl_gen_sample_code(ins, sampler_idx, &sample_function, swizzle, flags, NULL, NULL,
> +            lod_param.param_str, NULL, "%s", coord_param.param_str);
>      shader_glsl_release_sample_function(ins->ctx, &sample_function);
>  }
>  
> @@ -6175,6 +6238,7 @@ static void shader_glsl_ld(const struct wined3d_shader_instruction *ins)
>      struct glsl_sample_function sample_function;
>      DWORD flags = WINED3D_GLSL_SAMPLE_LOAD;
>      BOOL has_lod_param;
> +    const struct shader_glsl_ctx_priv *priv = ins->ctx->backend_data;
>  
>      if (wined3d_shader_instruction_has_texel_offset(ins))
>          flags |= WINED3D_GLSL_SAMPLE_OFFSET;
> @@ -6189,6 +6253,10 @@ static void shader_glsl_ld(const struct wined3d_shader_instruction *ins)
>      }
>      has_lod_param = is_mipmapped(reg_maps->resource_info[resource_idx].type);
>  
> +    if (priv->cur_ps_args && priv->cur_ps_args->fetch4 & (1u << resource_idx)){
> +        FIXME("Unsupported FETCH4 and LD Sampling SM 5.0");
> +    }
Formatting. This case is unlikely to be hit, but I guess not impossible
if a game really tries to set this magic LOD value in d3d10/11.

> @@ -3833,10 +3849,8 @@ static void PRINTF_ATTR(9, 10) shader_glsl_gen_sample_code(const struct wined3d_
>          if (!string_buffer_resize(ins->ctx->buffer, ret))
>              break;
>      }
> -
>      if (np2_fixup)
>      {
Is the removed newline intentional?


> @@ -6214,46 +6282,57 @@ static void shader_glsl_sample(const struct wined3d_shader_instruction *ins)
> ...
> +    if (priv->cur_ps_args->fetch4 & (1u << sampler_idx))
> +    {
> +        flags = WINED3D_GLSL_SAMPLE_GATHER;
> +        swizzle = WINED3DSP_FETCH4_SWIZZLE;
> +    }
Afaics the WINED3DSIH_SAMPLE* opcodes are shader model 4+ only, so I
don't think you have to handle fetch4 here. Either write a fixme like in
shader_glsl_ld or ignore it entirely.

> @@ -6588,7 +6668,7 @@ static void shader_glsl_texm3x2tex(const struct wined3d_shader_instruction *ins)
>      shader_glsl_get_sample_function(ins->ctx, reg, reg, 0, &sample_function);
This function and the following ones could in theory be used with FETCH4
- they are pixel shader 1.x specific. I'm happy with a FIXME. Those
functions are somewhat confusing and special because they need to be
preceeded by one or more texm3x3pad instructions.

> diff --git a/dlls/wined3d/shader.c b/dlls/wined3d/shader.c
> index e11a37cf07..6e21444f8c 100644
> --- a/dlls/wined3d/shader.c
> +++ b/dlls/wined3d/shader.c
> @@ -4027,6 +4027,19 @@ void find_ps_compile_args(const struct wined3d_state *state, const struct wined3
>          }
>      }
>  
> +    if (gl_info->supported[ARB_TEXTURE_GATHER])
> +    {
I'm not a fan of checking the (rather static) GL capabilities in this
performance critical codepath. I think it would be better to remove
WINED3DFMT_FLAG_ALLOW_FETCH4 from all formats during adapter creation if
the texture is not supported. That would allow you to remove not only
this but also other gl_info->supported[ARB_TEXTURE_GATHER] in this patch
and patch 2.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20190207/1886039c/attachment-0001.sig>