[PATCH 0/1] MR7355: wined3d: Handle a NULL "push_constants" buffer in glsl_fragment_pipe_alpha_test_func().

[PATCH 1/1] wined3d: Handle a NULL "push_constants" buffer in glsl_fragment_pipe_alpha_test_func().

[Henri Verbeet]

From: Henri Verbeet

If WINED3D_RS_ALPHAREF was never set, the corresponding "push_constants"
buffer may not exist either. We don't call
glsl_fragment_pipe_alpha_test_func() when using core contexts, which is
the reason it works there. This fixes a regression introduced by commit
20fb590cfbbcc54f13cd0e2d08da4742ecdef760. Found by running the regression tests.
---
 dlls/wined3d/glsl_shader.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c
index df60415f062..000bf0d3995 100644
--- a/dlls/wined3d/glsl_shader.c
+++ b/dlls/wined3d/glsl_shader.c
@@ -12155,14 +12155,19 @@ static void glsl_fragment_pipe_tex_transform(struct wined3d_context *context,
 static void glsl_fragment_pipe_alpha_test_func(struct wined3d_context_gl *context_gl,
         const struct wined3d_state *state)
 {
-    const struct wined3d_ffp_ps_constants *constants = wined3d_buffer_load_sysmem(
-            context_gl->c.device->push_constants[WINED3D_PUSH_CONSTANTS_PS_FFP], &context_gl->c);
     const struct wined3d_gl_info *gl_info = context_gl->gl_info;
-    GLint func = wined3d_gl_compare_func(state->render_states[WINED3D_RS_ALPHAFUNC]);
+    const struct wined3d_ffp_ps_constants *constants;
+    struct wined3d_buffer *buffer;
+    GLfloat ref;
+    GLint func;

-    if (func)
+    if ((func = wined3d_gl_compare_func(state->render_states[WINED3D_RS_ALPHAFUNC])))
     {
-        gl_info->gl_ops.gl.p_glAlphaFunc(func, constants->alpha_test_ref);
+        if ((buffer = context_gl->c.device->push_constants[WINED3D_PUSH_CONSTANTS_PS_FFP]))
+            ref = (constants = wined3d_buffer_load_sysmem(buffer, &context_gl->c))->alpha_test_ref;
+        else
+            ref = 0.0f;
+        gl_info->gl_ops.gl.p_glAlphaFunc(func, ref);
         checkGLcall("glAlphaFunc");
     }
 }

[Elizabeth Figura (＠zfigura)]

> Found by running the regression tests.

Huh, where? The GL tests pass for me with master...

The intended solution here (which may not be the best one, I don't know) is that any draw that uses a stateblock should have set these constants. This is done by invalidating stateblock->changed.ffp_ps_constants when the primary stateblock is created [wined3d_stateblock_invalidate_initial_states()], so that those constants are pushed when wined3d_device_apply_stateblock() is called even if no states were ever set by the application.

[Henri Verbeet (＠hverbeet)]

> > Found by running the regression tests.
>
> Huh, where? The GL tests pass for me with master...

d3d10core, but it's not unique to that. Note that the issue won't happen when using core contexts, due to the check for WINED3D_GL_LEGACY_CONTEXT in shader_glsl_update_legacy_states().
> The intended solution here (which may not be the best one, I don't know) is that any draw that uses a stateblock should have set these constants. This is done by invalidating stateblock->changed.ffp_ps_constants when the primary stateblock is created [wined3d_stateblock_invalidate_initial_states()], so that those constants are pushed when wined3d_device_apply_stateblock() is called even if no states were ever set by the application.

The trouble with that approach is that glsl_fragment_pipe_alpha_test_func() also gets called for draws that don't use stateblocks. Specifically: shader_glsl_apply_draw_state() -> shader_glsl_update_legacy_states() -> glsl_fragment_pipe_alpha_test_func(), because of WINED3D_SHADER_TYPE_PIXEL in "shader_update_mask".

[PATCH 0/1] MR7355: wined3d: Handle a NULL "push_constants" buffer in glsl_fragment_pipe_alpha_test_func(). - approved

[Elizabeth Figura (＠zfigura)]

This merge request was approved by Elizabeth Figura.

[PATCH 0/1] MR7355: wined3d: Handle a NULL "push_constants" buffer in glsl_fragment_pipe_alpha_test_func(). - approved

[Jan Sikorski (＠jsikorski)]

This merge request was approved by Jan Sikorski.