[PATCH 0/3] MR3419: wined3d: Implement a zero-safe normalize function for FFP
DirectX's FFP normalizes the zero vector to the zero vector, but GLSL normalizes them to all NaN. This patch creates `ffp_normalize`, which normalizes vectors but has DirectX's behavior on 0 vectors. Further patches in this set switch other calls from GLSL's `normalize` to the 0-safe version. This fixes [36564](https://bugs.winehq.org/show_bug.cgi?id=36564). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3419
From: Petrichor Park <ppark(a)codeweavers.com> DirectX's FFP normalizes the zero vector to the zero vector, but GLSL normalizes them to all NaN. This patch creates `ffp_normalize`, which normalizes vectors but has DirectX's behavior on 0 vectors. Further patches in this set switch other calls from GLSL's `normalize` to the 0-safe version. This fixes [36564](https://bugs.winehq.org/show_bug.cgi?id=36564). --- dlls/wined3d/glsl_shader.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index 7dc0de1b596..67562d33205 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -9071,6 +9071,11 @@ static GLuint shader_glsl_generate_ffp_vertex_shader(struct shader_glsl_priv *pr declare_out_varying(gl_info, buffer, FALSE, "float ffp_varying_fogcoord;\n"); } + shader_addline(buffer, "\nvec3 ffp_normalize(vec3 n)\n{"); + shader_addline(buffer, " float lensq = dot(n, n);\n"); + shader_addline(buffer, " return lensq == 0.0 ? n : (n * inversesqrt(lensq));\n"); + shader_addline(buffer, "}"); + shader_addline(buffer, "\nvoid main()\n{\n"); shader_addline(buffer, "float m;\n"); shader_addline(buffer, "vec3 r;\n"); @@ -9132,7 +9137,7 @@ static GLuint shader_glsl_generate_ffp_vertex_shader(struct shader_glsl_priv *pr } if (settings->normalize) - shader_addline(buffer, "normal = normalize(normal);\n"); + shader_addline(buffer, "normal = ffp_normalize(normal);\n"); } shader_glsl_ffp_vertex_lighting(buffer, settings, legacy_lighting); @@ -9173,11 +9178,11 @@ static GLuint shader_glsl_generate_ffp_vertex_shader(struct shader_glsl_priv *pr case WINED3DTSS_TCI_CAMERASPACEREFLECTIONVECTOR: shader_addline(buffer, "ffp_varying_texcoord[%u] = ffp_texture_matrix[%u]" - " * vec4(reflect(normalize(ec_pos.xyz), normal), 1.0);\n", i, i); + " * vec4(reflect(ffp_normalize(ec_pos.xyz), normal), 1.0);\n", i, i); break; case WINED3DTSS_TCI_SPHEREMAP: - shader_addline(buffer, "r = reflect(normalize(ec_pos.xyz), normal);\n"); + shader_addline(buffer, "r = reflect(ffp_normalize(ec_pos.xyz), normal);\n"); shader_addline(buffer, "m = 2.0 * length(vec3(r.x, r.y, r.z + 1.0));\n"); shader_addline(buffer, "ffp_varying_texcoord[%u] = ffp_texture_matrix[%u]" " * vec4(r.x / m + 0.5, r.y / m + 0.5, 0.0, 1.0);\n", i, i); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3419
From: Petrichor Park <ppark(a)codeweavers.com> --- dlls/wined3d/glsl_shader.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index 67562d33205..67741e3fa81 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -8894,7 +8894,7 @@ static void shader_glsl_ffp_vertex_lighting(struct wined3d_string_buffer *buffer shader_addline(buffer, "}\n"); continue; } - shader_addline(buffer, "dir = normalize(dir);\n"); + shader_addline(buffer, "dir = ffp_normalize(dir);\n"); shader_glsl_ffp_vertex_lighting_footer(buffer, settings, idx, legacy_lighting); shader_addline(buffer, "}\n"); } @@ -8915,8 +8915,8 @@ static void shader_glsl_ffp_vertex_lighting(struct wined3d_string_buffer *buffer { shader_addline(buffer, "if (dst.y <= ffp_light[%u].range)\n{\n", idx); } - shader_addline(buffer, "dir = normalize(dir);\n"); - shader_addline(buffer, "t = dot(-dir, normalize(ffp_light[%u].direction));\n", idx); + shader_addline(buffer, "dir = ffp_normalize(dir);\n"); + shader_addline(buffer, "t = dot(-dir, ffp_normalize(ffp_light[%u].direction));\n", idx); shader_addline(buffer, "if (t > ffp_light[%u].cos_htheta) att = 1.0;\n", idx); shader_addline(buffer, "else if (t <= ffp_light[%u].cos_hphi) att = 0.0;\n", idx); shader_addline(buffer, "else att = pow((t - ffp_light[%u].cos_hphi)" @@ -8946,7 +8946,7 @@ static void shader_glsl_ffp_vertex_lighting(struct wined3d_string_buffer *buffer if (!settings->normal) continue; shader_addline(buffer, "att = 1.0;\n"); - shader_addline(buffer, "dir = normalize(ffp_light[%u].direction.xyz);\n", idx); + shader_addline(buffer, "dir = ffp_normalize(ffp_light[%u].direction.xyz);\n", idx); shader_glsl_ffp_vertex_lighting_footer(buffer, settings, idx, legacy_lighting); } @@ -8956,7 +8956,7 @@ static void shader_glsl_ffp_vertex_lighting(struct wined3d_string_buffer *buffer if (!settings->normal) continue; shader_addline(buffer, "att = 1.0;\n"); - shader_addline(buffer, "dir = normalize(ffp_light[%u].position.xyz);\n", idx); + shader_addline(buffer, "dir = ffp_normalize(ffp_light[%u].position.xyz);\n", idx); shader_glsl_ffp_vertex_lighting_footer(buffer, settings, idx, legacy_lighting); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3419
From: Petrichor Park <ppark(a)codeweavers.com> --- dlls/wined3d/glsl_shader.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index 67741e3fa81..181472c36f7 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -8835,9 +8835,9 @@ static void shader_glsl_ffp_vertex_lighting_footer(struct wined3d_string_buffer shader_addline(buffer, "diffuse += clamp(dot(dir, normal), 0.0, 1.0)" " * ffp_light[%u].diffuse.xyz * att;\n", idx); if (settings->localviewer) - shader_addline(buffer, "t = dot(normal, normalize(dir - normalize(ec_pos.xyz)));\n"); + shader_addline(buffer, "t = dot(normal, ffp_normalize(dir - ffp_normalize(ec_pos.xyz)));\n"); else - shader_addline(buffer, "t = dot(normal, normalize(dir + vec3(0.0, 0.0, -1.0)));\n"); + shader_addline(buffer, "t = dot(normal, ffp_normalize(dir + vec3(0.0, 0.0, -1.0)));\n"); shader_addline(buffer, "if (dot(dir, normal) > 0.0 && t > 0.0%s) specular +=" " pow(t, ffp_material.shininess) * ffp_light[%u].specular * att;\n", legacy_lighting ? " && ffp_material.shininess > 0.0" : "", idx); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3419
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3419
Not sure if it matters much, but the commit messages are missing the `wined3d:` prefix and the last two commits are missing a period. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3419#note_40392
```diff + shader_addline(buffer, "\nvec3 ffp_normalize(vec3 n)\n{"); + shader_addline(buffer, " float lensq = dot(n, n);\n"); + shader_addline(buffer, " return lensq == 0.0 ? n : (n * inversesqrt(lensq));\n"); + shader_addline(buffer, "}"); ```
This is missing newlines after "{" and "}". It'll still work, of course, but the generated code will be slightly ugly.
``` This fixes [36564](https://bugs.winehq.org/show_bug.cgi?id=36564). ```
We normally use "Wine-Bug: " for those. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3419#note_40393
Commit messages seem to be formatted with Markdown, which doesn't seem to be a standard assumption. Neither `git` commands (which just print them verbatim to the terminal) nor GitLab parse them with Markdown, so I'd avoid that style (this applies both to backticks and links). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3419#note_40453
participants (6)
-
Gijs Vermeulen (＠gverm) -
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Petrichor Park
-
Petrichor Park (＠petrathekat)
-
Zebediah Figura (＠zfigura)