I'm very worried about this change. While I don't doubt that the new tolerance values may have made a particular conversion case better, I'd be surprised if the sensitivity across random inputs is improved by setting those "less strict" tolerances. The magic numbers are now merely tuned to a specific set of input geometry. It could very well make conversion worse for other geometry.
I'll see if the new conversion script shows an improvement, but a more general solution is needed. If there is a bn_tol struct, that is an alternative source of tolerance values less strict than VUNITIZE_TOL and SMALL_FASTF. If having a specific magic number other than one of the well-defined defaults (like VUNITIZE_TOL and SMALL_FASTF), then it should also be documented in detail so someone else isn't left guessing why such specific values are in the code. Cheers! Sean On Nov 24, 2010, at 9:07 PM, [email protected] wrote: > Revision: 41463 > http://brlcad.svn.sourceforge.net/brlcad/?rev=41463&view=rev > Author: r_weiss > Date: 2010-11-25 02:07:28 +0000 (Thu, 25 Nov 2010) > > Log Message: > ----------- > Made changes to functions bn_isect_line3_line3 and bn_coplanar. > Updated most of the tolerances in these functions using values > determined by capturing values during test runs and determining > where the values converge to 0, 1, -1 etc. These tolerances are not > perfect and I believe these values should be computed since they > can vary. These tolerance changes appear to improve the results of > the mged 'ev' command and 'facetize' command and fewer error > messages are reported. > > Modified Paths: > -------------- > brlcad/trunk/src/libbn/plane.c > > Modified: brlcad/trunk/src/libbn/plane.c > =================================================================== > --- brlcad/trunk/src/libbn/plane.c 2010-11-24 20:37:44 UTC (rev 41462) > +++ brlcad/trunk/src/libbn/plane.c 2010-11-25 02:07:28 UTC (rev 41463) > @@ -1235,7 +1235,6 @@ > int parallel = 0; > int colinear = 0; > > - > BN_CK_TOL(tol); > > if (NEAR_ZERO(MAGSQ(c), VUNITIZE_TOL) || NEAR_ZERO(MAGSQ(d), > VUNITIZE_TOL)) { > @@ -1254,11 +1253,12 @@ > > VCROSS(n, d, c); > det = VDOT(n, p) - VDOT(n, a); > - if (!NEAR_ZERO(det, SMALL_FASTF)) { > + > + if (!NEAR_ZERO(det, 1.0e-10)) { > return -1; /* no intersection, lines not in same plane */ > } > > - if (NEAR_ZERO(MAGSQ(n), VUNITIZE_TOL)) { > + if (NEAR_ZERO(MAGSQ(n), 1.0e-19)) { > /* lines are parallel, must find another way to get normal vector */ > vect_t a_to_p; > > @@ -1266,7 +1266,7 @@ > VSUB2(a_to_p, p, a); > VCROSS(n, a_to_p, d); > > - if (NEAR_ZERO(MAGSQ(n), VUNITIZE_TOL)) { > + if (NEAR_ZERO(MAGSQ(n), 1.0e-19)) { > /* lines are parallel and colinear */ > > colinear = 1; > @@ -1274,7 +1274,7 @@ > } > } > > - if (NEAR_ZERO(MAGSQ(n), VUNITIZE_TOL)) { > + if (NEAR_ZERO(MAGSQ(n), 1.0e-19)) { > bu_bomb("bn_isect_line3_line3(): ortho vector zero magnitude\n"); > } > > @@ -1370,11 +1370,9 @@ > */ > VSUB2(h, a, p); > det = c[q] * d[r] - d[q] * c[r]; > - det1 = (c[q] * h[r] - h[q] * c[r]); /* see below */ > - /* XXX This should be no smaller than 1e-16. See > - * bn_isect_line2_line2 for details. > - */ > - if (NEAR_ZERO(det, DETERMINANT_TOL)) { > + det1 = (c[q] * h[r] - h[q] * c[r]); > + > + if (NEAR_ZERO(det, 1.0e-10)) { > /* Lines are parallel */ > if (!colinear || !NEAR_ZERO(det1, DETERMINANT_TOL)) { > return -2; /* parallel, not colinear, no intersection */ > @@ -1420,15 +1418,9 @@ > > /* Check that these values of t and u satisfy the 3rd equation as > * well! > - * > - * XXX It isn't clear that "det" is exactly a model-space > - * distance. > */ > det = *t * d[s] - *u * c[s] - h[s]; > - if (!NEAR_ZERO(det, SMALL_FASTF)) { > - /* XXX This tolerance needs to be much less loose than > - * SQRT_SMALL_FASTF. What about DETERMINANT_TOL? > - */ > + if (!NEAR_ZERO(det, 1.0e-23)) { > /* Inconsistent solution, lines miss each other */ > return -1; > } > @@ -2244,15 +2236,16 @@ > } > > dot = VDOT(a, b); > + > VSCALE(pt_a, a, a[3]); > VSCALE(pt_b, b, b[3]); > > - if (NEAR_ZERO(dot, SMALL_FASTF)) { > + if (NEAR_ZERO(dot, 1.0e-8)) { > return 0; /* planes are perpendicular */ > } > > - /* parallel is when dot is within SMALL_FASTF of either -1 or > 1 */ > - if ((dot <= -SMALL_FASTF) ? (NEAR_ZERO(dot + 1.0, > SMALL_FASTF)) : (NEAR_ZERO(dot - 1.0, SMALL_FASTF))) { > + /* parallel is when dot is within 4.5e-16 of either -1 or 1 */ > + if ((dot <= -SMALL_FASTF) ? (NEAR_ZERO(dot + 1.0, 4.5e-16)) : > (NEAR_ZERO(dot - 1.0, 4.5e-16))) { > if (bn_pt3_pt3_equal(pt_a, pt_b, tol)) { > /* test for coplanar */ > if (dot >= SMALL_FASTF) { > > > This was sent by the SourceForge.net collaborative development > platform, the world's largest Open Source development site. > > ---------------------------------------------------------------------- > -------- > Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! > Tap into the largest installed PC base & get more eyes on your game by > optimizing for Intel(R) Graphics Technology. Get started today with > the > Intel(R) Software Partner Program. Five $500 cash prizes are up for > grabs. > http://p.sf.net/sfu/intelisp-dev2dev > _______________________________________________ > BRL-CAD Source Commits mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/brlcad-commits ------------------------------------------------------------------------------ Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! Tap into the largest installed PC base & get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs. http://p.sf.net/sfu/intelisp-dev2dev _______________________________________________ BRL-CAD Developer mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/brlcad-devel
