From f487ea7c547dbd72e0ac837363c766e6b0f88a53 Mon Sep 17 00:00:00 2001 From: rambetter Date: Wed, 29 Dec 2010 05:20:32 +0000 Subject: [PATCH] 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. git-svn-id: svn://svn.icculus.org/gtkradiant/GtkRadiant/trunk@378 8a3a26a2-13c4-0310-b231-cf6edde360e5 --- .../q3map2/disappearing_sliver/README.txt | 21 +++++++++++- .../q3map2/disappearing_sliver2/README.txt | 18 +++++++--- .../q3map2/disappearing_sliver3/README.txt | 34 ++++++++++++++++++- .../q3map2/sparkly_seam/README.txt | 5 ++- 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/regression_tests/q3map2/disappearing_sliver/README.txt b/regression_tests/q3map2/disappearing_sliver/README.txt index 49154030..a65d9203 100644 --- a/regression_tests/q3map2/disappearing_sliver/README.txt +++ b/regression_tests/q3map2/disappearing_sliver/README.txt @@ -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. diff --git a/regression_tests/q3map2/disappearing_sliver2/README.txt b/regression_tests/q3map2/disappearing_sliver2/README.txt index 55f9b51d..ffa12be5 100644 --- a/regression_tests/q3map2/disappearing_sliver2/README.txt +++ b/regression_tests/q3map2/disappearing_sliver2/README.txt @@ -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. diff --git a/regression_tests/q3map2/disappearing_sliver3/README.txt b/regression_tests/q3map2/disappearing_sliver3/README.txt index 9fda8b0c..839bd692 100644 --- a/regression_tests/q3map2/disappearing_sliver3/README.txt +++ b/regression_tests/q3map2/disappearing_sliver3/README.txt @@ -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. diff --git a/regression_tests/q3map2/sparkly_seam/README.txt b/regression_tests/q3map2/sparkly_seam/README.txt index 4ffab57c..3c54feb5 100644 --- a/regression_tests/q3map2/sparkly_seam/README.txt +++ b/regression_tests/q3map2/sparkly_seam/README.txt @@ -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.