Re: [PATCH 1/2] d3dx9: Implement D3DXCreateBox()

[Henri Verbeet]

I'm not really in charge of d3dx9, and I would've expected Matteo or
Rico to write this instead, but here are some comments.

On 27 February 2014 11:38, Gediminas Jakutis  wrote:
> - static const float sign_x[24] = {-1.0f, -1.0f, -1.0f, -1.0f, -1.0f, -1.0f,  1.0f,  1.0f,
> -
>                                  1.0f,  1.0f,  1.0f,  1.0f, -1.0f, -1.0f,  1.0f,  1.0f,
> -
>                                 -1.0f,  1.0f,  1.0f, -1.0f, -1.0f, -1.0f,  1.0f,  1.0f};

The explicit array size isn't needed, and I don't think it adds much
for clarity. The indentation looks pretty arbitrary, the usual style
for tables like these is to put the brace on a new line, and then
indent the elements once. I.e.:
    static const float sign_x[] =
    {
        -1.0f, -1.0f, -1.0f, -1.0f, -1.0f, -1.0f,  1.0f,  1.0f,
         1.0f,  1.0f,  1.0f,  1.0f, -1.0f, -1.0f,  1.0f,  1.0f,
        -1.0f,  1.0f,  1.0f, -1.0f, -1.0f, -1.0f,  1.0f,  1.0f,
    };
> - TRACE("(%p, %f, %f, %f, %p, %p)\n", device, width, height, depth, mesh, adjacency);

I know d3dx9 isn't terribly consistent about this, but:
    TRACE("device %p, width %.8e, height %.8e, depth %.8e, mesh %p,
adjacency %p.\n",
            device, width, height, depth, mesh, adjacency);
> - hr = D3DXCreateMeshFVF(12, 24, D3DXMESH_MANAGED, D3DFVF_XYZ | D3DFVF_NORMAL, device, &box);
> -
> - if (FAILED(hr))
> - {
> -
>    return hr;
> - }
> -
> - if (FAILED(hr = box->lpVtbl->LockVertexBuffer(box, 0, (void **)&vertices)))
> - {
> -
>    box->lpVtbl->Release(box);
> -
>    return hr;
> - }

These are inconsistent. I'd prefer the second.
> - for (i = 0; i < 24; i++)
> - {
> -
>    vertices[i].position.x = width * sign_x[i];
> -
>    vertices[i].position.y = height * sign_y[i];
> -
>    vertices[i].position.z = depth * sign_z[i];

I suppose this works, but I think it would be clearer to store a unit
box as an array of D3DXVECTOR3s, and then call D3DXVec3Scale() on the
vectors. The "24" would then become "ARRAY_SIZE(unit_box)" as well.
> -
>    hr = D3DXCreateBuffer(36 * sizeof(DWORD), adjacency);
> -
> -
>    if (FAILED(hr))

if (FAILED(hr = D3DXCreateBuffer(sizeof(adjacency_table), adjacency)))