[Bf-codereview] modeler normals don't match render normals (issue4280049)

[brechtvanlommel at gmail.com]

http://codereview.appspot.com/4280049/diff/8003/source/blender/blenkernel/BKE_DerivedMesh.h
File source/blender/blenkernel/BKE_DerivedMesh.h (right):

http://codereview.appspot.com/4280049/diff/8003/source/blender/blenkernel/BKE_DerivedMesh.h#newcode213
source/blender/blenkernel/BKE_DerivedMesh.h:213: void
(*getRealVertNo)(DerivedMesh *dm, int face_index, int vert_num, float
no_r[3]);
I guess this function is no longer needed?

http://codereview.appspot.com/4280049/diff/8003/source/blender/blenkernel/BKE_DerivedMesh.h#newcode216
source/blender/blenkernel/BKE_DerivedMesh.h:216: float
(*getFaceNo)(DerivedMesh *dm, int index, float no_r[3]);
If you're going to use this, it would need to be implemented for all
types of derived meshes. Probably it's easier to drop it.

http://codereview.appspot.com/4280049/diff/8003/source/blender/blenkernel/intern/cdderivedmesh.c
File source/blender/blenkernel/intern/cdderivedmesh.c (right):

http://codereview.appspot.com/4280049/diff/8003/source/blender/blenkernel/intern/cdderivedmesh.c#newcode1869
source/blender/blenkernel/intern/cdderivedmesh.c:1869: /* calculate face
normals and add to vertex normals */
We can use mesh_calc_normals here instead, since the arrays are the
same. That's one duplicate less.

http://codereview.appspot.com/4280049/diff/8003/source/blender/blenkernel/intern/mesh.c
File source/blender/blenkernel/intern/mesh.c (right):

http://codereview.appspot.com/4280049/diff/8003/source/blender/blenkernel/intern/mesh.c#newcode1276
source/blender/blenkernel/intern/mesh.c:1276: {
It would be good to have a comment here to keep it in sync with
recalc_editnormals and calc_vertexnormals, and do the same in those
functions.

http://codereview.appspot.com/4280049/diff/8003/source/blender/blenkernel/intern/mesh.c#newcode1284
source/blender/blenkernel/intern/mesh.c:1284: {
I'm going to be pedantic here, but please follow the coding style from
the file/function as a minimum. Put { on the same line as if statement
here and in other places.

http://codereview.appspot.com/4280049/diff/8003/source/blender/blenkernel/intern/mesh.c#newcode1304
source/blender/blenkernel/intern/mesh.c:1304: if(tnorms[mf->v1][0]==0 &&
tnorms[mf->v1][1]==0 && tnorms[mf->v1][2]==0) VECCOPY(tnorms[mf->v1],
f_no);
Can use is_zero_v3 here and in other places. Also copy_v3_v3 is
preferred over VECCOPY.

http://codereview.appspot.com/4280049/diff/8003/source/blender/render/intern/source/convertblender.c
File source/blender/render/intern/source/convertblender.c (right):

http://codereview.appspot.com/4280049/diff/8003/source/blender/render/intern/source/convertblender.c#newcode606
source/blender/render/intern/source/convertblender.c:606: float n1[3],
n2[3], n3[3], n4[3];
Computing n1, n2, n3, n4 seems no longer necessary?

http://codereview.appspot.com/4280049/diff/8003/source/blender/render/intern/source/convertblender.c#newcode3234
source/blender/render/intern/source/convertblender.c:3234: int
iCalcNormalsInRender = 0;	// false by default
Please follow coding style, e.g rename to recalc_normals.

http://codereview.appspot.com/4280049/diff/8003/source/blender/render/intern/source/convertblender.c#newcode3324
source/blender/render/intern/source/convertblender.c:3324:
//dm->getVertNo(dm, a, ver->n);
Best to use normal_short_to_float_v3 here. In case of autosmooth it
wouldn't be normalized otherwise.

http://codereview.appspot.com/4280049/diff/8003/source/blender/render/intern/source/convertblender.c#newcode3419
source/blender/render/intern/source/convertblender.c:3419:
negate_v3(vlr->n);
As mentioned before, perhaps easiest to recalculate here and drop
getFaceNo.

http://codereview.appspot.com/4280049/