Re: [PATCH] wined3d: Implement SM5 f32tof16 opcode in glsl.

[Józef Kucia]

A test would be nice to have.

On Fri, Jul 15, 2016 at 1:37 PM, Guillaume Charifi
<guillaume.charifi(a)sfr.fr> wrote:
> Signed-off-by: Guillaume Charifi <guillaume.charifi(a)sfr.fr>
> ---
>  dlls/wined3d/arb_program_shader.c |  1 +
>  dlls/wined3d/glsl_shader.c        | 38 ++++++++++++++++++++++++++++++++++++++
>  dlls/wined3d/shader_sm4.c         |  2 ++
>  dlls/wined3d/wined3d_private.h    |  1 +
>  4 files changed, 42 insertions(+)
>
> diff --git a/dlls/wined3d/arb_program_shader.c b/dlls/wined3d/arb_program_shader.c
> index 24c1194..9aa4252 100644
> --- a/dlls/wined3d/arb_program_shader.c
> +++ b/dlls/wined3d/arb_program_shader.c
> @@ -5286,6 +5286,7 @@ static const SHADER_HANDLER shader_arb_instruction_handler_table[WINED3DSIH_TABL
>      /* WINED3DSIH_EQ                               */ NULL,
>      /* WINED3DSIH_EXP                              */ shader_hw_scalar_op,
>      /* WINED3DSIH_EXPP                             */ shader_hw_scalar_op,
> +    /* WINED3DSIH_F32TOF16                         */ NULL,
>      /* WINED3DSIH_FRC                              */ shader_hw_map2gl,
>      /* WINED3DSIH_FTOI                             */ NULL,
>      /* WINED3DSIH_FTOU                             */ NULL,

You need to add WINED3DSIH_F32TOF16 in other places as well, e.g. in shader.c.
> +            shader_addline(ins->ctx->buffer, "unpackHalf2x16(floatBitsToUint(%s)).x", src_param.param_str);

You shouldn't need floatBitsToUint. shader_glsl_add_src_param() takes
care about it when you set a proper type for the parameter.
> +            *dst_mask_ptr++ = dst_masks[i];
> +        } else
> +            shader_addline(ins->ctx->buffer, "0");
> +
> +        if(i != NB_COMPS - 1)
> +            shader_addline(ins->ctx->buffer, ",");
> +        shader_addline(ins->ctx->buffer, "\n");
> +    }
> +
> +    *dst_mask_ptr++ = '\0';

You shouldn't need to construct dst_mask manually.
> +    shader_addline(ins->ctx->buffer, ")%s);\n", dst_mask);
> +#undef NB_COMPS
> +}
> +
>  static void shader_glsl_else(const struct wined3d_shader_instruction *ins)
>  {
>      shader_addline(ins->ctx->buffer, "} else {\n");
> @@ -8704,6 +8741,7 @@ static const SHADER_HANDLER shader_glsl_instruction_handler_table[WINED3DSIH_TAB
>      /* WINED3DSIH_EQ                               */ shader_glsl_relop,
>      /* WINED3DSIH_EXP                              */ shader_glsl_scalar_op,
>      /* WINED3DSIH_EXPP                             */ shader_glsl_expp,
> +    /* WINED3DSIH_F32TOF16                         */ shader_glsl_f32tof16,
>      /* WINED3DSIH_FRC                              */ shader_glsl_map2gl,
>      /* WINED3DSIH_FTOI                             */ shader_glsl_to_int,
>      /* WINED3DSIH_FTOU                             */ shader_glsl_to_uint,
> diff --git a/dlls/wined3d/shader_sm4.c b/dlls/wined3d/shader_sm4.c
> index c8fdb32..6c15751 100644
> --- a/dlls/wined3d/shader_sm4.c
> +++ b/dlls/wined3d/shader_sm4.c
> @@ -215,6 +215,7 @@ enum wined3d_sm4_opcode
>      WINED3D_SM5_OP_DERIV_RTY_COARSE                 = 0x7c,
>      WINED3D_SM5_OP_DERIV_RTY_FINE                   = 0x7d,
>      WINED3D_SM5_OP_GATHER4_C                        = 0x7e,
> +    WINED3D_SM5_OP_F32TOF16                         = 0x82,
>      WINED3D_SM5_OP_BFI                              = 0x8c,
>      WINED3D_SM5_OP_BFREV                            = 0x8d,
>      WINED3D_SM5_OP_SWAPC                            = 0x8e,
> @@ -811,6 +812,7 @@ static const struct wined3d_sm4_opcode_info opcode_table[] =
>      {WINED3D_SM5_OP_DERIV_RTY_COARSE,                 WINED3DSIH_DSY_COARSE,                       "f",    "f"},
>      {WINED3D_SM5_OP_DERIV_RTY_FINE,                   WINED3DSIH_DSY_FINE,                         "f",    "f"},
>      {WINED3D_SM5_OP_GATHER4_C,                        WINED3DSIH_GATHER4_C,                        "f",    "fRSf"},
> +    {WINED3D_SM5_OP_F32TOF16,                         WINED3DSIH_F32TOF16,                         "f",    "f"},

You should use "u" for the source parameters.

Similar comments as to other patches also applies.