Author: rambetter
authorRudolf Polzer <divverent@alientrap.org>
Wed, 29 Dec 2010 16:38:51 +0000 (17:38 +0100)
committerRudolf Polzer <divverent@alientrap.org>
Wed, 29 Dec 2010 16:38:51 +0000 (17:38 +0100)
Date: Tue Dec 28 23:20:32 2010
New Revision: 378
Updating notes on regression tests.  They are "mostly sort of fixed".  Won't
be fixed for good until I look at the last bit of code that has not been
examined yet, which is the plane intersection code.  I want the errors to be
much less than they are now, even though the disappearing_sliver* tests are
now working.

regression_tests/q3map2/disappearing_sliver/README.txt
regression_tests/q3map2/disappearing_sliver2/README.txt
regression_tests/q3map2/disappearing_sliver3/README.txt
regression_tests/q3map2/sparkly_seam/README.txt

index 4915403..a65d920 100644 (file)
@@ -46,7 +46,7 @@ And we get rid of the recpirocal length ilength altogether.  Even the
 slightest math errors are magnified in successive calls to linear algebra
 functions.
 
-The change described above was commmitted to GtkRadiant trunk as revision r363.
+The change described above was commmitted to GtkRadiant trunk as revision 363.
 
 
 POSSIBLE SIDE EFFECTS:
@@ -162,3 +162,22 @@ It seems that FixBrokenSurface() should be fixed to completely fix the case
 where there are two close points, and should report the surface as fixed.
 This might be a destabilizing change however, so if this is indeed fixed, it
 may make sense to activate the fix only if a certain flag is set.
+
+
+MORE NOTES:
+===========
+
+As stated above, the accuracy after revision 363 is:
+
+  (67.000229 -1021.998657 0.000000)
+  (88.000175 -891.999146 -767.997437)
+  (133.999146 -1014.998779 0.000000)
+
+A further change was committed for a related problem in revision 377.  After
+this change:
+
+  (66.99955750 -1022.00262451 0.00000000)
+  (87.99969482 -892.00170898 -768.00524902)
+  (133.99958801 -1015.00195312 0.00000000)
+
+The results look similar with respect to the amount of error present.
index 55f9b51..ffa12be 100644 (file)
@@ -16,7 +16,7 @@ SOLUTION TO PROBLEM:
 
 It was discovered that BaseWindingForPlane() in polylib.c did some sloppy
 mathematics with significant loss of precision.  Those problems have been
-addressed in commit revision 371.
+addressed in commits to revisions 371 and 377.
 
 
 POSSIBLE SIDE EFFECTS:
@@ -67,17 +67,27 @@ is this:
 As you can see, all points but one have an increase in accuracy.  This is
 still not accurate enough in my opinion, but is a step in the right direction.
 
+After the fix committed in revision 377, which is a further attempt to address
+BaseWindingForPlane(), we get the following accuracy:
+
+  (6784.00000000 16241.00000000 -1722.00000000)
+  (6144.00000000 16083.00000000 -1443.00000000)
+  (6144.00000000 16122.00000000 -1424.00000000)
+
+It's just a fluke for this particular case, but obviouly revision 377 looks
+favorably upon this regression test, because there is zero percent error.
+
 
 MORE NOTES:
 ===========
 
 I attempted to improve upon revision 371 by streamlining the code in
-BaseWindingForPlane() some more.  Those attempts were committed as r375.
-After revision 375:
+BaseWindingForPlane() some more.  Those attempts were committed as revision
+375.  After revision 375:
 
   (6784.09375000 16241.01757812 -1722.04687500)
   (6144.00000000 16082.99414062 -1443.00390625)
   (6144.00000000 16122.00000000 -1424.00097656)
 
 Revision 375 has since been reverted (undone) because of the loss in
-accuracy.
+accuracy.  Revision 377 is a fix for those failed attempts.
index 9fda8b0..839bd69 100644 (file)
@@ -14,4 +14,36 @@ compile for any Q3 mod.
 SOLUTION TO PROBLEM:
 ====================
 
-None yet.  Probably due to sloppy math code.
+More work has been done to BaseWindingForPlane() to make it more accurate.
+This function is in polylib.c.  The changes to fix this regression test were
+committed in revision 377; however, those changes are not "good enough".
+
+
+IN-DEPTH DISCUSSION:
+====================
+
+This is the problem triangle:
+
+  In ParseRawBrush() for brush 0
+      Side 0:
+          (6144.000000 16122.000000 -2048.000000)
+          (6144.000000 16083.000000 -2048.000000)
+          (6784.000000 16241.000000 -2048.000000)
+
+Computed winding before fix:
+
+  (6784.16406250 16241.04101562 -2048.00000000)
+  (6144.00000000 16122.00976562 -2048.00000000)
+  (6144.00000000 16083.00000000 -2048.00000000)
+
+Obviously the 6784.16406250 is beyond epsilon error.
+
+After revision 377:
+
+  (6783.85937500 16240.96484375 -2048.00000000)
+  (6144.00000000 16121.99218750 -2048.00000000)
+  (6144.00000000 16083.00000000 -2048.00000000)
+
+Even though this fixes the regression test, the error in 6783.85937500 is
+still greater than epsilon (but fortunately in the opposite direction).  So
+I don't consider this test case to be fixed quite yet.
index 4ffab57..3c54feb 100644 (file)
@@ -15,4 +15,7 @@ SOLUTION TO PROBLEM:
 ====================
 
 None yet.  The problem is likely caused by sloppy math operations (significant
-loss of precision).
+loss of precision).  This bug pops in and out of existence with every other
+commit at the moment.  The problem is likely caused by the operations in the
+brush winding computation (where the planes are intersected with each other).
+I have not gotten around to addressing that code yet.