Re: [v2 PATCH 3/3] d3drm: Implement AddNormal()

[Henri Verbeet]

On 7 June 2017 at 12:38, Nikolay Sivov  wrote:
> @@ -1165,12 +1169,13 @@ HRESULT load_mesh_data(IDirect3DRMMeshBuilder3 *iface, IDirectXFileData *pData,
>              mesh_builder->nb_normals = *(DWORD*)ptr;
>              nb_faces_normals = *(DWORD*)(ptr + sizeof(DWORD) + mesh_builder->nb_normals * sizeof(D3DVECTOR));
> -
>        TRACE("MeshNormals: nb_normals = %d, nb_faces_normals = %d\n", mesh_builder->nb_normals, nb_faces_normals);
> -
>        TRACE("MeshNormals: nb_normals = %lu, nb_faces_normals = %d\n", mesh_builder->nb_normals, nb_faces_normals);
>        if (nb_faces_normals != mesh_builder->nb_faces)
> -
>            WARN("nb_face_normals (%d) != nb_faces (%d)\n", nb_faces_normals, mesh_builder->nb_normals);
> -
>            WARN("nb_face_normals (%d) != nb_faces (%d)\n", nb_faces_normals, mesh_builder->nb_faces);
>
> -
>        mesh_builder->pNormals = HeapAlloc(GetProcessHeap(), 0, mesh_builder->nb_normals * sizeof(D3DVECTOR));
> -
>        memcpy(mesh_builder->pNormals, ptr + sizeof(DWORD), mesh_builder->nb_normals * sizeof(D3DVECTOR));
> -
>        d3drm_array_reserve((void **)&mesh_builder->normals, &mesh_builder->normals_size,
> -
>                mesh_builder->nb_normals + 1, sizeof(*mesh_builder->normals));

That's not quite equivalent, but probably harmless.
> @@ -1379,8 +1384,8 @@ HRESULT load_mesh_data(IDirect3DRMMeshBuilder3 *iface, IDirectXFileData *pData,
>      if (!mesh_builder->nb_normals)
>      {
>          /* Allocate normals, one per vertex */
> -
>    mesh_builder->pNormals = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, mesh_builder->nb_vertices * sizeof(D3DVECTOR));
> -
>    if (!mesh_builder->pNormals)
> -
>    if (!d3drm_array_reserve((void **)&mesh_builder->normals, &mesh_builder->normals_size,
> -
>            mesh_builder->nb_normals + 1, sizeof(*mesh_builder->normals)))

This one isn't quite equivalent either, and less harmless.
> @@ -1445,7 +1450,7 @@ HRESULT load_mesh_data(IDirect3DRMMeshBuilder3 *iface, IDirectXFileData *pData,
>                  }
>                  *(faces_data_ptr + faces_data_size++) = vertex_idx;
>                  /* Add face normal to vertex normal */
> -
>            D3DRMVectorAdd(&mesh_builder->pNormals[vertex_idx], &mesh_builder->pNormals[vertex_idx], &face_normal);
> -
>            D3DRMVectorAdd(&mesh_builder->normals[vertex_idx], &mesh_builder->normals[vertex_idx], &face_normal);

In particular, note that this line depends on the initial vertex
normals being {0.0f, 0.0f, 0.0f}.