From 741e429151b8994c19709ad25ffb4df75b69d5da Mon Sep 17 00:00:00 2001 From: rambetter Date: Mon, 3 Jan 2011 07:02:56 +0000 Subject: [PATCH] Improving little tidbits. - Removing redundant use of ourNormalEpsilon in SnapNormal(). The choice for normalEpsilon is good for the long run in my opinion, even with the change to SnapNormal(). - Adding "const" to function sig SnapPlaneImproved(), last arg. - Adding function CopyWindingAccuIncreaseSizeAndFreeOld() in polylib.c, needed by the chopping function. - Tying up loose ends in ChopWindingInPlaceAccu() in polylib.c. Adding comment related to duplicate plane. Chooseing maxpts more intelligently. If we hit the maxpts bound, no problem! We dynamically increase the size of the winding. git-svn-id: svn://svn.icculus.org/gtkradiant/GtkRadiant/branches/Rambetter-math-fix-experiments@404 8a3a26a2-13c4-0310-b231-cf6edde360e5 --- tools/quake3/common/polylib.c | 62 ++++++++++++++++++++++++++--------- tools/quake3/q3map2/map.c | 19 ++++------- 2 files changed, 54 insertions(+), 27 deletions(-) diff --git a/tools/quake3/common/polylib.c b/tools/quake3/common/polylib.c index adc686b9..17bf4076 100644 --- a/tools/quake3/common/polylib.c +++ b/tools/quake3/common/polylib.c @@ -461,6 +461,28 @@ winding_t *CopyWinding (winding_t *w) return c; } +/* +================== +CopyWindingAccuIncreaseSizeAndFreeOld +================== +*/ +winding_accu_t *CopyWindingAccuIncreaseSizeAndFreeOld(winding_accu_t *w) +{ + int i; + winding_accu_t *c; + + if (!w) Error("CopyWindingAccuIncreaseSizeAndFreeOld: winding is NULL"); + + c = AllocWindingAccu(w->numpoints + 1); + c->numpoints = w->numpoints; + for (i = 0; i < c->numpoints; i++) + { + VectorCopyAccu(w->p[i], c->p[i]); + } + FreeWindingAccu(w); + return c; +} + /* ================== CopyWindingAccuToRegular @@ -690,19 +712,17 @@ void ChopWindingInPlaceAccu(winding_accu_t **inout, vec3_t normal, vec_t dist, v *inout = NULL; return; } + // TODO: I'm wondering what happens if a brush has 2 planes that are the same. In + // other words, nothing is on SIDE_FRONT but everything is on SIDE_ON. This will cause + // both of the duplicate planes to have a NULL winding, as far as I can tell, because if + // all points are on SIDE_ON, the above "if" statement will be true and the winding will + // be NULL'ed out. This may actually be the legacy and desired behavior. Write a + // regression test for this. if (!counts[SIDE_BACK]) { return; // Winding is unmodified. } - // TODO: Why the hell are we using this number? The original comment says something along - // the lines of "Can't use counts[0] + 2 because of fp grouping errors", and I'm not - // entirely sure what that means. Is it a problem later on in how the caller of this function - // uses the returned winding afterwards? Is it a problem in that the algorithm below will - // bump into the max in some cases? Anyhow, investigate and fix this number to be a correct - // and predictable number. We can also dynamically grow inside of this function in case we - // do hit the max for some strange reason. - maxpts = in->numpoints + 4; - + maxpts = counts[SIDE_FRONT] + 2; // We dynamically expand if this is too small. f = AllocWindingAccu(maxpts); for (i = 0; i < in->numpoints; i++) @@ -711,20 +731,28 @@ void ChopWindingInPlaceAccu(winding_accu_t **inout, vec3_t normal, vec_t dist, v if (sides[i] == SIDE_ON) { - if (f->numpoints >= maxpts) - Error("ChopWindingInPlaceAccu: points exceeded estimate"); if (f->numpoints >= MAX_POINTS_ON_WINDING) Error("ChopWindingInPlaceAccu: MAX_POINTS_ON_WINDING"); + if (f->numpoints >= maxpts) // This will probably never happen. + { + Sys_FPrintf(SYS_VRB, "WARNING: estimate on chopped winding size incorrect (no problem)\n"); + f = CopyWindingAccuIncreaseSizeAndFreeOld(f); + maxpts++; + } VectorCopyAccu(p1, f->p[f->numpoints]); f->numpoints++; continue; } if (sides[i] == SIDE_FRONT) { - if (f->numpoints >= maxpts) - Error("ChopWindingInPlaceAccu: points exceeded estimate"); if (f->numpoints >= MAX_POINTS_ON_WINDING) Error("ChopWindingInPlaceAccu: MAX_POINTS_ON_WINDING"); + if (f->numpoints >= maxpts) // This will probably never happen. + { + Sys_FPrintf(SYS_VRB, "WARNING: estimate on chopped winding size incorrect (no problem)\n"); + f = CopyWindingAccuIncreaseSizeAndFreeOld(f); + maxpts++; + } VectorCopyAccu(p1, f->p[f->numpoints]); f->numpoints++; } @@ -747,10 +775,14 @@ void ChopWindingInPlaceAccu(winding_accu_t **inout, vec3_t normal, vec_t dist, v else if (normal[j] == -1) mid[j] = -dist; else mid[j] = p1[j] + (w * (p2[j] - p1[j])); } - if (f->numpoints >= maxpts) - Error("ChopWindingInPlaceAccu: points exceeded estimate"); if (f->numpoints >= MAX_POINTS_ON_WINDING) Error("ChopWindingInPlaceAccu: MAX_POINTS_ON_WINDING"); + if (f->numpoints >= maxpts) // This will probably never happen. + { + Sys_FPrintf(SYS_VRB, "WARNING: estimate on chopped winding size incorrect (no problem)\n"); + f = CopyWindingAccuIncreaseSizeAndFreeOld(f); + maxpts++; + } VectorCopyAccu(mid, f->p[f->numpoints]); f->numpoints++; } diff --git a/tools/quake3/q3map2/map.c b/tools/quake3/q3map2/map.c index 162df7f8..4011de37 100644 --- a/tools/quake3/q3map2/map.c +++ b/tools/quake3/q3map2/map.c @@ -164,13 +164,7 @@ qboolean SnapNormal( vec3_t normal ) vec_t ourNormalEpsilon; qboolean adjusted = qfalse; - // We should maybe think about scaling normalEpsilon by a factor of 10 - // or so to compensate for the fact that we're comparing the near-zero - // magnitude components, not the near-one component. For now, don't scale. - - ourNormalEpsilon = (vec_t) (normalEpsilon * 1.0); - - // Another change from the original SnapNormal() is that we snap each + // A change from the original SnapNormal() is that we snap each // component that's close to 0. So for example if a normal is // (0.707, 0.707, 0.0000001), it will get snapped to lie perfectly in the // XY plane (its Z component will be set to 0 and its length will be @@ -179,7 +173,7 @@ qboolean SnapNormal( vec3_t normal ) for (i = 0; i < 3; i++) { - if (normal[i] != 0.0 && -ourNormalEpsilon < normal[i] && normal[i] < ourNormalEpsilon) + if (normal[i] != 0.0 && -normalEpsilon < normal[i] && normal[i] < normalEpsilon) { normal[i] = 0.0; adjusted = qtrue; @@ -295,9 +289,9 @@ void SnapPlane( vec3_t normal, vec_t *dist ) /* SnapPlaneImproved() -snaps a plane to normal/distance epsilons, improved code uses center +snaps a plane to normal/distance epsilons, improved code */ -void SnapPlaneImproved(vec3_t normal, vec_t *dist, int numPoints, vec3_t *points) // TODO: const const on points +void SnapPlaneImproved(vec3_t normal, vec_t *dist, int numPoints, const vec3_t *points) { int i; vec3_t center; @@ -307,6 +301,7 @@ void SnapPlaneImproved(vec3_t normal, vec_t *dist, int numPoints, vec3_t *points { if (numPoints > 0) { + // Adjust the dist so that the provided points don't drift away. VectorClear(center); for (i = 0; i < numPoints; i++) { @@ -347,7 +342,7 @@ int FindFloatPlane( vec3_t normal, vec_t dist, int numPoints, vec3_t *points ) #if EXPERIMENTAL_SNAP_PLANE_FIX - SnapPlaneImproved(normal, &dist, numPoints, points); + SnapPlaneImproved(normal, &dist, numPoints, (const vec3_t *) points); #else SnapPlane( normal, &dist ); #endif @@ -392,7 +387,7 @@ int FindFloatPlane( vec3_t normal, vec_t dist, int numPoints, vec3_t *points ) plane_t *p; #if EXPERIMENTAL_SNAP_PLANE_FIX - SnapPlaneImproved(normal, &dist, numPoints, points); + SnapPlaneImproved(normal, &dist, numPoints, (const vec3_t *) points); #else SnapPlane( normal, &dist ); #endif