Plane constructors inconsistent

[Basewq]

Release Type: Official Release

Version: All versions up to the latest (4.2.0.2381 as of this issue)

Platform(s): All (tested on Windows)

Describe the bug From #2670, I believe according to the documentation: The distance of the plane along its normal from the origin. means positive 'D' moves in the positive direction of its normal vector.`

This would imply Plane.D represents the plane equation in the form P.N = d instead of P.N + d = 0

This line of thinking is true for the following constructor:

var p = new Vector3(0, 1, 0);
var normal = new Vector3(0, 1, 0);
var plane = new Plane(p, normal);
System.Diagnostics.Debug.WriteLine($"Normal: {plane.Normal}, D: {plane.D}");

This returns

Normal: X:0 Y:1 Z:0, D: 1

However, the following constructor returns the wrong 'D' value (three points sitting on Y = 1, made in anticlockwise direction)

var p1 = new Vector3(-1, 1, -1);
var p2 = new Vector3(-1, 1, 1);
var p3 = new Vector3(1, 1, 1);
var plane = new Plane(p1, p2, p3);
System.Diagnostics.Debug.WriteLine($"Normal: {plane.Normal}, D: {plane.D}");

This returns

Normal: X:0 Y:1 Z:0, D: -1

I have changed the three points to clockwise (to double check anticlockwise is the correct way to get the plane pointing up):

var p1 = new Vector3(-1, 1, -1);
var p2 = new Vector3(-1, 1, 1);
var p3 = new Vector3(1, 1, 1);
var plane = new Plane(p1, p3, p2);
System.Diagnostics.Debug.WriteLine($"Normal: {plane.Normal}, D: {plane.D}");

This returns

Normal: X:0 Y:-1 Z:0, D: 1

This confirms the three points should be anticlockwise to make the normal point "outwards", but D is now flipped.

Expected behavior In my opinion, my thinking is D: 1 is the correct value, but I am doubting myself now, so now need confirmation from others.

Additional context Related to #2670

Unfortunately, System.Numerics also follows the same issue with the three points (assuming this is an issue) https://github.com/dotnet/runtime/blob/1d1bf92fcf43aa6981804dc53c5174445069c9e4/src/libraries/System.Private.CoreLib/src/System/Numerics/Plane.cs#L62 Their documentation also states D represents The distance of the plane along its normal from the origin. so now I'm doubting which way the plane equation is supposed to be.

EDIT: Some poor dev in the past also casting doubt: https://github.com/stride3d/stride/blob/975c8a8ea55cf2ab20a64a32a261eb62d6922bce/sources/engine/Stride.Rendering/Rendering/SortModeDistance.cs#L42 https://github.com/stride3d/stride/blob/975c8a8ea55cf2ab20a64a32a261eb62d6922bce/sources/engine/Stride.Rendering/Rendering/VisibilityGroup.cs#L117

[Kryptos-FR]

Is it a case of left-handed versus right-handed coordinates? In any case, the expected behavior should be documented.

[Basewq]

Is it a case of left-handed versus right-handed coordinates? In any case, the expected behavior should be documented.

My understanding is both Stride's three points arguments constructor and System.Numerics.Plane.CreateFromVertices follow right-handed coordinates, because both produce "up" direction from following the points in an anticlockwise direction with the right hand.

The problem now is understanding what D is supposed to represent, because it could be either Ax+By+Cz+D=0 or Ax+By+Cz=D I don't think either of these equations have any significance whether it should be left or right handed, only whether the sign should be positive or negative (not an expert so I could be mistaken!).

I will need to look through other various engines, but I'm now suspecting Stride's Plane should also be Ax+By+Cz+D=0, but this means the constructors Plane(point, normal) and Plane(normal, d) are the ones that are wrong. (As a note to myself, this also means checking Bullet physics, because it might be using the Plane equation differently...)

EDIT: just to point out:

1. Plane(point, normal) and Plane(normal, d) seem to make a plane in the form Ax+By+Cz=D
2. Plane(p1, p2, p3) and System.Numerics makes a plane in the form Ax+By+Cz+D=0 To make matters worse, Stride does internally use (1) so this would need to be examined carefully on which way it should be fixed.

[Basewq]

Recap of this issue

There are two equations used to represent a plane, Ax+By+Cz+D=0 or Ax+By+Cz=D, where (A, B, C) is the Normal vector, and D is the constant (sometimes called 'distance').

This D value differs in the sign depending on whether the plane is using the first equation or the second.

Note that Ax+By+Cz=D is intuitive one: If you have a flat XZ plane, ie.Normal pointing straight up (0, 1, 0), increasing D value pushes the plane upwards (ie. "along the normal").

Ax+By+Cz+D=0 is the more "formal" one: If you have a flat XZ plane, ie.Normal pointing straight up (0, 1, 0), increasing D value pushes the plane downwards (ie. backward direction from the normal).

Other implementations

Here's my rushed research into what others are using:

Engine/Library	Equation	Source
Bullet	Ax+By+Cz=D	StaticPlaneColliderShape
System.Numerics	Ax+By+Cz+D=0	Plane.CreateFromVertices
DirectX	Ax+By+Cz+D=0	XMPlaneFromPoints
MonoGame	Ax+By+Cz+D=0	Plane
Unity	Ax+By+Cz+D=0	Plane
Godot	Ax+By+Cz=D	Plane
Unreal	?	Can't confirm

Current Stride implementation

This isn't a comprehensive list of inconsistent Plane forms, but the obvious ones are:

This constructor makes a Plane in the form Ax+By+Cz=D: https://github.com/stride3d/stride/blob/2421472953d38cfbbd99088ea8c75f3f8cc10289/sources/core/Stride.Core.Mathematics/Plane.cs#L81-L85

This constructor makes a Plane in the form Ax+By+Cz+D=0: https://github.com/stride3d/stride/blob/2421472953d38cfbbd99088ea8c75f3f8cc10289/sources/core/Stride.Core.Mathematics/Plane.cs#L104-L121

This method is only correct for a Plane in the form Ax+By+Cz+D=0: https://github.com/stride3d/stride/blob/975c8a8ea55cf2ab20a64a32a261eb62d6922bce/sources/core/Stride.Core.Mathematics/CollisionHelper.cs#L488-L516 I suspect all the Plane related methods in CollisionHelper are for Ax+By+Cz+D=0 form, but can't 100% confirm since I haven''t read each method line by line...

Solution?

On one side, making Ax+By+Cz=D makes it consist with Bullet, though Stride only refers to Bullet's "D" as Offset (and Bullet calls it planeConstant). However Bullet is being pushed out for Bepu, though as far as I'm aware, Bepu does not have an infinite plane. This is also intuitive to the documentation on D: The distance of the plane along its normal from the origin.

On the other side, there have been numerous attempts to push towards using System.Numerics, which "probably" uses Ax+By+Cz+D=0 One thing to caution about, is that having read through this issue https://github.com/dotnet/runtime/issues/24758 other than a single implementation Plane.CreateFromVertices it seems there's only an implicit acknowledgement that the Plane should be Ax+By+Cz+D=0. This lack of explicit acknowledgement was the cause of this issue within Stride, and may sneak into System.Numerics as they have yet to fully commit to it in their api documentation.

The last "solution" is fix nothing, except add documentation remarks on each Plane constructor and plane related methods which form of the plane equation is expected to be used. This means programmers must be extremely vigilant on which Plane constructor they must use.

[Kryptos-FR]

Thanks a lot for your analysis. I would suggest to follow Numerics as we might use it at some point, and fix the constructors that don't follow that logic. It's a decision that needs to be discussed anyway, as that would be a breaking change.