Re: [2/2] wined3d: Take abs() of vertex z coordinate as FFP fog coordinate (try 5)

[Matteo Bruni]

2014-11-06 21:51 GMT+01:00 Joachim Priesner :
> Take the absolute value of vertex.z as fog coordinate instead of just
> the z coordinate. This fixes e.g. fog for applications that use
> right-handed projection matrices.
>
> Also test the clamp behavior of the oFog vertex shader output.
>
> Tested on openSuse 13.1 and Windows 8.1 (VMware Player).
> Try 5 that fixes a test failure I overlooked.
>
> dlls/d3d8/tests/visual.c   | 296 +++++++++++++++++++++++++++++++++++++
>  dlls/d3d9/tests/visual.c   | 360 +++++++++++++++++++++++++++++++++++++++++++++
>  dlls/ddraw/tests/ddraw7.c  | 161 ++++++++++++++++++++
>  dlls/wined3d/glsl_shader.c |   2 +-
>  4 files changed, 818 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/d3d8/tests/visual.c b/dlls/d3d8/tests/visual.c
> index 163110c..a1eabcd 100644
> --- a/dlls/d3d8/tests/visual.c
> +++ b/dlls/d3d8/tests/visual.c
> @@ -696,6 +696,301 @@ done:
>      DestroyWindow(window);
>  }
>
> +/* This test tests fog in combination with negative vertex z coordinates. */
> +static void fog_negative_z_test(void)
> +{
> - enum
> - {
> -
>    C_ALPHA_0x00  = 0x00000000,
> -
>    C_CLEAR       = 0xffff00ff,
> -
>    C_FOGGED      = 0x0000ff00,
> -
>    C_HALF_FOGGED = 0x00808000,
> -
>    C_UNFOGGED    = 0x00ff0000,
> -
>    C_CLAMPED_FOG = 0xdeadbeef /* triggers special codepath when used as middle_color */
> - };
> -
> - /* Fill the null-shader entry with the FVF (SetVertexShader is "overloaded" on d3d8). */
> - DWORD vertex_shader[3] = {D3DFVF_XYZ | D3DFVF_DIFFUSE | D3DFVF_SPECULAR, 0, 0};
> - DWORD pixel_shader[2] = {0, 0};
> - IDirect3D8 *d3d;
> - IDirect3DDevice8 *device;
> - BOOL has_vs_support, has_ps_support, has_table_fog_support;
> - unsigned int i, ps, y;
> - D3DCOLOR color, expected_middle_color;
> - ULONG refcount;
> - D3DCAPS8 caps;
> - HWND window;
> - HRESULT hr;
> -
> - /* Basic vertex shader without fog computation ("non foggy") */
> - static const DWORD vertex_shader_code1[] =
> - {
> -
>    0xfffe0100,                                                             /* vs_1_0                */
> -
>    /* output.Pos.z = input.Pos.z * 0.5 + 0.5 */
> -
>    0x00000005, 0x80010000, 0x90aa0000, 0xa0000000,                         /* mul r0.x, v0.z, c0.x  */
> -
>    0x00000002, 0xc0040000, 0x80000000, 0xa0000000,                         /* add oPos.z, r0.x c0.x */
> -
>    /* output.Pos.xyw = input.Pos.xyw */
> -
>    0x00000001, 0xc00b0000, 0x90e40000,                                     /* mov oPos.xyw, v0      */
> -
>    /* output.Color = input.Color */
> -
>    0x00000001, 0xd00f0000, 0x90e40001,                                     /* mov oD0, v1           */
> -
>    0x0000ffff,                                                             /* END                   */
> - };

Where are you setting the value of c0 in the d3d8 test?

[2/2] wined3d: Take abs() of vertex z coordinate as FFP fog coordinate (try 5)

[Joachim Priesner]

Hi Matteo,
> Where are you setting the value of c0 in the d3d8 test?

In the vertex declaration (line 797), which is apparently the only way to do it in d3d8.

I could move the vertex declaration directly before the shader code like in test_scalar_instructions, would that make it clear enough?

Best
Joachim

[2/2] wined3d: Take abs() of vertex z coordinate as FFP fog coordinate (try 5)

[Matteo Bruni]

2014-11-07 19:53 GMT+01:00 Joachim Priesner :
> Hi Matteo,
> > Where are you setting the value of c0 in the d3d8 test?
>
> In the vertex declaration (line 797), which is apparently the only way to do it in d3d8.

Ah, right. It's not the only way since you can also use
SetVertexShaderConstant(), but that works too.
> I could move the vertex declaration directly before the shader code like in test_scalar_instructions, would that make it clear enough?

I think that would be nicer, yes.
> Best
> Joachim

Thanks,
Matteo.

[2/2] wined3d: Take abs() of vertex z coordinate as FFP fog coordinate (try 5)

[Stefan Dösinger]

2014-11-07 20:05 GMT+01:00 Matteo Bruni :
> 2014-11-07 19:53 GMT+01:00 Joachim Priesner :
> > I could move the vertex declaration directly before the shader code like
>
> in test_scalar_instructions, would that make it clear enough?
>
> I think that would be nicer, yes.

I don't think the ordering of the variable declaration is why Henri didn't
ack this patch. Maybe he didn't have time to look at it yet or maybe
there's something else he doesn't like.

I spotted another problem though: Please test negative fog start or end
values, e.g. fogstart = -1 and fogend = 0. As the attached (unfinished)
test shows, vertex fog evaluates the fog equation in the vertex pipeline
and then linearly interpolates the result into the fragment pipeline, which
mixes the colors. This means we have to know if the abs() happens before
the fog equation or afterwards. In my quick testing it seems that the abs
happens before the fog equation, so your change in glsl_shader.c shouldn't
need any changes. I expect that when you set start = -1 and end = 0 all
your tests except the NONE/NONE and foggy shader test should come up with a
fully fogged quad.

The way vertex fog is evaluated also explains the different handling of
fogstart == fogend for vertex and table fog. The attached patch still has
problems with table fog on the Radeon X1600 - for some reason it picks a
static depth = 1.0, and I don't see why. I suspect a driver bug, but I am
not sure yet. The test works on the Radeon 9000 and GeForce 7 GPUs.