On Sun, Jul 3, 2011 at 8:41 AM, Dylan Smith dylan.ah.smith@gmail.com wrote:
On Sat, Jul 2, 2011 at 1:01 PM, Michael Mc Donnell michael@mcdonnell.dk wrote:
+static HRESULT init_edge_face_map(struct edge_face_map *edge_face_map, CONST DWORD *index_buffer, CONST DWORD *point_reps, CONST DWORD num_faces) +{
...
- TRACE("(%p, %p, %p, %d)\n", edge_face_map, index_buffer, point_reps, num_faces);
A TRACE call for an internal initialization function like this seems unnecessary. They are commonly provided for external functions since it captures application behavior.
Ok, I've removed them.
- if (!point_reps) /* Identity point reps */
- {
- id_point_reps = generate_identity_point_reps(num_faces, num_vertices);
- if (!id_point_reps)
- {
- hr = E_OUTOFMEMORY;
- goto cleanup;
...
- hr = iface->lpVtbl->LockIndexBuffer(iface, D3DLOCK_READONLY, &ib_ptr);
- if (FAILED(hr)) goto cleanup;
...
+cleanup:
- HeapFree(GetProcessHeap(), 0, id_point_reps);
- if (indices_are_16_bit) HeapFree(GetProcessHeap(), 0, ib);
- free_edge_face_map(&edge_face_map);
- iface->lpVtbl->UnlockIndexBuffer(iface);
- return hr;
}
The index buffer is unconditionally unlocked in the cleanup code, but goto cleanup can now be called before the index buffer is locked.
I've added a check to see if it's locked.
+static DWORD *generate_identity_point_reps(DWORD num_faces, DWORD num_vertices) +{
- DWORD *id_point_reps;
- DWORD i;
- TRACE("(%d, %d)\n", num_faces, num_vertices);
- id_point_reps = HeapAlloc(GetProcessHeap(), 0, 3 * num_faces * sizeof(*id_point_reps));
- if (!id_point_reps)
- return NULL;
- for (i = 0; i < num_vertices; i++)
- {
- id_point_reps[i] = i;
- }
- for (i = num_vertices; i < 3 * num_faces; i++)
- {
- id_point_reps[i] = -1;
- }
- return id_point_reps;
+}
Why are there exra point reps allocated with -1 value? Won't only num_vertices point_reps get used?
Yes that was a mistake. Only num_vertices are used and not 3*num_faces. I have also removed some unnecessary -1 checks.