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 <dylan.ah.smith at gmail.com> wrote:
> On Wed, Jun 22, 2011 at 10:12 AM, Michael Mc Donnell
> <michael at mcdonnell.dk> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-d3dx9-tests-Implemented-ConvertAdjacencyToPointReps-.patch
Type: text/x-patch
Size: 13755 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20110622/0a154ac7/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-d3dx9-Implemented-ConvertAdjacencyToPointReps-mesh-m.patch
Type: text/x-patch
Size: 9284 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20110622/0a154ac7/attachment-0003.bin>