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

[Michael Mc Donnell]

On Wed, Jun 22, 2011 at 4:53 PM, Dylan Smith  wrote:
> On Wed, Jun 22, 2011 at 10:12 AM, Michael Mc Donnell
>  wrote:
> > Thanks your comments. I've attached an updated version.
> > - if (indices) HeapFree(GetProcessHeap(), 0, indices);
> > - }
> > - if (new_indices) HeapFree(GetProcessHeap(), 0, new_indices);
>
> HeapFree checks for NULL for you, so remove the NULL check before
> calling HeapFree.

Ok. I see that now in the Wine source. That's a lot nicer than on Windows.
> > - struct
> > - {
> > - ID3DXMesh *meshes[ARRAY_SIZE(vertices)];
> > - const DWORD *indices[ARRAY_SIZE(vertices)];
> > - const DWORD num_vertices[ARRAY_SIZE(vertices)];
> > - const DWORD *adjacencies[ARRAY_SIZE(vertices)];
> > - DWORD *point_reps[ARRAY_SIZE(vertices)];
> > - const DWORD *exp_point_reps[ARRAY_SIZE(vertices)];
> > - const DWORD options[ARRAY_SIZE(vertices)];
> > - }
> > - tc =
> > - {
> > - {NULL, NULL, NULL, NULL, NULL, NULL, NULL},
> > - {indices0, indices1, indices2, indices3, indices4, indices5, indices6, (DWORD*)indices6_16bit},
> > - {num_vertices0, num_vertices1, num_vertices2, num_vertices3, num_vertices4, num_vertices5, num_vertices6, num_vertices6},
> > - {adjacency0, adjacency1, adjacency2, adjacency3, adjacency4, adjacency5, adjacency6, adjacency6},
> > - {point_rep0, point_rep1, point_rep2, point_rep3, point_rep4, point_rep5, point_rep6, point_rep6},
> > - {exp_point_rep0, exp_point_rep1, exp_point_rep2, exp_point_rep3, exp_point_rep4, exp_point_rep5, exp_point_rep6, exp_point_rep6},
> > - {options, options, options, options, options, options, options, options_16bit}
> > - };
>
> I actually meant something more like:
>
> --- snip ---
> struct {
>    const DWORD *indices,
>    const DWORD num_vertices,
>    const DWORD *adjacencies,
>    DWORD *point_reps,
>    const DWORD *exp_point_reps,
>    const DWORD options,
> } tc[] = {
>    {indices0, num_vertices0, adjacency0, point_rep0, exp_point_rep0, options},
>    {indices1, num_vertices1, adjacency1, point_rep1, exp_point_rep1, options},
>    ...
> }

Ah ok :-) That makes more sense.
> ...
>
> for (i = 0; i < ARRAY_SIZE(tc); i++)
> {
>    ...
> }
> --- snip ---
>
> I don't think storing an array of meshes is necessary.  You can just
> release the mesh and set the mesh variable to NULL (as needed to avoid
> re-releasing) before creating another one.

I've changed it so that it only keeps the first mesh around for the NULL checks.

My UpdateSemantics patch finally got applied so I've removed the
duplicate functions too.