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

Morten Mikkelsen mikkelsen7 at gmail.com
Tue Mar 15 18:37:24 CET 2011


Don't think it matters but it seems source/blender/ is missing from the
paths in the patch I sent.
I must have been in blender/source/blender when I made it or something?
Either way all my changes were in that folder so if you successfully patched
already then this
is the very same.




On Tue, Mar 15, 2011 at 9:20 AM, Morten Mikkelsen <mikkelsen7 at gmail.com>wrote:

> Having trouble figuring out where, in the tool, I can add the alternative
> patch.
> So I am e-mailing it instead.
>
> On Tue, Mar 15, 2011 at 8:22 AM, Morten Mikkelsen <mikkelsen7 at gmail.com>wrote:
>
>> I'll make a version of the patch without any code sharing instead.
>>
>>
>>
>> On Tue, Mar 15, 2011 at 8:02 AM, <brechtvanlommel at gmail.com> wrote:
>>
>>> On 2011/03/15 14:51:05, mikkelsen7 wrote:
>>>
>>>> But it has to iterate over the faces twice.
>>>>
>>>
>>>  Removing the iteration from the abstraction would conflict with that.
>>>>
>>>
>>> Is the second loop really necessary, those normals should never be used?
>>> Maybe for export.. If it is necessary, just add two loops then.
>>>
>>>
>>> http://codereview.appspot.com/4280049/
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.blender.org/pipermail/bf-codereview/attachments/20110315/e453df82/attachment.htm 
-------------- next part --------------
Index: source/blender/render/intern/source/convertblender.c
===================================================================
--- source/blender/render/intern/source/convertblender.c	(revision 35505)
+++ source/blender/render/intern/source/convertblender.c	(working copy)
@@ -499,6 +499,11 @@
 }
 
 
+
+/****************************************************************
+************ tangent space generation interface *****************
+****************************************************************/
+
 typedef struct
 {
 	ObjectRen *obr;
@@ -599,7 +604,6 @@
 			VertRen *v3= vlr->v3;
 			VertRen *v4= vlr->v4;
 			float n1[3], n2[3], n3[3], n4[3];
-			float fac1, fac2, fac3, fac4=0.0f;
 			
 			sub_v3_v3v3(n1, v2->co, v1->co);
 			normalize_v3(n1);
@@ -608,10 +612,6 @@
 			if(v4==NULL) {
 				sub_v3_v3v3(n3, v1->co, v3->co);
 				normalize_v3(n3);
-
-				fac1= saacos(-n1[0]*n3[0]-n1[1]*n3[1]-n1[2]*n3[2]);
-				fac2= saacos(-n1[0]*n2[0]-n1[1]*n2[1]-n1[2]*n2[2]);
-				fac3= saacos(-n2[0]*n3[0]-n2[1]*n3[1]-n2[2]*n3[2]);
 			}
 			else {
 				sub_v3_v3v3(n3, v4->co, v3->co);
@@ -619,27 +619,22 @@
 				sub_v3_v3v3(n4, v1->co, v4->co);
 				normalize_v3(n4);
 
-				fac1= saacos(-n4[0]*n1[0]-n4[1]*n1[1]-n4[2]*n1[2]);
-				fac2= saacos(-n1[0]*n2[0]-n1[1]*n2[1]-n1[2]*n2[2]);
-				fac3= saacos(-n2[0]*n3[0]-n2[1]*n3[1]-n2[2]*n3[2]);
-				fac4= saacos(-n3[0]*n4[0]-n3[1]*n4[1]-n3[2]*n4[2]);
-
-				v4->n[0] +=fac4*vlr->n[0];
-				v4->n[1] +=fac4*vlr->n[1];
-				v4->n[2] +=fac4*vlr->n[2];
+				v4->n[0] +=vlr->n[0];
+				v4->n[1] +=vlr->n[1];
+				v4->n[2] +=vlr->n[2];
 			}
 
-			v1->n[0] +=fac1*vlr->n[0];
-			v1->n[1] +=fac1*vlr->n[1];
-			v1->n[2] +=fac1*vlr->n[2];
+			v1->n[0] +=vlr->n[0];
+			v1->n[1] +=vlr->n[1];
+			v1->n[2] +=vlr->n[2];
 
-			v2->n[0] +=fac2*vlr->n[0];
-			v2->n[1] +=fac2*vlr->n[1];
-			v2->n[2] +=fac2*vlr->n[2];
+			v2->n[0] +=vlr->n[0];
+			v2->n[1] +=vlr->n[1];
+			v2->n[2] +=vlr->n[2];
 
-			v3->n[0] +=fac3*vlr->n[0];
-			v3->n[1] +=fac3*vlr->n[1];
-			v3->n[2] +=fac3*vlr->n[2];
+			v3->n[0] +=vlr->n[0];
+			v3->n[1] +=vlr->n[1];
+			v3->n[2] +=vlr->n[2];
 			
 		}
 		if(do_nmap_tangent || do_tangent) {
@@ -3236,6 +3231,7 @@
 	int a, a1, ok, vertofs;
 	int end, do_autosmooth=0, totvert = 0;
 	int use_original_normals= 0;
+	int iCalcNormalsInRender = 0;	// false by default
 
 	me= ob->data;
 
@@ -3313,6 +3309,7 @@
 	
 	ma= give_render_material(re, ob, 1);
 
+
 	if(ma->material_type == MA_TYPE_HALO) {
 		make_render_halos(re, obr, me, totvert, mvert, ma, orco);
 	}
@@ -3321,8 +3318,17 @@
 		for(a=0; a<totvert; a++, mvert++) {
 			ver= RE_findOrAddVert(obr, obr->totvert++);
 			VECCOPY(ver->co, mvert->co);
+			ver->n[0]=mvert->no[0];
+			ver->n[1]=mvert->no[1];
+			ver->n[2]=mvert->no[2];
+			//dm->getVertNo(dm, a, ver->n);
 			if(do_autosmooth==0)	/* autosmooth on original unrotated data to prevent differences between frames */
+			{
 				mul_m4_v3(mat, ver->co);
+				mul_transposed_m3_v3(imat, ver->n);
+				normalize_v3(ver->n);
+				negate_v3(ver->n);
+			}
   
 			if(orco) {
 				ver->orco= orco;
@@ -3370,6 +3376,9 @@
 					end= dm->getNumFaces(dm);
 					mface= dm->getFaceArray(dm);
 
+					if(need_nmap_tangent!=0 && CustomData_get_layer_index(&dm->faceData, CD_TANGENT) == -1)
+						DM_add_tangent_layer(dm);
+					
 					for(a=0; a<end; a++, mface++) {
 						int v1, v2, v3, v4, flag;
 						
@@ -3400,10 +3409,14 @@
 									len= normal_tri_v3( vlr->n,mv[mf->v3].co, mv[mf->v2].co, mv[mf->v1].co);
 							}
 							else {
-								if(vlr->v4) 
+								/*if(vlr->v4) 
 									len= normal_quad_v3( vlr->n,vlr->v4->co, vlr->v3->co, vlr->v2->co, vlr->v1->co);
 								else 
-									len= normal_tri_v3( vlr->n,vlr->v3->co, vlr->v2->co, vlr->v1->co);
+									len= normal_tri_v3( vlr->n,vlr->v3->co, vlr->v2->co, vlr->v1->co);*/
+								len = dm->getFaceNo(dm, a, vlr->n);
+								mul_transposed_m3_v3(imat, vlr->n);
+								len *= normalize_v3(vlr->n);
+								negate_v3(vlr->n);
 							}
 
 							vlr->mat= ma;
@@ -3415,7 +3428,7 @@
 								CustomDataLayer *layer;
 								MTFace *mtface, *mtf;
 								MCol *mcol, *mc;
-								int index, mtfn= 0, mcn= 0;
+								int index, mtfn= 0, mcn= 0, mtng=0;
 								char *name;
 
 								for(index=0; index<dm->faceData.totlayer; index++) {
@@ -3432,6 +3445,22 @@
 										mcol= (MCol*)layer->data;
 										memcpy(mc, &mcol[a*4], sizeof(MCol)*4);
 									}
+									else if(layer->type == CD_TANGENT && mtng < 1)
+									{
+										if(need_nmap_tangent!=0)
+										{
+											const float * tangent = (const float *) layer->data;
+											int t;
+											int nr_verts = v4!=0 ? 4 : 3;
+											float * ftang = RE_vlakren_get_nmap_tangent(obr, vlr, 1);
+											for(t=0; t<nr_verts; t++)
+											{
+												QUATCOPY(ftang+t*4, tangent+a*16+t*4);
+												mul_mat3_m4_v3(mat, ftang+t*4);
+												normalize_v3(ftang+t*4);
+											}
+										}
+									}
 								}
 							}
 						}
@@ -3447,6 +3476,7 @@
 				MEdge *medge;
 				struct edgesort *edgetable;
 				int totedge= 0;
+				iCalcNormalsInRender=1;
 				
 				medge= dm->getEdgeArray(dm);
 				
@@ -3489,6 +3519,7 @@
 	
 	if(!timeoffset) {
 		if (test_for_displace(re, ob ) ) {
+			iCalcNormalsInRender = 1;
 			calc_vertexnormals(re, obr, 0, 0);
 			if(do_autosmooth)
 				do_displacement(re, obr, mat, imat);
@@ -3497,11 +3528,13 @@
 		}
 
 		if(do_autosmooth) {
+			iCalcNormalsInRender = 1;
 			autosmooth(re, obr, mat, me->smoothresh);
 		}
 
-		calc_vertexnormals(re, obr, need_tangent, need_nmap_tangent);
-
+		if(iCalcNormalsInRender!=0 || need_tangent!=0)
+			calc_vertexnormals(re, obr, need_tangent, need_nmap_tangent);
+		
 		if(need_stress)
 			calc_edge_stress(re, obr, me);
 	}
Index: source/blender/blenkernel/intern/cdderivedmesh.c
===================================================================
--- source/blender/blenkernel/intern/cdderivedmesh.c	(revision 35505)
+++ source/blender/blenkernel/intern/cdderivedmesh.c	(working copy)
@@ -179,6 +179,54 @@
 	no_r[2] = no[2]/32767.f;
 }
 
+static void cdDM_getRealVertNo(DerivedMesh *dm, int face_index, int vert_num, float no_r[3])
+{
+	CDDerivedMesh *cddm = (CDDerivedMesh*) dm;
+	const int smoothnormal = (cddm->mface[face_index].flag & ME_SMOOTH);
+	if(!smoothnormal)	// flat
+	{
+		dm->getFaceNo(dm, face_index, no_r);
+	}
+	else
+	{
+		unsigned int indices[] = {	cddm->mface[face_index].v1, cddm->mface[face_index].v2,
+								cddm->mface[face_index].v3, cddm->mface[face_index].v4 };
+		dm->getVertNo(dm, indices[vert_num], no_r);
+		normalize_v3(no_r);
+	}
+}
+
+static float cdDM_getFaceNo(DerivedMesh *dm, int index, float no_r[3])
+{
+	float * nors = dm->getFaceDataArray(dm, CD_NORMAL);
+	float len;
+
+	if(nors)
+	{
+		VECCOPY(no_r, &nors[3*index]);
+		len = len_v3(no_r);
+	}
+	else
+	{
+		CDDerivedMesh *cddm = (CDDerivedMesh*) dm;
+		float * p0, * p1, * p2;
+		const int iGetNrVerts = cddm->mface[index].v4!=0 ? 4 : 3;
+		unsigned int indices[] = {	cddm->mface[index].v1, cddm->mface[index].v2,
+								cddm->mface[index].v3, cddm->mface[index].v4 };
+		p0 = cddm->mvert[indices[0]].co; p1 = cddm->mvert[indices[1]].co; p2 = cddm->mvert[indices[2]].co;
+		if(iGetNrVerts==4)
+		{
+			float * p3 = cddm->mvert[indices[3]].co;
+			len = normal_quad_v3( no_r, p0, p1, p2, p3);
+		}
+		else {
+			len = normal_tri_v3(no_r, p0, p1, p2);
+		}
+
+	}
+	return len;
+}
+
 static ListBase *cdDM_getFaceMap(Object *ob, DerivedMesh *dm)
 {
 	CDDerivedMesh *cddm = (CDDerivedMesh*) dm;
@@ -1480,6 +1528,8 @@
 	dm->getVertCos = cdDM_getVertCos;
 	dm->getVertCo = cdDM_getVertCo;
 	dm->getVertNo = cdDM_getVertNo;
+	dm->getRealVertNo = cdDM_getRealVertNo;
+	dm->getFaceNo = cdDM_getFaceNo;
 
 	dm->getPBVH = cdDM_getPBVH;
 	dm->getFaceMap = cdDM_getFaceMap;
@@ -1798,7 +1848,7 @@
 	int i;
 	int numVerts = dm->numVertData;
 	int numFaces = dm->numFaceData;
-	MFace *mf;
+	MFace *mfaces;
 	MVert *mv;
 
 	if(numVerts == 0) return;
@@ -1817,22 +1867,41 @@
 										 NULL, dm->numFaceData);
 
 	/* calculate face normals and add to vertex normals */
-	mf = CDDM_get_faces(dm);
-	for(i = 0; i < numFaces; i++, mf++) {
-		float *f_no = face_nors[i];
+	mfaces = CDDM_get_faces(dm);
+	for(i = 0; i < numFaces; i++) {
+		MFace * mf = &mfaces[i];
+		if((mf->flag&ME_SMOOTH)!=0)
+		{
+			float *f_no = face_nors[i];
 
-		if(mf->v4)
-			normal_quad_v3( f_no,mv[mf->v1].co, mv[mf->v2].co, mv[mf->v3].co, mv[mf->v4].co);
-		else
-			normal_tri_v3( f_no,mv[mf->v1].co, mv[mf->v2].co, mv[mf->v3].co);
+			if(mf->v4)
+				normal_quad_v3( f_no,mv[mf->v1].co, mv[mf->v2].co, mv[mf->v3].co, mv[mf->v4].co);
+			else
+				normal_tri_v3( f_no,mv[mf->v1].co, mv[mf->v2].co, mv[mf->v3].co);
 		
-		add_v3_v3(temp_nors[mf->v1], f_no);
-		add_v3_v3(temp_nors[mf->v2], f_no);
-		add_v3_v3(temp_nors[mf->v3], f_no);
-		if(mf->v4)
-			add_v3_v3(temp_nors[mf->v4], f_no);
+			add_v3_v3(temp_nors[mf->v1], f_no);
+			add_v3_v3(temp_nors[mf->v2], f_no);
+			add_v3_v3(temp_nors[mf->v3], f_no);
+			if(mf->v4)
+				add_v3_v3(temp_nors[mf->v4], f_no);
+		}
 	}
 
+	for(i = 0; i < numFaces; i++) {
+		MFace * mf = &mfaces[i];
+		if((mf->flag&ME_SMOOTH)!=0)
+		{
+			float *f_no = face_nors[i];
+
+			if(temp_nors[mf->v1][0]==0 && temp_nors[mf->v1][1]==0 && temp_nors[mf->v1][2]==0) VECCOPY(temp_nors[mf->v1], f_no);
+			if(temp_nors[mf->v2][0]==0 && temp_nors[mf->v2][1]==0 && temp_nors[mf->v2][2]==0) VECCOPY(temp_nors[mf->v2], f_no);
+			if(temp_nors[mf->v3][0]==0 && temp_nors[mf->v3][1]==0 && temp_nors[mf->v3][2]==0) VECCOPY(temp_nors[mf->v3], f_no);
+			if(mf->v4)
+				if(temp_nors[mf->v4][0]==0 && temp_nors[mf->v4][1]==0 && temp_nors[mf->v4][2]==0) VECCOPY(temp_nors[mf->v4], f_no);
+		}
+	}
+
+
 	/* normalize vertex normals and assign */
 	for(i = 0; i < numVerts; i++, mv++) {
 		float *no = temp_nors[i];
Index: source/blender/blenkernel/intern/mesh.c
===================================================================
--- source/blender/blenkernel/intern/mesh.c	(revision 35505)
+++ source/blender/blenkernel/intern/mesh.c	(working copy)
@@ -1280,19 +1280,35 @@
 
 	for (i=0; i<numFaces; i++) {
 		MFace *mf= &mfaces[i];
-		float *f_no= &fnors[i*3];
+		if((mf->flag&ME_SMOOTH)!=0)
+		{
+			float *f_no= &fnors[i*3];
 
-		if (mf->v4)
-			normal_quad_v3( f_no,mverts[mf->v1].co, mverts[mf->v2].co, mverts[mf->v3].co, mverts[mf->v4].co);
-		else
-			normal_tri_v3( f_no,mverts[mf->v1].co, mverts[mf->v2].co, mverts[mf->v3].co);
+			if (mf->v4)
+				normal_quad_v3( f_no,mverts[mf->v1].co, mverts[mf->v2].co, mverts[mf->v3].co, mverts[mf->v4].co);
+			else
+				normal_tri_v3( f_no,mverts[mf->v1].co, mverts[mf->v2].co, mverts[mf->v3].co);
 		
-		add_v3_v3(tnorms[mf->v1], f_no);
-		add_v3_v3(tnorms[mf->v2], f_no);
-		add_v3_v3(tnorms[mf->v3], f_no);
-		if (mf->v4)
-			add_v3_v3(tnorms[mf->v4], f_no);
+			add_v3_v3(tnorms[mf->v1], f_no);
+			add_v3_v3(tnorms[mf->v2], f_no);
+			add_v3_v3(tnorms[mf->v3], f_no);
+			if (mf->v4)
+				add_v3_v3(tnorms[mf->v4], f_no);
+		}
 	}
+	for (i=0; i<numFaces; i++) {
+		MFace *mf= &mfaces[i];
+		if((mf->flag&ME_SMOOTH)==0)
+		{
+			float *f_no= &fnors[i*3];
+			if(tnorms[mf->v1][0]==0 && tnorms[mf->v1][1]==0 && tnorms[mf->v1][2]==0) VECCOPY(tnorms[mf->v1], f_no);
+			if(tnorms[mf->v2][0]==0 && tnorms[mf->v2][1]==0 && tnorms[mf->v2][2]==0) VECCOPY(tnorms[mf->v2], f_no);
+			if(tnorms[mf->v3][0]==0 && tnorms[mf->v3][1]==0 && tnorms[mf->v3][2]==0) VECCOPY(tnorms[mf->v3], f_no);
+			if(mf->v4)
+				if(tnorms[mf->v4][0]==0 && tnorms[mf->v4][1]==0 && tnorms[mf->v4][2]==0) VECCOPY(tnorms[mf->v4], f_no);
+		}
+	}
+
 	for (i=0; i<numVerts; i++) {
 		MVert *mv= &mverts[i];
 		float *no= tnorms[i];
Index: source/blender/blenkernel/BKE_DerivedMesh.h
===================================================================
--- source/blender/blenkernel/BKE_DerivedMesh.h	(revision 35505)
+++ source/blender/blenkernel/BKE_DerivedMesh.h	(working copy)
@@ -206,9 +206,15 @@
 	/* Fill the array (of length .getNumVerts()) with all vertex locations */
 	void (*getVertCos)(DerivedMesh *dm, float (*cos_r)[3]);
 
-	/* Get vertex normal, undefined if index is not valid */
+	/* Get smooth vertex normal, undefined if index is not valid */
 	void (*getVertNo)(DerivedMesh *dm, int index, float no_r[3]);
 
+	/* Get real vertex normal, takes faces set to hard into account */
+	void (*getRealVertNo)(DerivedMesh *dm, int face_index, int vert_num, float no_r[3]);
+
+	/* Get face normal, returns the length before normalization */
+	float (*getFaceNo)(DerivedMesh *dm, int index, float no_r[3]);
+
 	/* Get a map of vertices to faces
 	 */
 	struct ListBase *(*getFaceMap)(struct Object *ob, DerivedMesh *dm);
Index: source/blender/editors/mesh/editmesh_lib.c
===================================================================
--- source/blender/editors/mesh/editmesh_lib.c	(revision 35505)
+++ source/blender/editors/mesh/editmesh_lib.c	(working copy)
@@ -2007,18 +2007,32 @@
 	}
 
 	for(efa= em->faces.first; efa; efa=efa->next) {
-		if(efa->v4) {
-			normal_quad_v3( efa->n,efa->v1->co, efa->v2->co, efa->v3->co, efa->v4->co);
-			cent_quad_v3(efa->cent, efa->v1->co, efa->v2->co, efa->v3->co, efa->v4->co);
-			add_v3_v3(efa->v4->no, efa->n);
+		if((efa->flag&ME_SMOOTH)!=0)
+		{
+			if(efa->v4) {
+				normal_quad_v3( efa->n,efa->v1->co, efa->v2->co, efa->v3->co, efa->v4->co);
+				cent_quad_v3(efa->cent, efa->v1->co, efa->v2->co, efa->v3->co, efa->v4->co);
+				add_v3_v3(efa->v4->no, efa->n);
+			}
+			else {
+				normal_tri_v3( efa->n,efa->v1->co, efa->v2->co, efa->v3->co);
+				cent_tri_v3(efa->cent, efa->v1->co, efa->v2->co, efa->v3->co);
+			}
+			add_v3_v3(efa->v1->no, efa->n);
+			add_v3_v3(efa->v2->no, efa->n);
+			add_v3_v3(efa->v3->no, efa->n);
 		}
-		else {
-			normal_tri_v3( efa->n,efa->v1->co, efa->v2->co, efa->v3->co);
-			cent_tri_v3(efa->cent, efa->v1->co, efa->v2->co, efa->v3->co);
+	}
+
+	for(efa= em->faces.first; efa; efa=efa->next) {
+		if((efa->flag&ME_SMOOTH)==0)
+		{
+			if(efa->v1->no[0]==0 && efa->v1->no[1]==0 && efa->v1->no[2]==0) VECCOPY(efa->v1->no, efa->n);
+			if(efa->v2->no[0]==0 && efa->v2->no[1]==0 && efa->v2->no[2]==0) VECCOPY(efa->v2->no, efa->n);
+			if(efa->v3->no[0]==0 && efa->v3->no[1]==0 && efa->v3->no[2]==0) VECCOPY(efa->v3->no, efa->n);
+			if(efa->v4)
+				if(efa->v4->no[0]==0 && efa->v4->no[1]==0 && efa->v4->no[2]==0) VECCOPY(efa->v4->no, efa->n);
 		}
-		add_v3_v3(efa->v1->no, efa->n);
-		add_v3_v3(efa->v2->no, efa->n);
-		add_v3_v3(efa->v3->no, efa->n);
 	}
 
 	/* following Mesh convention; we use vertex coordinate itself for normal in this case */


More information about the Bf-codereview mailing list