[PATCH] wined3d: Properly handle immediate NaN and Inf in GLSL backend.

Józef Kucia joseph.kucia at gmail.com
Tue Jun 14 03:09:21 CDT 2016


In my opinion, we do not want to solve the issue with immediate
constant buffers this way. However, this patch might still be useful.
Some games (Magicka 2) use shaders which contain NaN values in
immediate constants. This patch represents a one way to solve the
issue with NaN values in immediate constants. If we decide that we
want to apply this change we should be more careful though. We
probably want to use this for SM4+ shaders only, or use it only when
GL_ARB_shader_bit_encoding is available.

On a side note, "uintBitsToFloat" cannot be used as a constant
expression (at least in GLSL 1.30), so to make this work with the
immediate constant buffers patch you would have to drop "const".

On Mon, Jun 13, 2016 at 9:30 PM, Guillaume Charifi
<guillaume.charifi at sfr.fr> wrote:
> Signed-off-by: Guillaume Charifi <guillaume.charifi at sfr.fr>
> ---
>  dlls/wined3d/glsl_shader.c | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c
> index def224c..fde4c5e 100644
> --- a/dlls/wined3d/glsl_shader.c
> +++ b/dlls/wined3d/glsl_shader.c
> @@ -34,6 +34,8 @@
>
>  #include <limits.h>
>  #include <stdio.h>
> +#include <stdint.h>
> +#include <inttypes.h>

We avoid C99 in Wine. unsigned int and %#xu should be good enough.

>  #ifdef HAVE_FLOAT_H
>  # include <float.h>
>  #endif
> @@ -321,14 +323,26 @@ static const char *shader_glsl_get_version(const struct wined3d_gl_info *gl_info
>          return "#version 120";
>  }
>
> +/* Allow raw NaN and Inf in GLSL code. */
> +void wined3d_glsl_ftoa(float value, char *s)

The function should be static.

> +{
> +    if (!isfinite(value))
> +    {
> +        /* We need a buffer of at least 28 characters. */
> +        snprintf(s, 28, "uintBitsToFloat(%"PRIu32"u)", *(uint32_t *)&value);

It seems that 29 bytes are needed at least (including the null character).

> +    }
> +    else
> +        wined3d_ftoa(value, s);
> +}

In wined3d, generally, if one block in if-else statement needs curly
braces we use curly braces for all blocks in this statement.

else
{
    wined3d_ftoa(value, s);
}

> +
>  static void shader_glsl_append_imm_vec4(struct wined3d_string_buffer *buffer, const float *values)
>  {
> -    char str[4][17];
> +    char str[4][28];
>
> -    wined3d_ftoa(values[0], str[0]);
> -    wined3d_ftoa(values[1], str[1]);
> -    wined3d_ftoa(values[2], str[2]);
> -    wined3d_ftoa(values[3], str[3]);
> +    wined3d_glsl_ftoa(values[0], str[0]);
> +    wined3d_glsl_ftoa(values[1], str[1]);
> +    wined3d_glsl_ftoa(values[2], str[2]);
> +    wined3d_glsl_ftoa(values[3], str[3]);
>      shader_addline(buffer, "vec4(%s, %s, %s, %s)", str[0], str[1], str[2], str[3]);
>  }
>
> @@ -2299,7 +2313,7 @@ static void shader_glsl_get_register_name(const struct wined3d_shader_register *
>      const struct wined3d_gl_info *gl_info = ins->ctx->gl_info;
>      const char *prefix = shader_glsl_get_prefix(version->type);
>      struct glsl_src_param rel_param0, rel_param1;
> -    char imm_str[4][17];
> +    char imm_str[4][28];
>
>      if (reg->idx[0].offset != ~0U && reg->idx[0].rel_addr)
>          shader_glsl_add_src_param(ins, reg->idx[0].rel_addr, WINED3DSP_WRITEMASK_0, &rel_param0);
> @@ -2513,7 +2527,7 @@ static void shader_glsl_get_register_name(const struct wined3d_shader_register *
>                      switch (reg->data_type)
>                      {
>                          case WINED3D_DATA_FLOAT:
> -                            wined3d_ftoa(*(const float *)reg->immconst_data, register_name);
> +                            wined3d_glsl_ftoa(*(const float *)reg->immconst_data, register_name);
>                              break;
>                          case WINED3D_DATA_INT:
>                              sprintf(register_name, "%#x", reg->immconst_data[0]);
> @@ -2533,10 +2547,10 @@ static void shader_glsl_get_register_name(const struct wined3d_shader_register *
>                      switch (reg->data_type)
>                      {
>                          case WINED3D_DATA_FLOAT:
> -                            wined3d_ftoa(*(const float *)&reg->immconst_data[0], imm_str[0]);
> -                            wined3d_ftoa(*(const float *)&reg->immconst_data[1], imm_str[1]);
> -                            wined3d_ftoa(*(const float *)&reg->immconst_data[2], imm_str[2]);
> -                            wined3d_ftoa(*(const float *)&reg->immconst_data[3], imm_str[3]);
> +                            wined3d_glsl_ftoa(*(const float *)&reg->immconst_data[0], imm_str[0]);
> +                            wined3d_glsl_ftoa(*(const float *)&reg->immconst_data[1], imm_str[1]);
> +                            wined3d_glsl_ftoa(*(const float *)&reg->immconst_data[2], imm_str[2]);
> +                            wined3d_glsl_ftoa(*(const float *)&reg->immconst_data[3], imm_str[3]);
>                              sprintf(register_name, "vec4(%s, %s, %s, %s)",
>                                      imm_str[0], imm_str[1], imm_str[2], imm_str[3]);

It looks like shader_glsl_append_imm_vec4() could be called there instead.



More information about the wine-devel mailing list