[brlcad-devel] Fwd: [brlcad-commits] SF.net SVN: brlcad:[50074] brlcad/trunk/src/librt

[Christopher S. M. <br...@ma...>]

Keith/Cliff,

I didn't test what all might be affected by a change I just made, but it looks to be NURBS trimming related.  There was a bug Coverity rightly detected that looks significant for edge-case rays where it wasn't setting the adjacent edge.  It's not clear if it fixes a significant rendering bug or causes a change (or both), but it does fix a problem with the logic.  Just a heads up if someone notices a change in outputs.

Cheers!
Sean


Begin forwarded message:

> From: br...@us...
> Date: April 16, 2012 9:35:50 AM EDT
> To: brl...@li...
> Subject: [brlcad-commits] SF.net SVN: brlcad:[50074] brlcad/trunk/src/librt
> Reply-To: brl...@li...
> 
> Revision: 50074
>          http://brlcad.svn.sourceforge.net/brlcad/?rev=50074&view=rev
> Author:   brlcad
> Date:     2012-04-16 13:35:49 +0000 (Mon, 16 Apr 2012)
> Log Message:
> -----------
> looks like BVNode<BV>::isTrimmed() is trying to set a pointer, so we need a pointer to it for it to persist back to the caller.  this fixes a deadcode condition detected by coverity (CID 209), which looks like a bonefide bug in the trimming code where it was keeping track of an adjacent trimmed face id (where it would always get set to -99).
> 
> Modified Paths:
> --------------
>    brlcad/trunk/src/librt/opennurbs_ext.h
>    brlcad/trunk/src/librt/primitives/brep/brep.cpp
>    brlcad/trunk/src/librt/primitives/brep/brep_debug.cpp
> 
> Modified: brlcad/trunk/src/librt/opennurbs_ext.h
> ===================================================================
> --- brlcad/trunk/src/librt/opennurbs_ext.h	2012-04-16 13:21:08 UTC (rev 50073)
> +++ brlcad/trunk/src/librt/opennurbs_ext.h	2012-04-16 13:35:49 UTC (rev 50074)
> @@ -836,7 +836,7 @@
>     ON_2dPoint getClosestPointEstimate(const ON_3dPoint& pt);
>     ON_2dPoint getClosestPointEstimate(const ON_3dPoint& pt, ON_Interval& u, ON_Interval& v);
>     int getLeavesBoundingPoint(const ON_3dPoint& pt, std::list<BVNode<BV> *>& out);
> -    int isTrimmed(const ON_2dPoint& uv, BRNode* closest, fastf_t &closesttrim);
> +    int isTrimmed(const ON_2dPoint& uv, BRNode** closest, fastf_t &closesttrim);
>     bool doTrimming() const;
> 
>     void getTrimsAbove(const ON_2dPoint& uv, std::list<BRNode*>& out_leaves);
> @@ -1184,14 +1184,16 @@
> 
> template<class BV>
> int
> -BVNode<BV>::isTrimmed(const ON_2dPoint& uv, BRNode* closest, fastf_t &closesttrim)
> +BVNode<BV>::isTrimmed(const ON_2dPoint& uv, BRNode** closest, fastf_t &closesttrim)
> {
>     BRNode* br;
>     std::list<BRNode*> trims;
> 
>     closesttrim = -1.0;
>     if (m_checkTrim) {
> +
> 	getTrimsAbove(uv, trims);
> +
> 	if (trims.empty()) {
> 	    return 1;
> 	} else {//find closest BB
> @@ -1232,14 +1234,16 @@
> 		fastf_t v;
> 		int trimstatus = br->isTrimmed(uv, v);
> 		if (v >= 0.0) {
> -		    if (closest == NULL) {
> +		    if (closest && *closest == NULL) {
> 			currHeight = v;
> 			currTrimStatus = trimstatus;
> -			closest = br;
> +			if (closest)
> +			    *closest = br;
> 		    } else if (v < currHeight) {
> 			currHeight = v;
> 			currTrimStatus = trimstatus;
> -			closest = br;
> +			if (closest)
> +			    *closest = br;
> 		    }
> 		} else {
> 		    double dist = fabs(v);
> @@ -1254,25 +1258,29 @@
> 		    }
> 		}
> 	    }
> -	    if (closest == NULL) {
> +	    if (closest && *closest == NULL) {
> 		if (verticalTrim) {
> 		    closesttrim = vdist;
> -		    closest = vclosest;
> +		    if (closest)
> +			*closest = vclosest;
> 		}
> 		if ((underTrim) && (!verticalTrim || (udist < closesttrim))) {
> 		    closesttrim = udist;
> -		    closest = uclosest;
> +		    if (closest)
> +			*closest = uclosest;
> 		}
> 		return 1;
> 	    } else {
> 		closesttrim = currHeight;
> 		if ((verticalTrim) && (vdist < closesttrim)) {
> 		    closesttrim = vdist;
> -		    closest = vclosest;
> +		    if (closest)
> +			*closest = vclosest;
> 		}
> 		if ((underTrim) && (udist < closesttrim)) {
> 		    closesttrim = udist;
> -		    closest = uclosest;
> +		    if (closest)
> +			*closest = uclosest;
> 		}
> 		return currTrimStatus;
> 	    }
> 
> Modified: brlcad/trunk/src/librt/primitives/brep/brep.cpp
> ===================================================================
> --- brlcad/trunk/src/librt/primitives/brep/brep.cpp	2012-04-16 13:21:08 UTC (rev 50073)
> +++ brlcad/trunk/src/librt/primitives/brep/brep.cpp	2012-04-16 13:35:49 UTC (rev 50074)
> @@ -1178,7 +1178,7 @@
> 	for (int i = 0; i < numhits; i++) {
> 	    fastf_t closesttrim;
> 	    BRNode* trimBR = NULL;
> -	    int trim_status = ((BBNode*) sbv)->isTrimmed(ouv[i], trimBR, closesttrim);
> +	    int trim_status = ((BBNode*) sbv)->isTrimmed(ouv[i], &trimBR, closesttrim);
> 	    if (trim_status != 1) {
> 		ON_3dPoint _pt;
> 		ON_3dVector _norm(N[i]);
> @@ -1194,7 +1194,7 @@
> 			    (const fastf_t*) _norm, uv);
> 		bh.trimmed = false;
> 		if (trimBR != NULL) {
> -		    bh.m_adj_face_index = btrimBR->m_adj_face_index;
> +		    bh.m_adj_face_index = trimBR->m_adj_face_index;
> 		} else {
> 		    bh.m_adj_face_index = -99;
> 		}
> @@ -1274,7 +1274,7 @@
>     if ((sbv->m_u[0] < ouv[0]) && (sbv->m_u[1] > ouv[0]) &&
> 	(sbv->m_v[0] < ouv[1]) && (sbv->m_v[1] > ouv[1])) {
> 	BRNode* trimBR = NULL;
> -	int trim_status = ((BBNode*)sbv)->isTrimmed(ouv, trimBR, closesttrim);
> +	int trim_status = ((BBNode*)sbv)->isTrimmed(ouv, &trimBR, closesttrim);
> 	if (converged && (t > 1.e-2)) {
> 	    if (trim_status != 1) {
> 		hit = true;
> @@ -1350,7 +1350,7 @@
>     }
> 
>     BRNode* trimBR = NULL;
> -    int trim_status = ((BBNode*)sbv)->isTrimmed(uv, trimBR, closesttrim);
> +    int trim_status = ((BBNode*)sbv)->isTrimmed(uv, &trimBR, closesttrim);
>     if ((found > 0) &&  (trim_status != 1)) {
> 	ON_3dPoint _pt;
> 	ON_3dVector _norm;
> 
> Modified: brlcad/trunk/src/librt/primitives/brep/brep_debug.cpp
> ===================================================================
> --- brlcad/trunk/src/librt/primitives/brep/brep_debug.cpp	2012-04-16 13:21:08 UTC (rev 50073)
> +++ brlcad/trunk/src/librt/primitives/brep/brep_debug.cpp	2012-04-16 13:35:49 UTC (rev 50074)
> @@ -271,8 +271,8 @@
> 	    uv[0].y = v;
> 	    uv[1].x = u+uinc;
> 	    uv[1].y = v+vinc;
> -	    trim1_status = bb->isTrimmed(uv[0], trimBR, closesttrim1);
> -	    trim2_status = bb->isTrimmed(uv[1], trimBR, closesttrim2);
> +	    trim1_status = bb->isTrimmed(uv[0], &trimBR, closesttrim1);
> +	    trim2_status = bb->isTrimmed(uv[1], &trimBR, closesttrim2);
> 
> 	    if (((trim1_status != 1) || (fabs(closesttrim1) < BREP_EDGE_MISS_TOLERANCE)) &&
> 		((trim2_status != 1) || (fabs(closesttrim2) < BREP_EDGE_MISS_TOLERANCE))) {
> 
> This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.
> 
> 
> ------------------------------------------------------------------------------
> For Developers, A Lot Can Happen In A Second.
> Boundary is the first to Know...and Tell You.
> Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
> http://p.sf.net/sfu/Boundary-d2dvs2
> _______________________________________________
> BRL-CAD Source Commits mailing list
> brl...@li...
> https://lists.sourceforge.net/lists/listinfo/brlcad-commits