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

Henri Verbeet hverbeet at gmail.com
Thu Feb 27 09:05:52 CST 2014


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 <gediminas at varciai.lt> 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)))



More information about the wine-devel mailing list