Trapezoidal surfaces with Shape Rectangle give buggy PierceSurface results

[DeadParrot]

Four-sided surfaces that are isosceles trapezoids are currently given a Shape of Rectangle. In PierceSurface Rectangle surfaces are handled with a special case (for speed) that is not valid for trapezoids so it is returning incorrect results that can say a surface is hit by the ray that isn't or vice versa. It is not known if a similar problem with such surfaces exists elsewhere in the code.

The PierceSurface errors may not be considered critical but the incorrect behavior is causing solution differences for the octree-based surface algorithms I am working on, which correctly avoid sending some non-hit surfaces to PierceSurface to be checked. Given the planned use of octree lookups in FY16 production releases, resolving this issue first will help smooth that migration.

@mjwitte suggests that a new shape value of Trapezoid could be added. This will require changes to any code that really wants to treat trapezoids and rectangles in the same way. It would also need to be determined whether all trapezoids or only isosceles trapezoids will be put in the new shape category.

Switching the set of Shapes from a group of integers to an enum would be a nice thing to do along with this change. Finding and fixing any code that depends on the actual shape flag integer would go along with this.

[mjwitte]

The shape test is here in SurfaceGeometry::ProcessSurfaceVertices.

            if ( Surface( ThisSurf ).Sides == 4 ) {
                Diagonal1 = VecLength( Surface( ThisSurf ).Vertex( 1 ) - Surface( ThisSurf ).Vertex( 3 ) );
                Diagonal2 = VecLength( Surface( ThisSurf ).Vertex( 2 ) - Surface( ThisSurf ).Vertex( 4 ) );
                // Test for rectangularity
                if ( std::abs( Diagonal1 - Diagonal2 ) < 0.020 ) {
                    Surface( ThisSurf ).Shape = Rectangle;
                } else {
                    Surface( ThisSurf ).Shape = Quadrilateral;
                }
            }

The current test will tag rectangles and isosceles trapezoids, and parallelograms all as Shape=Rectangle, because they all have 4 sides and equal "diagonals".

The simplest fix is to add a more robust test here so that the trapezoids and parallelograms are treated as general Quadrilaterals (no new shape types).

If this imposes a performance hit in PierceSurface, then additional special shape types could be added if there is a more efficient way to handle those particular shapes. Suggested fix to make the rectangle test more robust is to keep the diagonals test and add another test to check if any corner is a right angle. @DeadParrot suggests checking for dot product of adjacent side vectors being sufficiently close to zero should be faster than a trig call.

[mjwitte]

@Myoldmopar Tagging this a high priority for v8.4 to assess the diffs.

[DeadParrot]

I don't think there are enough of these surfaces for there to be a significant performance hit in PierceSurface: let's get the correctness first. I plan to write a fast PierceSurface in FY16 as part of the octree integration work so at that time I'll exploit all these special cases if it is beneficial for performance.

[mjwitte]

Closed via #5175