New auto smooth creates artifacts with flat shaded faces(BI) #39735

New Issue

Nahuel Belich · 2014-04-14T17:09:35+02:00

Nahuel Belich commented

2014-04-14 17:09:35 +02:00

System Information
13.10 ubuntu - nvidia 470gtx
Windows 7 - nvidia 570

Blender Version
Broken: Hash: 2ab9a0f

Short description of error
Check the attached picture.
Turning on "Auto Smooth" under object option generate some weird thin artifacts around flatten faces, this happens only at render time(viewport preview or final render) using blender internal, cycles works fine.

Exact steps for others to reproduce the error
1- open attached .blend
2- start any render

from scratch
1- add a monkey
2- add a subsurf modifier
3- switch to blender internal and start viewport render
4- turn on auto smooth
5- look close

referen image

auto_smooth_-_shade_flat.blend blend file

**System Information** 13.10 ubuntu - nvidia 470gtx Windows 7 - nvidia 570 **Blender Version** Broken: Hash: 2ab9a0f **Short description of error** Check the attached picture. Turning on "Auto Smooth" under object option generate some weird thin artifacts around flatten faces, this happens only at render time(viewport preview or final render) using blender internal, cycles works fine. **Exact steps for others to reproduce the error** 1- open attached .blend 2- start any render ----- from scratch 1- add a monkey 2- add a subsurf modifier 3- switch to blender internal and start viewport render 4- turn on auto smooth 5- look close ![PrtScr_capture.jpg](https://archive.blender.org/developer/F84857/PrtScr_capture.jpg) referen image [auto_smooth_-_shade_flat.blend](https://archive.blender.org/developer/F84858/auto_smooth_-_shade_flat.blend) blend file

Nahuel Belich commented

2014-04-14 17:09:35 +02:00

Changed status to: 'Open'

Nahuel Belich commented

2014-04-14 17:09:35 +02:00

Added subscriber: @NahuelBelich

Nahuel Belich commented

2014-04-14 17:13:33 +02:00

by the way, an off topic
don't know way that "2-start any render" its bold and bigger and i cant edit my reports, the site doesn't allow me

by the way, an off topic don't know way that "2-start any render" its bold and bigger and i cant edit my reports, the site doesn't allow me

Willi commented

2014-04-14 18:15:32 +02:00

Added subscriber: @willi-2

Willi commented

2014-04-14 18:15:32 +02:00

Additional info: Same problem with edge split modifier, also after applying modifier. At least back to Blender 2.63

Note: After triangulating the quads, the error disappears. After Alt+J to create quads from tris again, the error is still gone! (Maybe something to do with tesselation and vertex order?)

Additional info: Same problem with edge split modifier, also after applying modifier. At least back to Blender 2.63 Note: After triangulating the quads, the error disappears. After Alt+J to create quads from tris again, the error is still gone! (Maybe something to do with tesselation and vertex order?)

Bastien Montagne self-assigned this 2014-04-14 18:15:35 +02:00

Bastien Montagne commented

2014-04-14 18:15:35 +02:00

(About 2-, it’s because you had '------' just below, phabricator uses some remarkup-like syntax ;) ).

Could not reproduce it from scratch, but can confirm it with your file… Note however issue is the same with old autosmooth of BI (you can verify it with official 2.70a). Probably some float precision issue? Will try to investigate.

(About 2-, it’s because you had '------' just below, phabricator uses some remarkup-like syntax ;) ). Could not reproduce it from scratch, but can confirm it with your file… Note however issue is the same with old autosmooth of BI (you can verify it with official 2.70a). Probably some float precision issue? Will try to investigate.

Dave Holloway commented

2014-04-16 13:56:15 +02:00

Added subscriber: @daveh

Dave Holloway commented

2014-04-16 13:56:15 +02:00

Not sure if this is the same bug, but I have found most of the primatives, and I suspect every object has one badly shaded face. At first I thought it was flat shaded, but on closer examination, I think it is smooth shaded with the direction reversed from the rest of the mesh.
http://blenderartists.org/forum/showthread.php?318765-Blender-2-7x-development-thread&p=2625611&viewfull=1#post2625611

Not sure if this is the same bug, but I have found most of the primatives, and I suspect every object has one badly shaded face. At first I thought it was flat shaded, but on closer examination, I think it is smooth shaded with the direction reversed from the rest of the mesh. http://blenderartists.org/forum/showthread.php?318765-Blender-2-7x-development-thread&p=2625611&viewfull=1#post2625611

Bastien Montagne commented

2014-04-16 15:14:34 +02:00

@daveh: other bug, please make a new bug report (one bug per report). :)

Bastien Montagne commented

2014-04-16 16:57:51 +02:00

@daveh: fixed as a7120b9730.

@daveh: fixed as a7120b9730.

Nahuel Belich commented

2014-04-16 17:14:07 +02:00

@mont thanks for the tip about "---" ill take didn't know about it ^^, and also for the fix, ill test it as soon as i get a new version from builder or graphic all.

Bastien Montagne commented

2014-04-16 18:13:17 +02:00

@NahuelBelich arg! That’s why we need one bug, one report! I fixed issue reported by @daveh, not the one you reported… :/

Bastien Montagne commented

2014-04-17 18:31:54 +02:00

Added subscriber: @brecht

Bastien Montagne commented

2014-04-17 18:31:54 +02:00

Grmll… I had the idea this was some kind of precision glitch, and thought offsetting slightly split vertices would do the trick. So I spent two hours writing following patch, but it changes nearly nothing. :(

P42: #39735

diff --git a/source/blender/render/intern/source/convertblender.c b/source/blender/render/intern/source/convertblender.c
index a9e2e1c..0799abd 100644
--- a/source/blender/render/intern/source/convertblender.c
+++ b/source/blender/render/intern/source/convertblender.c
@@ -548,10 +548,11 @@ static VertRen *as_findvertex_lnor(VlakRen *vlr, VertRen *ver, ASvert *asv, cons
 		for (a = 0; a < 4; a++) {
 			if (asf->vlr- [x] && asf->vlr- [x] != vlr) {
 				/* this face already made a copy for this vertex! */
-				if (asf->nver[a]) {
-					if (equals_v3v3(lnor, asf->nver- [x]->n)) {
-						return asf->nver[a];
-					}
+				VertRen *nver = asf->nver[a];
+
+				/* this face already made a copy for this vertex! */
+				if (!ELEM(nver, NULL, ver) && equals_v3v3(lnor, nver->n)) {
+					return nver;
 				}
 			}
 		}
@@ -583,8 +584,8 @@ static void as_addvert_lnor(ObjectRen *obr, ASvert *asv, VertRen *ver, VlakRen *
 		v1 = RE_vertren_copy(obr, ver);
 		copy_v3_v3(v1->n, lnor);
 	}
+	asf->nver[asf_idx] = v1;
 	if (v1 != ver) {
-		asf->nver[asf_idx] = v1;
 		if (vlr->v1 == ver) vlr->v1 = v1;
 		if (vlr->v2 == ver) vlr->v2 = v1;
 		if (vlr->v3 == ver) vlr->v3 = v1;
@@ -592,6 +593,110 @@ static void as_addvert_lnor(ObjectRen *obr, ASvert *asv, VertRen *ver, VlakRen *
 	}
 }
 
+static bool as_ver_in_array(VertRen *ver, VertRen **arr, int size)
+{
+	VertRen **ver_it = arr;
+	int a;
+
+	for (a = 0; a < size && *ver_it != ver; ++a, ++ver_it) {}
+
+	return (a != size);
+}
+
+static void as_tweak_vcos(ASvert *asv)
+{
+	ASface *asf;
+	const float fac = 1e-4f;
+	const int tot_asf = asv->totface;
+	int a, tot_done_verts = 0;
+
+	VertRen *done_verts_buff[16] = {
+	    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+	};
+	VertRen **done_verts = (tot_asf < 17) ? done_verts_buff : MEM_callocN(sizeof(VertRen *) * tot_asf, __func__);
+
+	VertRen *work_verts_buff[32] = {
+	    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+	    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+	};
+	VertRen **work_verts = (tot_asf < 17) ? work_verts_buff : MEM_callocN(sizeof(VertRen *) * tot_asf * 2, __func__);
+
+	asf = asv->faces.first;
+	while (asf) {
+		for (a = 0; a < 4; ++a) {
+			VertRen *ver = asf->nver[a];
+			float n- [x], d- [x] = {0.0f, 0.0f, 0.0f};
+			ASface *asf_it = asf;
+			int b = a, tot_work_verts = 0;
+
+			if (!ver || as_ver_in_array(ver, done_verts, tot_asf)) {
+				continue;
+			}
+
+			/* this ver has not yet been handled. */
+			while (asf_it) {
+				for (; b < 4; ++b) {
+					if (asf_it->nver- [x] == ver) {
+						VlakRen *vlr = asf_it->vlr[b];
+						VertRen *v1, *v2;
+
+						BLI_assert(vlr != NULL);
+
+						if (ver == vlr->v1) {
+							v1 = vlr->v2;
+							v2 = vlr->v4 ? vlr->v4 : vlr->v3;
+						}
+						else if (ver == vlr->v2) {
+							v1 = vlr->v3;
+							v2 = vlr->v1;
+						}
+						else if (ver == vlr->v3) {
+							v1 = vlr->v4 ? vlr->v4 : vlr->v1;
+							v2 = vlr->v2;
+						}
+						else if (ver == vlr->v4) {
+							v1 = vlr->v1;
+							v2 = vlr->v3;
+						}
+
+						if (!as_ver_in_array(v1, work_verts, tot_work_verts)) {
+							sub_v3_v3v3(n, ver->co, v1->co);
+							madd_v3_v3fl(d, n, fac);
+							work_verts[tot_work_verts++] = v1;
+						}
+						if (!as_ver_in_array(v2, work_verts, tot_work_verts)) {
+							sub_v3_v3v3(n, ver->co, v2->co);
+							madd_v3_v3fl(d, n, fac);
+							work_verts[tot_work_verts++] = v2;
+						}
+
+						BLI_assert(tot_work_verts <= tot_asf * 2);
+					}
+				}
+
+				b = 0;
+				asf_it = asf_it->next;
+			}
+
+			if (!is_zero_v3(d)) {
+				add_v3_v3(ver->co, d);
+			}
+
+			done_verts[tot_done_verts++] = ver;
+
+			BLI_assert(tot_done_verts <= tot_asf);
+		}
+		asf = asf->next;
+	}
+
+	if (done_verts != done_verts_buff) {
+		MEM_freeN(done_verts);
+	}
+	if (work_verts != work_verts_buff) {
+		MEM_freeN(work_verts);
+	}
+}
+
 /* note; autosmooth happens in object space still, after applying autosmooth we rotate */
 /* note2; actually, when original mesh and displist are equal sized, face normals are from original mesh */
 static void autosmooth(Render *UNUSED(re), ObjectRen *obr, float mat- [x]- [x], short (*lnors)- [x][3])
@@ -624,13 +729,14 @@ static void autosmooth(Render *UNUSED(re), ObjectRen *obr, float mat- [x]- [x], shor
 		}
 	}
 
-	/* free */
+	/* tweak slightly coordinates of 'split' vertices, to avoids blackdots artifacts (see #39735), and free.*/
 	for (a = 0; a < totvert; a++) {
+		as_tweak_vcos(&asverts[a]);
 		BLI_freelistN(&asverts- [x].faces);
 	}
 	MEM_freeN(asverts);
 
-	/* rotate vertices and calculate normal of faces */
+	/* rotate vertices */
 	for (a = 0; a < obr->totvert; a++) {
 		ver = RE_findOrAddVert(obr, a);
 		mul_m4_v3(mat, ver->co);
@@ -640,6 +746,7 @@ static void autosmooth(Render *UNUSED(re), ObjectRen *obr, float mat- [x]- [x], shor
 			normalize_v3(ver->n);
 		}
 	}
+	/* calculate normal of faces */
 	for (a = 0; a < obr->totvlak; a++) {
 		vlr = RE_findOrAddVlak(obr, a);

Brecht, I’d like to summon you here, I do not know BI well enough to understand fully what happens, and how to fix this, if possible!

Grmll… I had the idea this was some kind of precision glitch, and thought offsetting slightly split vertices would do the trick. So I spent two hours writing following patch, but it changes nearly nothing. :( [P42: #39735](https://archive.blender.org/developer/P42.txt) ```diff diff --git a/source/blender/render/intern/source/convertblender.c b/source/blender/render/intern/source/convertblender.c index a9e2e1c..0799abd 100644 --- a/source/blender/render/intern/source/convertblender.c +++ b/source/blender/render/intern/source/convertblender.c @@ -548,10 +548,11 @@ static VertRen *as_findvertex_lnor(VlakRen *vlr, VertRen *ver, ASvert *asv, cons for (a = 0; a < 4; a++) { if (asf->vlr- [x] && asf->vlr- [x] != vlr) { /* this face already made a copy for this vertex! */ - if (asf->nver[a]) { - if (equals_v3v3(lnor, asf->nver- [x]->n)) { - return asf->nver[a]; - } + VertRen *nver = asf->nver[a]; + + /* this face already made a copy for this vertex! */ + if (!ELEM(nver, NULL, ver) && equals_v3v3(lnor, nver->n)) { + return nver; } } } @@ -583,8 +584,8 @@ static void as_addvert_lnor(ObjectRen *obr, ASvert *asv, VertRen *ver, VlakRen * v1 = RE_vertren_copy(obr, ver); copy_v3_v3(v1->n, lnor); } + asf->nver[asf_idx] = v1; if (v1 != ver) { - asf->nver[asf_idx] = v1; if (vlr->v1 == ver) vlr->v1 = v1; if (vlr->v2 == ver) vlr->v2 = v1; if (vlr->v3 == ver) vlr->v3 = v1; @@ -592,6 +593,110 @@ static void as_addvert_lnor(ObjectRen *obr, ASvert *asv, VertRen *ver, VlakRen * } } +static bool as_ver_in_array(VertRen *ver, VertRen **arr, int size) +{ + VertRen **ver_it = arr; + int a; + + for (a = 0; a < size && *ver_it != ver; ++a, ++ver_it) {} + + return (a != size); +} + +static void as_tweak_vcos(ASvert *asv) +{ + ASface *asf; + const float fac = 1e-4f; + const int tot_asf = asv->totface; + int a, tot_done_verts = 0; + + VertRen *done_verts_buff[16] = { + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, + }; + VertRen **done_verts = (tot_asf < 17) ? done_verts_buff : MEM_callocN(sizeof(VertRen *) * tot_asf, __func__); + + VertRen *work_verts_buff[32] = { + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, + }; + VertRen **work_verts = (tot_asf < 17) ? work_verts_buff : MEM_callocN(sizeof(VertRen *) * tot_asf * 2, __func__); + + asf = asv->faces.first; + while (asf) { + for (a = 0; a < 4; ++a) { + VertRen *ver = asf->nver[a]; + float n- [x], d- [x] = {0.0f, 0.0f, 0.0f}; + ASface *asf_it = asf; + int b = a, tot_work_verts = 0; + + if (!ver || as_ver_in_array(ver, done_verts, tot_asf)) { + continue; + } + + /* this ver has not yet been handled. */ + while (asf_it) { + for (; b < 4; ++b) { + if (asf_it->nver- [x] == ver) { + VlakRen *vlr = asf_it->vlr[b]; + VertRen *v1, *v2; + + BLI_assert(vlr != NULL); + + if (ver == vlr->v1) { + v1 = vlr->v2; + v2 = vlr->v4 ? vlr->v4 : vlr->v3; + } + else if (ver == vlr->v2) { + v1 = vlr->v3; + v2 = vlr->v1; + } + else if (ver == vlr->v3) { + v1 = vlr->v4 ? vlr->v4 : vlr->v1; + v2 = vlr->v2; + } + else if (ver == vlr->v4) { + v1 = vlr->v1; + v2 = vlr->v3; + } + + if (!as_ver_in_array(v1, work_verts, tot_work_verts)) { + sub_v3_v3v3(n, ver->co, v1->co); + madd_v3_v3fl(d, n, fac); + work_verts[tot_work_verts++] = v1; + } + if (!as_ver_in_array(v2, work_verts, tot_work_verts)) { + sub_v3_v3v3(n, ver->co, v2->co); + madd_v3_v3fl(d, n, fac); + work_verts[tot_work_verts++] = v2; + } + + BLI_assert(tot_work_verts <= tot_asf * 2); + } + } + + b = 0; + asf_it = asf_it->next; + } + + if (!is_zero_v3(d)) { + add_v3_v3(ver->co, d); + } + + done_verts[tot_done_verts++] = ver; + + BLI_assert(tot_done_verts <= tot_asf); + } + asf = asf->next; + } + + if (done_verts != done_verts_buff) { + MEM_freeN(done_verts); + } + if (work_verts != work_verts_buff) { + MEM_freeN(work_verts); + } +} + /* note; autosmooth happens in object space still, after applying autosmooth we rotate */ /* note2; actually, when original mesh and displist are equal sized, face normals are from original mesh */ static void autosmooth(Render *UNUSED(re), ObjectRen *obr, float mat- [x]- [x], short (*lnors)- [x][3]) @@ -624,13 +729,14 @@ static void autosmooth(Render *UNUSED(re), ObjectRen *obr, float mat- [x]- [x], shor } } - /* free */ + /* tweak slightly coordinates of 'split' vertices, to avoids blackdots artifacts (see #39735), and free.*/ for (a = 0; a < totvert; a++) { + as_tweak_vcos(&asverts[a]); BLI_freelistN(&asverts- [x].faces); } MEM_freeN(asverts); - /* rotate vertices and calculate normal of faces */ + /* rotate vertices */ for (a = 0; a < obr->totvert; a++) { ver = RE_findOrAddVert(obr, a); mul_m4_v3(mat, ver->co); @@ -640,6 +746,7 @@ static void autosmooth(Render *UNUSED(re), ObjectRen *obr, float mat- [x]- [x], shor normalize_v3(ver->n); } } + /* calculate normal of faces */ for (a = 0; a < obr->totvlak; a++) { vlr = RE_findOrAddVlak(obr, a); ``` Brecht, I’d like to summon you here, I do not know BI well enough to understand fully what happens, and how to fix this, if possible!

Brecht Van Lommel commented

2014-04-17 19:01:13 +02:00

Blender Internal's raytracer depends on face adjacency tests to avoid some types of self intersection (where the ray starts from some face but due to precision issues intersects the face itself). If the edges are split that check is no longer possible. It seems the edgesplit modifier suffers from the same issue.

I'm not sure what to suggest, either this self intersection system needs to be revised, or you should store the normals in such a way that face adjacency does not change. This bug is as old as raytracing in Blender, not really related to the new autosmooth.

Blender Internal's raytracer depends on face adjacency tests to avoid some types of self intersection (where the ray starts from some face but due to precision issues intersects the face itself). If the edges are split that check is no longer possible. It seems the edgesplit modifier suffers from the same issue. I'm not sure what to suggest, either this self intersection system needs to be revised, or you should store the normals in such a way that face adjacency does not change. This bug is as old as raytracing in Blender, not really related to the new autosmooth.

Bastien Montagne commented

2014-04-18 15:56:44 +02:00

Thanks for the hint, Brecht. Here is a working patch:

P44: #39735

diff --git a/source/blender/render/intern/include/rayintersection.h b/source/blender/render/intern/include/rayintersection.h
index 3607e66..89d2c31 100644
--- a/source/blender/render/intern/include/rayintersection.h
+++ b/source/blender/render/intern/include/rayintersection.h
@@ -121,7 +121,7 @@ typedef struct Isect {
 
 /* arbitrary, but can't use e.g. FLT_MAX because of precision issues */
 #define RE_RAYTRACE_MAXDIST	1e15f
-#define RE_RAYTRACE_EPSILON 0.0f
+#define RE_RAYTRACE_EPSILON -1e-7f
 
 #ifdef __cplusplus
 }
diff --git a/source/blender/render/intern/include/renderdatabase.h b/source/blender/render/intern/include/renderdatabase.h
index 9b14560..da45a2b 100644
--- a/source/blender/render/intern/include/renderdatabase.h
+++ b/source/blender/render/intern/include/renderdatabase.h
@@ -33,6 +33,10 @@
 #ifndef __RENDERDATABASE_H__
 #define __RENDERDATABASE_H__
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 struct Object;
 struct VlakRen;
 struct VertRen;
@@ -159,6 +163,9 @@ void area_lamp_vectors(struct LampRen *lar);
 void init_render_world(Render *re);
 void RE_Database_FromScene_Vectors(Render *re, struct Main *bmain, struct Scene *sce, unsigned int lay);
 
+#ifdef __cplusplus
+}
+#endif
 
 #endif /* __RENDERDATABASE_H__ */
 
diff --git a/source/blender/render/intern/raytrace/rayobject.cpp b/source/blender/render/intern/raytrace/rayobject.cpp
index 9e63950..1ef44bb 100644
--- a/source/blender/render/intern/raytrace/rayobject.cpp
+++ b/source/blender/render/intern/raytrace/rayobject.cpp
@@ -43,6 +43,7 @@
 #include "rayobject.h"
 #include "raycounter.h"
 #include "render_types.h"
+#include "renderdatabase.h"
 
 /* RayFace
  *
@@ -326,15 +327,33 @@ MALWAYS_INLINE int intersect_rayface(RayObject *hit_obj, RayFace *face, Isect *i
 			if (dist < 0.1f && is->orig.ob == face->ob) {
 				VlakRen *a = (VlakRen *)is->orig.face;
 				VlakRen *b = (VlakRen *)face->face;
+				ObjectRen *obr = ((ObjectInstanceRen *)face->ob)->obr;
+
+				VertRen **va, **vb;
+				int *org_idx_a, *org_idx_b;
+				int i, j;
+				bool is_neighbor = false;
+
+				/* "same" vertex means either the actual same VertRen, or the same 'final org index', if available
+				 * (autosmooth only, currently). */
+				for (i = 0, va = &a->v1; !is_neighbor && i < 4 && *va; ++i, ++va) {
+					org_idx_a = RE_vertren_get_origindex(obr, *va, false);
+					for (j = 0, vb = &b->v1; !is_neighbor && j < 4 && *vb; ++j, ++vb) {
+						if (*va == *vb) {
+							is_neighbor = true;
+						}
+						else if (org_idx_a) {
+							org_idx_b = RE_vertren_get_origindex(obr, *vb, 0);
+							if (org_idx_b && *org_idx_a == *org_idx_b) {
+								is_neighbor = true;
+							}
+						}
+					}
+				}
 
-				/* so there's a shared edge or vertex, let's intersect ray with
-				 * face itself, if that's true we can safely return 1, otherwise
-				 * we assume the intersection is invalid, 0 */
-				if (a->v1 == b->v1 || a->v2 == b->v1 || a->v3 == b->v1 || a->v4 == b->v1 ||
-				    a->v1 == b->v2 || a->v2 == b->v2 || a->v3 == b->v2 || a->v4 == b->v2 ||
-				    a->v1 == b->v3 || a->v2 == b->v3 || a->v3 == b->v3 || a->v4 == b->v3 ||
-				    (b->v4 && (a->v1 == b->v4 || a->v2 == b->v4 || a->v3 == b->v4 || a->v4 == b->v4)))
-				{
+				/* So there's a shared edge or vertex, let's intersect ray with self, if that's true
+				 * we can safely return 1, otherwise we assume the intersection is invalid, 0 */
+				if (is_neighbor) {
 					/* create RayFace from original face, transformed if necessary */
 					RayFace origface;
 					ObjectInstanceRen *ob = (ObjectInstanceRen *)is->orig.ob;
diff --git a/source/blender/render/intern/source/convertblender.c b/source/blender/render/intern/source/convertblender.c
index a9e2e1c..908215f 100644
--- a/source/blender/render/intern/source/convertblender.c
+++ b/source/blender/render/intern/source/convertblender.c
@@ -3188,10 +3188,6 @@ static void init_render_mesh(Render *re, ObjectRen *obr, int timeoffset)
 		}
 		need_nmap_tangent= 1;
 	}
-	
-	/* origindex currently only used when baking to vertex colors */
-	if (re->flag & R_BAKING && re->r.bake_flag & R_BAKE_VCOL)
-		need_origindex= 1;
 
 	/* check autosmooth and displacement, we then have to skip only-verts optimize
 	 * Note: not sure what we want to give higher priority, currently do_displace
@@ -3201,7 +3197,10 @@ static void init_render_mesh(Render *re, ObjectRen *obr, int timeoffset)
 	do_autosmooth = ((me->flag & ME_AUTOSMOOTH) != 0) && !do_displace;
 	if (do_autosmooth || do_displace)
 		timeoffset = 0;
-	
+
+	/* origindex currently used when using autosmooth, or baking to vertex colors. */
+	need_origindex = (do_autosmooth || ((re->flag & R_BAKING) && (re->r.bake_flag & R_BAKE_VCOL)));
+
 	mask= CD_MASK_BAREMESH|CD_MASK_MTFACE|CD_MASK_MCOL;
 	if (!timeoffset)
 		if (need_orco)

Basic idea is, when autosmooth is enabled, to use orgidx for verts.

Then, when this data is available and both vertices pointers are not the same, we do an extra check over those org indices, which allows us to address the split vertices case.

Notes:

Check to find neighbor face might now be slower (no idea how much, quick test with an heavy mesh (>1.6M verts, > 3.3M tris) did not show any significant difference in common case, but about 8% slower when all the 1.6M verts are split, and the expected minor increase in peak mem usage, due to orgidx data, up to 4% in 'all verts split' worst case).
Is the usage of RE_vertren_get_origindex() in rayobject.cpp considered valid?
This is only working currently when enabling autosmooth, yet it can address SplitEdge case as well, should we consider enabling it always, to handle all modifiers that might create such cases?
Using real orgidx might generate 'false positives' in some rare cases (modifier stack making complex operations)? Would not consider this an issue, though.

Note I also had to tweak slightly the RE_RAYTRACE_EPSILON value, else I still get a few black dots in this file: untitled2.blend.

Thanks for the hint, Brecht. Here is a working patch: [P44: #39735](https://archive.blender.org/developer/P44.txt) ```diff diff --git a/source/blender/render/intern/include/rayintersection.h b/source/blender/render/intern/include/rayintersection.h index 3607e66..89d2c31 100644 --- a/source/blender/render/intern/include/rayintersection.h +++ b/source/blender/render/intern/include/rayintersection.h @@ -121,7 +121,7 @@ typedef struct Isect { /* arbitrary, but can't use e.g. FLT_MAX because of precision issues */ #define RE_RAYTRACE_MAXDIST 1e15f -#define RE_RAYTRACE_EPSILON 0.0f +#define RE_RAYTRACE_EPSILON -1e-7f #ifdef __cplusplus } diff --git a/source/blender/render/intern/include/renderdatabase.h b/source/blender/render/intern/include/renderdatabase.h index 9b14560..da45a2b 100644 --- a/source/blender/render/intern/include/renderdatabase.h +++ b/source/blender/render/intern/include/renderdatabase.h @@ -33,6 +33,10 @@ #ifndef __RENDERDATABASE_H__ #define __RENDERDATABASE_H__ +#ifdef __cplusplus +extern "C" { +#endif + struct Object; struct VlakRen; struct VertRen; @@ -159,6 +163,9 @@ void area_lamp_vectors(struct LampRen *lar); void init_render_world(Render *re); void RE_Database_FromScene_Vectors(Render *re, struct Main *bmain, struct Scene *sce, unsigned int lay); +#ifdef __cplusplus +} +#endif #endif /* __RENDERDATABASE_H__ */ diff --git a/source/blender/render/intern/raytrace/rayobject.cpp b/source/blender/render/intern/raytrace/rayobject.cpp index 9e63950..1ef44bb 100644 --- a/source/blender/render/intern/raytrace/rayobject.cpp +++ b/source/blender/render/intern/raytrace/rayobject.cpp @@ -43,6 +43,7 @@ #include "rayobject.h" #include "raycounter.h" #include "render_types.h" +#include "renderdatabase.h" /* RayFace * @@ -326,15 +327,33 @@ MALWAYS_INLINE int intersect_rayface(RayObject *hit_obj, RayFace *face, Isect *i if (dist < 0.1f && is->orig.ob == face->ob) { VlakRen *a = (VlakRen *)is->orig.face; VlakRen *b = (VlakRen *)face->face; + ObjectRen *obr = ((ObjectInstanceRen *)face->ob)->obr; + + VertRen **va, **vb; + int *org_idx_a, *org_idx_b; + int i, j; + bool is_neighbor = false; + + /* "same" vertex means either the actual same VertRen, or the same 'final org index', if available + * (autosmooth only, currently). */ + for (i = 0, va = &a->v1; !is_neighbor && i < 4 && *va; ++i, ++va) { + org_idx_a = RE_vertren_get_origindex(obr, *va, false); + for (j = 0, vb = &b->v1; !is_neighbor && j < 4 && *vb; ++j, ++vb) { + if (*va == *vb) { + is_neighbor = true; + } + else if (org_idx_a) { + org_idx_b = RE_vertren_get_origindex(obr, *vb, 0); + if (org_idx_b && *org_idx_a == *org_idx_b) { + is_neighbor = true; + } + } + } + } - /* so there's a shared edge or vertex, let's intersect ray with - * face itself, if that's true we can safely return 1, otherwise - * we assume the intersection is invalid, 0 */ - if (a->v1 == b->v1 || a->v2 == b->v1 || a->v3 == b->v1 || a->v4 == b->v1 || - a->v1 == b->v2 || a->v2 == b->v2 || a->v3 == b->v2 || a->v4 == b->v2 || - a->v1 == b->v3 || a->v2 == b->v3 || a->v3 == b->v3 || a->v4 == b->v3 || - (b->v4 && (a->v1 == b->v4 || a->v2 == b->v4 || a->v3 == b->v4 || a->v4 == b->v4))) - { + /* So there's a shared edge or vertex, let's intersect ray with self, if that's true + * we can safely return 1, otherwise we assume the intersection is invalid, 0 */ + if (is_neighbor) { /* create RayFace from original face, transformed if necessary */ RayFace origface; ObjectInstanceRen *ob = (ObjectInstanceRen *)is->orig.ob; diff --git a/source/blender/render/intern/source/convertblender.c b/source/blender/render/intern/source/convertblender.c index a9e2e1c..908215f 100644 --- a/source/blender/render/intern/source/convertblender.c +++ b/source/blender/render/intern/source/convertblender.c @@ -3188,10 +3188,6 @@ static void init_render_mesh(Render *re, ObjectRen *obr, int timeoffset) } need_nmap_tangent= 1; } - - /* origindex currently only used when baking to vertex colors */ - if (re->flag & R_BAKING && re->r.bake_flag & R_BAKE_VCOL) - need_origindex= 1; /* check autosmooth and displacement, we then have to skip only-verts optimize * Note: not sure what we want to give higher priority, currently do_displace @@ -3201,7 +3197,10 @@ static void init_render_mesh(Render *re, ObjectRen *obr, int timeoffset) do_autosmooth = ((me->flag & ME_AUTOSMOOTH) != 0) && !do_displace; if (do_autosmooth || do_displace) timeoffset = 0; - + + /* origindex currently used when using autosmooth, or baking to vertex colors. */ + need_origindex = (do_autosmooth || ((re->flag & R_BAKING) && (re->r.bake_flag & R_BAKE_VCOL))); + mask= CD_MASK_BAREMESH|CD_MASK_MTFACE|CD_MASK_MCOL; if (!timeoffset) if (need_orco) ``` Basic idea is, when autosmooth is enabled, to use orgidx for verts. Then, when this data is available and both vertices pointers are not the same, we do an extra check over those org indices, which allows us to address the split vertices case. Notes: * Check to find neighbor face might now be slower (no idea how much, quick test with an heavy mesh (>1.6M verts, > 3.3M tris) did not show any significant difference in common case, but about 8% slower when **all** the 1.6M verts are split, and the expected minor increase in peak mem usage, due to orgidx data, up to 4% in 'all verts split' worst case). * Is the usage of `RE_vertren_get_origindex()` in `rayobject.cpp` considered valid? * This is only working currently when enabling autosmooth, yet it can address SplitEdge case as well, should we consider enabling it always, to handle all modifiers that might create such cases? * Using real orgidx might generate 'false positives' in some rare cases (modifier stack making complex operations)? Would not consider this an issue, though. Note I also had to tweak slightly the RE_RAYTRACE_EPSILON value, else I still get a few black dots in this file: [untitled2.blend](https://archive.blender.org/developer/F85168/untitled2.blend).

Brecht Van Lommel commented

2014-04-18 17:33:15 +02:00

I think this is acceptable.
Yes, using RE_vertren_get_origindex here should be fine.
I don't think it's a good idea to get that kind of memory increase for all meshes, especially a heavily subsurfed one that does not even suffer from this problem. I would either not do it or check for the existence of an edgesplit modifier.
I don't think this is a problem.

Increasing RE_RAYTRACE_EPSILON gives light leaks, it's a tradeoff between that and self intersections. I don't know how significant the light leaks will be with 1e-7 in practice. This stuff has been tweaked for various bug fixes over the years if you look in the git history, you could test the files from those bugfixes to see if it breaks them.

* I think this is acceptable. * Yes, using `RE_vertren_get_origindex` here should be fine. * I don't think it's a good idea to get that kind of memory increase for all meshes, especially a heavily subsurfed one that does not even suffer from this problem. I would either not do it or check for the existence of an edgesplit modifier. * I don't think this is a problem. Increasing RE_RAYTRACE_EPSILON gives light leaks, it's a tradeoff between that and self intersections. I don't know how significant the light leaks will be with 1e-7 in practice. This stuff has been tweaked for various bug fixes over the years if you look in the git history, you could test the files from those bugfixes to see if it breaks them.

Bastien Montagne commented

2014-04-18 23:03:51 +02:00

Thanks for quick review and advices, Brecht. Will finish that asap.