[PATCH 0/1] MR8666: Revert "wined3d/glsl: Transpose the bump environment matrix."
From: Elizabeth Figura <zfigura(a)codeweavers.com> This reverts commit ed62b8bf30ac926ae100eee42fee3f6ceb8541e2. GL_TRUE here signifies that the matrix is in row-major storage, and we do indeed pass it in row-major order here (like other d3d matrices). The problem is that this matrix, like other d3d matrices, is not only row-major in storage, it also uses what Sean Middleditch [1] calls "row-major notation". Contrary to GL, Vulkan, and almost the entire world of mathematics, Direct3D matrices have the *column* in their first component. A matrix which is row-major in both storage and notation is effectively identical, at least in storage order, to one which is column-major in both storage and notation, as here. That is, it should *not* be transposed. Adding to the confusion is the fact that, as mentioned by the commit being reverted (as well as texbem_test() in d3d9:visual), some drivers get this wrong too, and transpose the matrix. I originally thought that NVidia was broken, but it turns out that AMD is the one in the wrong here, at least for the BEM and TEXBEM(L) shader instructions. Incidentally, WARP incorrectly transposes the matrix for TEXBEM(L) but not for BEM. This fixes test failures in texbem_test() and fixed_function_bumpmap_test(). [1] https://web.archive.org/web/20180419095941/http://seanmiddleditch.com/matric... --- dlls/wined3d/glsl_shader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index c0071dcdfaa..e7b1d817314 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -1954,7 +1954,7 @@ static void shader_glsl_load_constants(struct shader_glsl_priv *priv, if (prog->ps.bumpenv_mat_location[i] == -1) continue; - GL_EXTCALL(glUniformMatrix2fv(prog->ps.bumpenv_mat_location[i], 1, GL_TRUE, + GL_EXTCALL(glUniformMatrix2fv(prog->ps.bumpenv_mat_location[i], 1, GL_FALSE, &constants->bumpenv.matrices[i]._00)); if (prog->ps.bumpenv_lum_scale_location[i] != -1) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8666
The really baffling thing is that I know I tested this, and saw no change. I incorrectly read the tests as being too coarse to actually validate that the matrix wasn't being transposed. They're not, and I don't know where I messed up. My best guess is that I failed to properly compile wined3d.dll before running the tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8666#note_111410
This merge request was approved by Jan Sikorski. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8666
participants (3)
-
Elizabeth Figura -
Elizabeth Figura (＠zfigura)
-
Jan Sikorski (＠jsikorski)