From 133e7ba571d2441ebf34bada6fbe9d91b14a23f1 Mon Sep 17 00:00:00 2001 From: Axel Davy Date: Sun, 2 Apr 2023 17:35:53 +0200 Subject: [PATCH] frontend/nine: fix wfog When wfog support is advertised, unless an orthogonal projection matrix is detected, w is supposed to be used instead of z for the fog equation when done in the pixel shader. Due to the spec being ambiguous, and tests being incomplete, it seems we had got things wrong. New tests confirm the behaviour. For the explanation we will denote z_vs and w_vs the position output's z and w channels in the vertex shader, and z_ps, w_ps the position input z and w channels in the pixel shader. w_ps = 1/w_vs z_ps = z_vs/w_vs In the programmable pixel shader, we used z_ps/w_ps, thus z_vs. As basically z_vs and w_vs are usually in the same range, we didn't notice an obvious difference with the correct behaviour. In the ff pixel shader, we used z_ps for zfog and w_vs else. z_ps was always used if a programmable vertex shader was detected. This latter behaviour led to issue https://gitlab.freedesktop.org/mesa/mesa/-/issues/8341 While using z_ps/w_ps like for programmable ps fixes the issue visually for the same reason as it did for programmable ps, it breaks wine tests using XYZRHW. These tests show that when passing pre-transformed vertices and an orthogonal projection matrix, z_vs is used, and due to the XYZRHW property, this is not recovered by the z_ps/w_ps computation (instead z_ps=z_vs). For the game affected by the issue, the projection matrix set is not orthogonal. The direct3D spec indicates that the projection matrix must be set correctly for fog to work properly, even if we do not use the transformation pipeline (could be related to xyzrhw, or programmable vs or both). Previous tests had shown that the projection matrix has the last two values of the last column tested against 0 and 1, in order to activate zfog or wfog. The R500 spec indicates that either z or 1/1/w can be used as source for the fog computation, but it is not clear whether this is z_vs or z_ps. Tests confirmed the intuition that the correct behaviour is to use z_ps (zfog) when an orthogonal projection matrix is set (the spec spirit being that in that case z_ps=z_vs), and 1/w_ps (wfog) else (even if programmable shaders are used). This patch introduces this behaviour. Fixes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/8341 Signed-off-by: Axel Davy Part-of: --- src/gallium/frontends/nine/nine_ff.c | 16 +--------------- src/gallium/frontends/nine/nine_shader.c | 7 ++++--- src/gallium/frontends/nine/nine_shader.h | 1 + src/gallium/frontends/nine/nine_state.c | 12 ++++++++++++ src/gallium/frontends/nine/nine_state.h | 1 + src/gallium/frontends/nine/pixelshader9.c | 5 +++-- src/gallium/frontends/nine/pixelshader9.h | 16 +++++++++------- 7 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/gallium/frontends/nine/nine_ff.c b/src/gallium/frontends/nine/nine_ff.c index 861092a..3169677 100644 --- a/src/gallium/frontends/nine/nine_ff.c +++ b/src/gallium/frontends/nine/nine_ff.c @@ -1521,14 +1521,8 @@ nine_ff_build_ps(struct NineDevice9 *device, struct nine_ff_ps_key *key) } /* Source is either W or Z. - * When we use vs ff, * Z is when an orthogonal projection matrix is detected, * W (WFOG) else. - * Z is used for programmable vs. - * Note: Tests indicate that the projection matrix coefficients do - * actually affect pixel fog (and not vertex fog) when vs ff is used, - * which justifies taking the position's w instead of taking the z coordinate - * before the projection in the vs shader. */ if (!key->fog_source) ureg_MOV(ureg, rFog, _ZZZZ(vPos)); @@ -1723,7 +1717,6 @@ static struct NinePixelShader9 * nine_ff_get_ps(struct NineDevice9 *device) { struct nine_context *context = &device->context; - D3DMATRIX *projection_matrix = GET_D3DTS(PROJECTION); struct NinePixelShader9 *ps; struct nine_ff_ps_key key; unsigned s; @@ -1832,15 +1825,8 @@ nine_ff_get_ps(struct NineDevice9 *device) if (context->rs[D3DRS_FOGENABLE]) key.fog_mode = context->rs[D3DRS_FOGTABLEMODE]; key.fog = !!context->rs[D3DRS_FOGENABLE]; - /* Pixel fog (with WFOG advertised): source is either Z or W. - * W is the source if vs ff is used, and the - * projection matrix is not orthogonal. - * Tests on Win 10 seem to indicate _34 - * and _33 are checked against 0, 1. */ if (key.fog_mode && key.fog) - key.fog_source = !context->programmable_vs && - !(projection_matrix->_34 == 0.0f && - projection_matrix->_44 == 1.0f); + key.fog_source = !context->zfog; DBG("PS ff key hash: %x\n", nine_ff_ps_key_hash(&key)); ps = util_hash_table_get(device->ff.ht_ps, &key); diff --git a/src/gallium/frontends/nine/nine_shader.c b/src/gallium/frontends/nine/nine_shader.c index c7e9a7a..4f622cb 100644 --- a/src/gallium/frontends/nine/nine_shader.c +++ b/src/gallium/frontends/nine/nine_shader.c @@ -3757,9 +3757,10 @@ shader_add_ps_fog_stage(struct shader_translator *tx, struct ureg_src src_col) if (tx->info->fog_mode != D3DFOG_NONE) { depth = tx_scratch_scalar(tx); - /* Depth used for fog is perspective interpolated */ - ureg_RCP(ureg, depth, ureg_scalar(nine_get_position_input(tx), TGSI_SWIZZLE_W)); - ureg_MUL(ureg, depth, ureg_src(depth), ureg_scalar(nine_get_position_input(tx), TGSI_SWIZZLE_Z)); + if (tx->info->zfog) + ureg_MOV(ureg, depth, ureg_scalar(nine_get_position_input(tx), TGSI_SWIZZLE_Z)); + else /* wfog: use w. position's w contains 1/w */ + ureg_RCP(ureg, depth, ureg_scalar(nine_get_position_input(tx), TGSI_SWIZZLE_W)); } fog_color = nine_float_constant_src(tx, 32); diff --git a/src/gallium/frontends/nine/nine_shader.h b/src/gallium/frontends/nine/nine_shader.h index 4a0804b..44baf89 100644 --- a/src/gallium/frontends/nine/nine_shader.h +++ b/src/gallium/frontends/nine/nine_shader.h @@ -69,6 +69,7 @@ struct nine_shader_info uint8_t fog_enable; uint8_t fog_mode; + uint8_t zfog; uint8_t force_color_in_centroid; uint8_t projected; /* ps 1.1 to 1.3 */ uint16_t fetch4; diff --git a/src/gallium/frontends/nine/nine_state.c b/src/gallium/frontends/nine/nine_state.c index cd627c8..fd19270 100644 --- a/src/gallium/frontends/nine/nine_state.c +++ b/src/gallium/frontends/nine/nine_state.c @@ -1905,6 +1905,17 @@ CSMT_ITEM_NO_WAIT(nine_context_set_transform, D3DMATRIX *M = nine_state_access_transform(&context->ff, State, TRUE); *M = *pMatrix; + if (State == D3DTS_PROJECTION) { + BOOL prev_zfog = context->zfog; + /* Pixel fog (with WFOG advertised): source is either Z or W. + * W is the source if the projection matrix is not orthogonal. + * Tests on Win 10 seem to indicate _34 + * and _33 are checked against 0, 1. */ + context->zfog = (M->_34 == 0.0f && + M->_44 == 1.0f); + if (context->zfog != prev_zfog) + context->changed.group |= NINE_STATE_PS_PARAMS_MISC; + } context->ff.changed.transform[State / 32] |= 1 << (State % 32); context->changed.group |= NINE_STATE_FF; } @@ -2896,6 +2907,7 @@ nine_state_set_defaults(struct NineDevice9 *device, const D3DCAPS9 *caps, memset(context->ps_const_i, 0, sizeof(context->ps_const_i)); memset(state->ps_const_b, 0, sizeof(state->ps_const_b)); memset(context->ps_const_b, 0, sizeof(context->ps_const_b)); + context->zfog = false; /* Guess from wine tests: both true or false are ok */ /* Cap dependent initial state: */ diff --git a/src/gallium/frontends/nine/nine_state.h b/src/gallium/frontends/nine/nine_state.h index dad9e60..0c20ac0 100644 --- a/src/gallium/frontends/nine/nine_state.h +++ b/src/gallium/frontends/nine/nine_state.h @@ -270,6 +270,7 @@ struct nine_context { int ps_const_i[NINE_MAX_CONST_I][4]; BOOL ps_const_b[NINE_MAX_CONST_B]; float *ps_lconstf_temp; + BOOL zfog; struct NineVertexDeclaration9 *vdecl; diff --git a/src/gallium/frontends/nine/pixelshader9.c b/src/gallium/frontends/nine/pixelshader9.c index 8df27a9..a6ea126 100644 --- a/src/gallium/frontends/nine/pixelshader9.c +++ b/src/gallium/frontends/nine/pixelshader9.c @@ -211,12 +211,13 @@ NinePixelShader9_GetVariant( struct NinePixelShader9 *This, } info.fog_enable = device->context.rs[D3DRS_FOGENABLE]; info.fog_mode = device->context.rs[D3DRS_FOGTABLEMODE]; - info.force_color_in_centroid = (key >> 23) & 1; + info.zfog = device->context.zfog; info.add_constants_defs.c_combination = nine_shader_constant_combination_get(This->c_combinations, (key >> 24) & 0xff); info.add_constants_defs.int_const_added = &This->int_slots_used; info.add_constants_defs.bool_const_added = &This->bool_slots_used; - info.fetch4 = key >> 32 ; + info.fetch4 = (key >> 32) & 0xffff; + info.force_color_in_centroid = (key >> 48) & 1; info.process_vertices = false; info.swvp_on = false; diff --git a/src/gallium/frontends/nine/pixelshader9.h b/src/gallium/frontends/nine/pixelshader9.h index dd5e1ed..9efd0c3 100644 --- a/src/gallium/frontends/nine/pixelshader9.h +++ b/src/gallium/frontends/nine/pixelshader9.h @@ -111,15 +111,12 @@ NinePixelShader9_UpdateKey( struct NinePixelShader9 *ps, } } - if (ps->byte_code.version < 0x30) { - key |= ((uint64_t)context->rs[D3DRS_FOGENABLE]) << 20; + if (ps->byte_code.version < 0x30 && context->rs[D3DRS_FOGENABLE]) { + key |= 1 << 20; key |= ((uint64_t)context->rs[D3DRS_FOGTABLEMODE]) << 21; /* 2 bits */ + key |= ((uint64_t)context->zfog) << 23; } - /* centroid interpolation automatically used for color ps inputs */ - if (context->rt[0]->base.info.nr_samples) - key |= ((uint64_t)1) << 23; - if ((ps->const_int_slots > 0 || ps->const_bool_slots > 0) && context->inline_constants) key |= ((uint64_t)nine_shader_constant_combination_key(&ps->c_combinations, ps->int_slots_used, @@ -127,7 +124,12 @@ NinePixelShader9_UpdateKey( struct NinePixelShader9 *ps, (void *)context->ps_const_i, context->ps_const_b)) << 24; - key |= ((uint64_t)(context->rs[NINED3DRS_FETCH4] & samplers_fetch4)) << 32; + key |= ((uint64_t)(context->rs[NINED3DRS_FETCH4] & samplers_fetch4)) << 32; /* 16 bits */ + + /* centroid interpolation automatically used for color ps inputs */ + if (context->rt[0]->base.info.nr_samples) + key |= ((uint64_t)1) << 48; + res = ps->last_key != key; if (res) ps->next_key = key; -- 2.7.4