mirror of
https://github.com/UberGames/GtkRadiant.git
synced 2024-11-10 06:31:41 +00:00
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
This commit is contained in:
parent
f36b40ae62
commit
f487ea7c54
4 changed files with 71 additions and 7 deletions
|
@ -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
|
slightest math errors are magnified in successive calls to linear algebra
|
||||||
functions.
|
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:
|
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.
|
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
|
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.
|
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.
|
||||||
|
|
|
@ -16,7 +16,7 @@ SOLUTION TO PROBLEM:
|
||||||
|
|
||||||
It was discovered that BaseWindingForPlane() in polylib.c did some sloppy
|
It was discovered that BaseWindingForPlane() in polylib.c did some sloppy
|
||||||
mathematics with significant loss of precision. Those problems have been
|
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:
|
POSSIBLE SIDE EFFECTS:
|
||||||
|
@ -67,17 +67,27 @@ is this:
|
||||||
As you can see, all points but one have an increase in accuracy. This is
|
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.
|
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:
|
MORE NOTES:
|
||||||
===========
|
===========
|
||||||
|
|
||||||
I attempted to improve upon revision 371 by streamlining the code in
|
I attempted to improve upon revision 371 by streamlining the code in
|
||||||
BaseWindingForPlane() some more. Those attempts were committed as r375.
|
BaseWindingForPlane() some more. Those attempts were committed as revision
|
||||||
After revision 375:
|
375. After revision 375:
|
||||||
|
|
||||||
(6784.09375000 16241.01757812 -1722.04687500)
|
(6784.09375000 16241.01757812 -1722.04687500)
|
||||||
(6144.00000000 16082.99414062 -1443.00390625)
|
(6144.00000000 16082.99414062 -1443.00390625)
|
||||||
(6144.00000000 16122.00000000 -1424.00097656)
|
(6144.00000000 16122.00000000 -1424.00097656)
|
||||||
|
|
||||||
Revision 375 has since been reverted (undone) because of the loss in
|
Revision 375 has since been reverted (undone) because of the loss in
|
||||||
accuracy.
|
accuracy. Revision 377 is a fix for those failed attempts.
|
||||||
|
|
|
@ -14,4 +14,36 @@ compile for any Q3 mod.
|
||||||
SOLUTION TO PROBLEM:
|
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.
|
||||||
|
|
|
@ -15,4 +15,7 @@ SOLUTION TO PROBLEM:
|
||||||
====================
|
====================
|
||||||
|
|
||||||
None yet. The problem is likely caused by sloppy math operations (significant
|
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.
|
||||||
|
|
Loading…
Reference in a new issue