Re: [PATCH] wined3d: Fix per-stage constant in GLSL fixed function pipeline + add tests. (try 2)
2013/9/4 Christian Costa <titan.costa(a)gmail.com>:
This patch fixes part of bug 33606 and 31918.
The current code generates shaders that support per-stage constant but does not declare the variables used causing thus compilation failures. With this patch, these variables are declared and updated when needed. Tests are also added.
Thanks to Matteo Bruni for review and support.
Try 2: - update with latest git
Hi, I have a few small nits still, please bear with me...
--- dlls/d3d9/tests/visual.c | 80 ++++++++++++++++++++++++++++++++++++++++++++ dlls/wined3d/glsl_shader.c | 34 ++++++++++++++++++- 2 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index b856c93..6072c23 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -10742,6 +10742,85 @@ static void dp3_alpha_test(IDirect3DDevice9 *device) { ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 0x%08x\n", hr); }
+static void perstage_constant_test(IDirect3DDevice9 *device) +{ + HRESULT hr; + D3DCAPS9 caps; + DWORD color; + struct vertex quad[] = { + { -1.0, -1.0, 0.1, 0x408080c0 }, + { 1.0, -1.0, 0.1, 0x408080c0 }, + { -1.0, 1.0, 0.1, 0x408080c0 }, + { 1.0, 1.0, 0.1, 0x408080c0 }, + }; +
Can you please make quad[] static const? Also, add the 'f' suffix to the float constants and move the opening brace to newline.
+ memset(&caps, 0, sizeof(caps)); + hr = IDirect3DDevice9_GetDeviceCaps(device, &caps); + ok(SUCCEEDED(hr), "GetDeviceCaps failed with 0x%08x\n", hr); + if (!(caps.PrimitiveMiscCaps & D3DPMISCCAPS_PERSTAGECONSTANT)) + { + skip("D3DPMISCCAPS_PERSTAGECONSTANT not supported\n");
Please put a '.' at the end of the message. Same for the following prints.
+ return; + } + + hr = IDirect3DDevice9_SetFVF(device, D3DFVF_XYZ | D3DFVF_DIFFUSE); + ok(hr == D3D_OK, "IDirect3DDevice9_SetFVF failed with 0x%08x\n", hr); + + hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_CONSTANT, 0x80a1b2c3); + ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 0x%08x\n", hr); + + /* Check color values */ + hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLOROP, D3DTOP_SELECTARG1); + ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 0x%08x\n", hr); + hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLORARG1, D3DTA_CONSTANT); + ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 0x%08x\n", hr); +
Although you don't strictly need it here, I'd explicitly set D3DTSS_COLOROP for stage 1 to D3DTOP_DISABLE. Also maybe set D3DTSS_ALPHAOP for stage 0 to disabled?
+ hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET | D3DCLEAR_ZBUFFER, 0x00000000, 1.0f, 0); + ok(hr == D3D_OK, "IDirect3DDevice9_Clear failed with 0x%08x\n", hr); + + hr = IDirect3DDevice9_BeginScene(device); + ok(hr == D3D_OK, "IDirect3DDevice9_BeginScene failed with 0x%08x\n", hr); + hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(*quad)); + ok(SUCCEEDED(hr), "DrawPrimitiveUP failed with 0x%08x\n", hr); + hr = IDirect3DDevice9_EndScene(device); + ok(hr == D3D_OK, "IDirect3DDevice9_EndScene failed with 0x%08x\n", hr); + + color = getPixelColor(device, 320, 240); + ok(color_match(color, 0x00a1b2c3, 4), "perstage constant test 0x%08x, expected 0x00a1b2c3\n", color); + hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL); + ok(SUCCEEDED(hr), "IDirect3DDevice9_Present failed with 0x%08x\n", hr); + + /* Check alpha value */ + hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLOROP, D3DTOP_SELECTARG1); + ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 0x%08x\n", hr); + hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLORARG1, D3DTA_CONSTANT | D3DTA_ALPHAREPLICATE); + ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 0x%08x\n", hr); + hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_ALPHAOP, D3DTOP_SELECTARG1); + ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 0x%08x\n", hr); + hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_ALPHAARG1, D3DTA_CONSTANT); + ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 0x%08x\n", hr); + + hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET | D3DCLEAR_ZBUFFER, 0x00000000, 1.0f, 0); + ok(hr == D3D_OK, "IDirect3DDevice9_Clear failed with 0x%08x\n", hr); + + hr = IDirect3DDevice9_BeginScene(device); + ok(hr == D3D_OK, "IDirect3DDevice9_BeginScene failed with 0x%08x\n", hr); + hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(*quad)); + ok(SUCCEEDED(hr), "DrawPrimitiveUP failed with 0x%08x\n", hr); + hr = IDirect3DDevice9_EndScene(device); + ok(hr == D3D_OK, "IDirect3DDevice9_EndScene failed with 0x%08x\n", hr); + + color = getPixelColor(device, 320, 240); + ok(color_match(color, 0x00808080, 4), "perstage constant test 0x%08x, expected 0x00808080\n", color); + hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL); + ok(SUCCEEDED(hr), "IDirect3DDevice9_Present failed with 0x%08x\n", hr); + + hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLOROP, D3DTOP_DISABLE); + ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 0x%08x\n", hr); + hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_ALPHAOP, D3DTOP_DISABLE); + ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 0x%08x\n", hr);
I'd move those to the test which actually requires it (which is depth_buffer2_test IIUC). It could be a separate patch. The other option would be to move this new test out of the way (e.g. by executing it next to resz_test(), thus using a separate device). I don't have a strong opinion in that regard though.
+} + static void zwriteenable_test(IDirect3DDevice9 *device) { HRESULT hr; DWORD color; @@ -14811,6 +14890,7 @@ START_TEST(visual) texop_range_test(device_ptr); alphareplicate_test(device_ptr); dp3_alpha_test(device_ptr); + perstage_constant_test(device_ptr); depth_buffer_test(device_ptr); depth_buffer2_test(device_ptr); depth_blit_test(device_ptr); diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index 740e100..2b8041a 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -132,6 +132,7 @@ struct glsl_ps_program GLint bumpenv_mat_location[MAX_TEXTURES]; GLint bumpenv_lum_scale_location[MAX_TEXTURES]; GLint bumpenv_lum_offset_location[MAX_TEXTURES]; + GLint const_location[MAX_TEXTURES]; GLint tex_factor_location; GLint specular_enable_location; GLint ycorrection_location; @@ -918,6 +919,15 @@ static void shader_glsl_load_constants(void *shader_priv, struct wined3d_context else GL_EXTCALL(glUniform4fARB(prog->ps.specular_enable_location, 0.0f, 0.0f, 0.0f, 0.0f));
+ for (i = 0; i < MAX_TEXTURES; ++i) + { + if (prog->ps.const_location[i] != -1) + { + D3DCOLORTOGLFLOAT4(state->texture_states[i][WINED3D_TSS_CONSTANT], col); + GL_EXTCALL(glUniform4fARB(prog->ps.const_location[i], col[0], col[1], col[2], col[3])); + } + } + checkGLcall("fixed function uniforms"); }
Like I already mentioned privately, this could have its own WINED3D_SHADER_CONST_* flag but I'm not sure it would be worth it. Comments?
@@ -5167,7 +5177,6 @@ static const char *shader_glsl_get_ffp_fragment_op_arg(struct wined3d_shader_buf break;
case WINED3DTA_CONSTANT: - FIXME("Per-stage constants not implemented.\n"); switch (stage) { case 0: ret = "const0"; break; @@ -5447,6 +5456,16 @@ static GLuint shader_glsl_generate_ffp_fragment_shader(struct wined3d_shader_buf
for (stage = 0; stage < MAX_TEXTURES; ++stage) { + if ((settings->op[stage].carg0 == WINED3DTA_CONSTANT) + || (settings->op[stage].carg1 == WINED3DTA_CONSTANT) + || (settings->op[stage].carg2 == WINED3DTA_CONSTANT) + || (settings->op[stage].aarg0 == WINED3DTA_CONSTANT) + || (settings->op[stage].aarg1 == WINED3DTA_CONSTANT) + || (settings->op[stage].aarg2 == WINED3DTA_CONSTANT)) + { + shader_addline(buffer, "uniform vec4 const%d;", stage); + } + if (!(tex_map & (1 << stage))) continue;
@@ -5803,6 +5822,8 @@ static void shader_glsl_init_ps_uniform_locations(const struct wined3d_gl_info * ps->bumpenv_lum_scale_location[i] = GL_EXTCALL(glGetUniformLocationARB(program_id, name)); snprintf(name, sizeof(name), "bumpenv_lum_offset%u", i); ps->bumpenv_lum_offset_location[i] = GL_EXTCALL(glGetUniformLocationARB(program_id, name)); + snprintf(name, sizeof(name), "const%u", i); + ps->const_location[i] = GL_EXTCALL(glGetUniformLocationARB(program_id, name)); }
ps->tex_factor_location = GL_EXTCALL(glGetUniformLocationARB(program_id, "tex_factor")); @@ -7106,7 +7127,8 @@ static void glsl_fragment_pipe_get_caps(const struct wined3d_gl_info *gl_info, s { caps->wined3d_caps = WINED3D_FRAGMENT_CAP_PROJ_CONTROL | WINED3D_FRAGMENT_CAP_SRGB_WRITE; - caps->PrimitiveMiscCaps = WINED3DPMISCCAPS_TSSARGTEMP; + caps->PrimitiveMiscCaps = WINED3DPMISCCAPS_TSSARGTEMP + | WINED3DPMISCCAPS_PERSTAGECONSTANT; caps->TextureOpCaps = WINED3DTEXOPCAPS_DISABLE | WINED3DTEXOPCAPS_SELECTARG1 | WINED3DTEXOPCAPS_SELECTARG2 @@ -7251,6 +7273,7 @@ static const struct StateEntryTemplate glsl_fragment_pipe_state_template[] = {STATE_TEXTURESTAGE(0, WINED3D_TSS_ALPHA_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(0, WINED3D_TSS_ALPHA_ARG0), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(0, WINED3D_TSS_RESULT_ARG), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, + {STATE_TEXTURESTAGE(0, WINED3D_TSS_CONSTANT), {STATE_TEXTURESTAGE(0, WINED3D_TSS_CONSTANT), glsl_fragment_pipe_invalidate_constants}, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(1, WINED3D_TSS_COLOR_OP), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(1, WINED3D_TSS_COLOR_ARG1), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(1, WINED3D_TSS_COLOR_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, @@ -7260,6 +7283,7 @@ static const struct StateEntryTemplate glsl_fragment_pipe_state_template[] = {STATE_TEXTURESTAGE(1, WINED3D_TSS_ALPHA_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(1, WINED3D_TSS_ALPHA_ARG0), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(1, WINED3D_TSS_RESULT_ARG), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, + {STATE_TEXTURESTAGE(1, WINED3D_TSS_CONSTANT), {STATE_TEXTURESTAGE(1, WINED3D_TSS_CONSTANT), glsl_fragment_pipe_invalidate_constants}, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(2, WINED3D_TSS_COLOR_OP), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(2, WINED3D_TSS_COLOR_ARG1), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(2, WINED3D_TSS_COLOR_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, @@ -7269,6 +7293,7 @@ static const struct StateEntryTemplate glsl_fragment_pipe_state_template[] = {STATE_TEXTURESTAGE(2, WINED3D_TSS_ALPHA_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(2, WINED3D_TSS_ALPHA_ARG0), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(2, WINED3D_TSS_RESULT_ARG), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, + {STATE_TEXTURESTAGE(2, WINED3D_TSS_CONSTANT), {STATE_TEXTURESTAGE(2, WINED3D_TSS_CONSTANT), glsl_fragment_pipe_invalidate_constants}, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(3, WINED3D_TSS_COLOR_OP), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(3, WINED3D_TSS_COLOR_ARG1), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(3, WINED3D_TSS_COLOR_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, @@ -7278,6 +7303,7 @@ static const struct StateEntryTemplate glsl_fragment_pipe_state_template[] = {STATE_TEXTURESTAGE(3, WINED3D_TSS_ALPHA_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(3, WINED3D_TSS_ALPHA_ARG0), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(3, WINED3D_TSS_RESULT_ARG), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, + {STATE_TEXTURESTAGE(3, WINED3D_TSS_CONSTANT), {STATE_TEXTURESTAGE(3, WINED3D_TSS_CONSTANT), glsl_fragment_pipe_invalidate_constants}, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(4, WINED3D_TSS_COLOR_OP), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(4, WINED3D_TSS_COLOR_ARG1), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(4, WINED3D_TSS_COLOR_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, @@ -7287,6 +7313,7 @@ static const struct StateEntryTemplate glsl_fragment_pipe_state_template[] = {STATE_TEXTURESTAGE(4, WINED3D_TSS_ALPHA_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(4, WINED3D_TSS_ALPHA_ARG0), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(4, WINED3D_TSS_RESULT_ARG), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, + {STATE_TEXTURESTAGE(4, WINED3D_TSS_CONSTANT), {STATE_TEXTURESTAGE(4, WINED3D_TSS_CONSTANT), glsl_fragment_pipe_invalidate_constants}, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(5, WINED3D_TSS_COLOR_OP), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(5, WINED3D_TSS_COLOR_ARG1), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(5, WINED3D_TSS_COLOR_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, @@ -7296,6 +7323,7 @@ static const struct StateEntryTemplate glsl_fragment_pipe_state_template[] = {STATE_TEXTURESTAGE(5, WINED3D_TSS_ALPHA_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(5, WINED3D_TSS_ALPHA_ARG0), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(5, WINED3D_TSS_RESULT_ARG), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, + {STATE_TEXTURESTAGE(5, WINED3D_TSS_CONSTANT), {STATE_TEXTURESTAGE(5, WINED3D_TSS_CONSTANT), glsl_fragment_pipe_invalidate_constants}, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(6, WINED3D_TSS_COLOR_OP), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(6, WINED3D_TSS_COLOR_ARG1), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(6, WINED3D_TSS_COLOR_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, @@ -7305,6 +7333,7 @@ static const struct StateEntryTemplate glsl_fragment_pipe_state_template[] = {STATE_TEXTURESTAGE(6, WINED3D_TSS_ALPHA_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(6, WINED3D_TSS_ALPHA_ARG0), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(6, WINED3D_TSS_RESULT_ARG), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, + {STATE_TEXTURESTAGE(6, WINED3D_TSS_CONSTANT), {STATE_TEXTURESTAGE(6, WINED3D_TSS_CONSTANT), glsl_fragment_pipe_invalidate_constants}, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(7, WINED3D_TSS_COLOR_OP), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(7, WINED3D_TSS_COLOR_ARG1), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(7, WINED3D_TSS_COLOR_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, @@ -7314,6 +7343,7 @@ static const struct StateEntryTemplate glsl_fragment_pipe_state_template[] = {STATE_TEXTURESTAGE(7, WINED3D_TSS_ALPHA_ARG2), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(7, WINED3D_TSS_ALPHA_ARG0), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, {STATE_TEXTURESTAGE(7, WINED3D_TSS_RESULT_ARG), {STATE_PIXELSHADER, NULL }, WINED3D_GL_EXT_NONE }, + {STATE_TEXTURESTAGE(7, WINED3D_TSS_CONSTANT), {STATE_TEXTURESTAGE(7, WINED3D_TSS_CONSTANT), glsl_fragment_pipe_invalidate_constants}, WINED3D_GL_EXT_NONE }, {STATE_PIXELSHADER, {STATE_PIXELSHADER, glsl_fragment_pipe_shader }, WINED3D_GL_EXT_NONE }, {STATE_RENDER(WINED3D_RS_FOGENABLE), {STATE_RENDER(WINED3D_RS_FOGENABLE), glsl_fragment_pipe_fog }, WINED3D_GL_EXT_NONE }, {STATE_RENDER(WINED3D_RS_FOGTABLEMODE), {STATE_RENDER(WINED3D_RS_FOGENABLE), NULL }, WINED3D_GL_EXT_NONE },
Cheers, Matteo.
participants (1)
-
Matteo Bruni