Re: wined3d: Correctly display fog for right-handed projection matrix (try 5)

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Am 2014-10-04 00:24, schrieb Joachim Priesner:
> +    static const D3DMATRIX proj_mat_rh =
> +    {{{
> +        0.828732f, 0.0f,       0.0f,       0.0f,
> +        0.0f,      1.104976f,  0.0f,       0.0f,
> +        0.0f,      0.0f,      -1.0001f,   -1.0f,
> +        0.0f,      0.0f,      -0.010001f,  0.0f
> +    }}};
> ...
> +    static const D3DMATRIX view_mat_rh =
> +    {{{
> +        1.0f, 0.0f,  0.0f, 0.0f,
> +        0.0f, 1.0f,  0.0f, 0.0f,
> +        0.0f, 0.0f, -0.1f, 0.0f,
> +        0.0f, 0.0f,  0.0f, 1.0f
> +    }}};
> I'm sorry that this reply is a bit late, but I've been wondering if you need the complicated projection matrix to show the abs() in the vertex pipeline. If I understand it correctly, a view and projection matrix like this should do the job as well with the same input coordinates:

static const D3DMATRIX proj_mat =
    {{{
        1.0f, 0.0f,  0.0f, 0.0f,
        0.0f, 1.0f,  0.0f, 0.0f,
        0.0f, 0.0f, -1.0f, 0.0f,
        0.0f, 0.0f,  0.0f, 1.0f
    }}};
    static const D3DMATRIX view_mat =
    {{{
        1.0f, 0.0f,  0.0f, 0.0f,
        0.0f, 1.0f,  0.0f, 0.0f,
        0.0f, 0.0f, -0.1f, 0.0f,
        0.0f, 0.0f,  0.0f, 1.0f
    }}};

The test results with these matrices are identical, and the test correctly detects the defect in Wine. The test is much easier to follow. Am I missing something?

As an added bonus you could replace the 10.0f in the input vertices with 1.0 and the -0.1f in the view matrix with -1.0f.

Some other suggestions:
> +/* Some color constants to make tests easier to read. */
> +#define COLOR_FOG       0xff00ff00
> +#define COLOR_FOGGED    0x0000ff00
> +#define COLOR_UNFOGGED  0x00ff0000
> +#define COLOR_CLEAR     0xffff00ff
> IMHO it would be nicer to either use an enum in the scope of fog_righthanded_test (preferred) or #undef them after the function.
> +    static struct
> +    {
> +        struct vec3 position;
> +        DWORD diffuse;
> +    }
> +    quad[] =
> +    {
> +        {{-1.0f, -1.0f, 10.0f}, COLOR_UNFOGGED},
> +        {{-1.0f,  1.0f, 10.0f}, COLOR_UNFOGGED},
> +        {{ 1.0f, -1.0f, 10.0f}, COLOR_UNFOGGED},
> +        {{ 1.0f,  1.0f, 10.0f}, COLOR_UNFOGGED},
> +    };
> Always drawing a gradient would be better. That avoids other fog setup problems that lead to either always fogged or never fogged quads.

Also, this can be static const in d3d8 and d3d9.
> +        hr = IDirect3DDevice8_Clear(device, 0, NULL, D3DCLEAR_TARGET | D3DCLEAR_ZBUFFER, COLOR_CLEAR, 1.0f, 0);
> +        ok(SUCCEEDED(hr), "IDirect3DDevice9_Clear failed (%08x)\n", hr);
> Do you need the Z buffer? Just disable the depth test, no need to clear it.
> +   /* Basic vertex shader with fog computation ("foggy").
> +    * Using the vertex' x coordinate as fog coordinate so that all test cases that use vertex fog
> +    * will have a gradient from fully fogged (green, left) to unfogged (red, right). */
> +    static const DWORD vertex_shader_code2[] =
> +    {
> +        0xfffe0100,                                                             /* vs_1_0             */
> ...
> +        0x0000ffff,                                                             /* END                */
> +     };
> +
> +    /* Basic pixel shader */
> Two indentation issues here. The comment needs one extra space, the closing bracket has one too many. I suspect your editor screwed this up when you copied the code from d3d9 to d3d8.
> +        if (caps.PixelShaderVersion >= D3DPS_VERSION(2, 0))
> +        {
> +            hr = IDirect3DDevice9_CreateVertexShader(device, vertex_shader_code3, &vertex_shader[3]);
> +            ok(SUCCEEDED(hr), "CreateVertexShader failed (%08x)\n", hr);
> +            hr = IDirect3DDevice9_CreatePixelShader(device, pixel_shader_code2, &pixel_shader[2]);
> +            ok(SUCCEEDED(hr), "CreatePixelShader failed (%08x)\n", hr);
> +        } else {
> +            skip("No shader model 2 support, skipping some fog tests.\n");
> +        }
> Curly bracket placement. This applies to a few other places.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJUNGWtAAoJEN0/YqbEcdMw/+YQAIQxmRjeNMne0+f9AuCwtNVJ
/MrhsXPnCFDWbYE6BTXM8/2CvlM5DzhCya2AQg8UFW4qMXQJmijTorCjDF0e7Vc7
RvuP4xJXsqhKDhyoYfSzcX8WYkJZcVow0tQIz++qKfjMbgqnGwoWocYdNgyymy9v
pU6Q/shM1qv2PUkDNuMAJI7Tv09R5muYEyi9sfRa+grFGQwDJGQpgOXwP5oR2DWM
wjlTIO7wMkXY0XGE5DInxE+slaF2gECrKrHRM+1HNicNq5hGOwYwdm/JcBA9qxGQ
uv+w0WMKR0n4FPtJBHXcm3/qshUyBvzSsMSEilfBwOzrrclOx9ctNEuogqufEpF9
6yGuX0PEp+W9g9rN6rCMMBjYmGJdal2bY2F1KQ+iLAYHhMsahy+fibIGEwE7Yugg
+ffx3RONm335mpZqQh5NrtHAwf3iLEEA7PQQawW5UKCoKANpV03KyZBFJLZ1lOC4
cEQLQQNo7p/M0211bm4dTN8SSYvucloDSuzwe+PqCNvyL8xJyJCiHMHuVX2cFdgt
7qPA13LdKze/7S4Jk1JjL31tTEdMCGZJSekkTEAJ7C/c01L3DiDVywn9VNVXdPsR
8oW8pfZI0t1g2/Miq08mP1T+IWTftYOQaHyiYrQ73hu8PtDgY3C92JHwV4SmvPUI
UxItWlY/H/8FLJG5v5dt
=dUDt
-----END PGP SIGNATURE-----