Re: GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9

[Dylan Smith]

On Thu, Jun 30, 2011 at 3:50 PM, Michael Mc Donnell
<michael at mcdonnell.dk> wrote:
> I've implemented the look-up scheme that you described.

> +    if (!point_reps) /* Indentity point reps */
> +    {
> +        memset(adjacency, -1, 3 * num_faces * sizeof(*adjacency));
> +        return D3D_OK;
> +    }

I think you are missing the most common cases, where vertices are
reused between triangles. For example:

0--1
| /|
|/ |
2--3

If identity point reps are passed to your function it will find the
adjacent edge, but if NULL is passed to the function for point reps,
then it will find no adjacencies even though semantically they should
be the same.

Also, note the spelling error: Indentity -> Identity

> +static void free_edge_face_map(struct edge_face_map *edge_face_map)
> +{
> +    if (!edge_face_map)
> +        return;
> +
> +    HeapFree(GetProcessHeap(), 0, edge_face_map->lists);
> +    HeapFree(GetProcessHeap(), 0, edge_face_map->entries);
> +}
...
>  static HRESULT WINAPI ID3DXMeshImpl_ConvertPointRepsToAdjacency(ID3DXMesh *iface, CONST DWORD *point_reps, DWORD *adjacency)
>  {
> +    struct edge_face_map *edge_face_map = NULL;
...
> +    free_edge_face_map(edge_face_map);
> +    iface->lpVtbl->UnlockIndexBuffer(iface);
> +    return hr;
>  }

edge_face_map isn't freed. Also, it doesn't need to be allocated on the heap.

> +    hr = iface->lpVtbl->LockIndexBuffer(iface, 0, &ib_ptr);

The LockIndexBuffer flags could be D3DLOCK_READONLY rather than 0
(which seems to have read-write semantics).