Methods Transform.PointToWorld and Transform.PointToLocal (probably others too) are bugged

[GavrilovNI]

Describe the bug

Transform.PointToWorld is not working properly due to mixed up order of multiplications:

public readonly Vector3 PointToWorld(Vector3 localPoint) { return Position + localPoint * Rotation * Scale; }

should be localPoint * Scale * Rotation not localPoint * Rotation * Scale like there:

public readonly Transform ToWorld(Transform child) { Transform result = default(Transform); result.Position = child.Position * Scale * Rotation + Position; result.Rotation = Rotation * child.Rotation; result.Scale = Scale * child.Scale; return result; }

To Reproduce

get Test component from https://pastebin.com/ascnrkWZ get scene from https://pastebin.com/LrJsJGTT see how its not working properly

Expected behavior

Transform.World.PointToWorld(Child.Transform.LocalPosition) should return the same value as Transform.World.ToWorld(Child.Transform.Local).Position

Media/Files

No response

Additional context

No response

[Gmod4phun]

My brain might be off here, but shouldn't you be using ToWorld methods only on Local Transforms, and ToLocal methods only on World Transforms?

[GavrilovNI]

My brain might be off here, but shouldn't you be using ToWorld methods only on Local Transforms, and ToLocal methods only on World Transforms?

they do the same thing in that case, anyway its bugged so Transform.Local.PointToWorld(Child.Transform.LocalPosition) will do the same thing as Transform.World.PointToWorld(Child.Transform.LocalPosition)

[garrynewman]

We have a couple of unit tests that prove this works, but maybe PointToLocal is backwards too?

	[TestMethod]
	public void PointToLocalWorld()
	{
		var point = SandboxSystem.Random.VectorInCube() * 100;
		var parent = new Transform( SandboxSystem.Random.VectorInCube() * 100, Rotation.LookAt( SandboxSystem.Random.VectorInCube() ) );

		var lp = parent.PointToLocal( point );
		var wp = parent.PointToWorld( lp );

		Assert.IsFalse( point == lp );
		Assert.IsTrue( wp == point );
	}

	[TestMethod]
	public void PointToLocalWorld_WithScale()
	{
		var point = new Vector3( 100, 100, 100 );
		var parent = new Transform( new Vector3( 1000, 1000, 1000 ), new Angles( 45, 0, 0 ), new Vector3( 1, 0.5f, 0.25f ) );

		var lp = parent.PointToLocal( point );

		System.Console.WriteLine( $"To Local: {point} => {lp}" );

		var wp = parent.PointToWorld( lp );

		System.Console.WriteLine( $"To World: {lp} => {wp}" );

		Assert.IsFalse( point == lp );
		Assert.IsTrue( point.AlmostEqual( wp, 0.001f ), $"{point} doesn't equal {wp}" );
	}

[GavrilovNI]

PointToLocal is backwards too, yeah You can see that multiplication order in Transform.ToWorld and its different to Transform.Local.PointToWorld (this one is wrong) Didnt look into other methods like NormalToWorld, probably they are also wrong

[Gmod4phun]

For what it's worth, I'm using the ToWorld/ToLocal methods on rotations, positions and normals in my teleportation code, and they seem to work just fine.

Code I'm talking about, for reference: https://github.com/Gmod4phun/sbox-stargate-code/blob/master/code/Components/Stargate/Base/EventHorizon.cs#L447

[GavrilovNI]

For what it's worth, I'm using the ToWorld/ToLocal methods on rotations, positions and normals in my teleportation code, and they seem to work just fine.

Code I'm talking about, for reference: https://github.com/Gmod4phun/sbox-stargate-code/blob/master/code/Components/Stargate/Base/EventHorizon.cs#L447

Try change scale AND rotation of parent object and use this methods for child. Changing rotation OR scale will not affect it, because its the same as multiplying by 1

[GavrilovNI]

Accidentaly closed

[garrynewman]

Yeah I suspect that this is only a problem when the scale is not uniform

[GavrilovNI]

here is working code

//
// Summary:
//     Convert a point in world space to a point in this transform's local space
public readonly Vector3 PointToLocal(Vector3 worldPoint)
{
    return (worldPoint - Position) * Rotation.Inverse / Scale;
}

//
// Summary:
//     Convert a world normal to a local normal
public readonly Vector3 NormalToLocal(Vector3 worldNormal)
{
    return (worldNormal * Rotation.Inverse * Scale.Inverse).Normal;
}

//
// Summary:
//     Convert a point in this transform's local space to a point in world space
public readonly Vector3 PointToWorld(Vector3 localPoint)
{
    return Position + localPoint * Scale * Rotation;
}

//
// Summary:
//     Convert a local normal to a world normal
public readonly Vector3 NormalToWorld(Vector3 localNormal)
{
    return (localNormal * Scale * Rotation).Normal;
}

RotationToWorld and RotationToLocal have no errors

[GavrilovNI]

also you probably want to write tests, where it compares Transform.Local.PointToWorld(X) == Transform.Local.ToWorld(new Transform(X)).Position Transform.World.PointToLocal(X) == Transform.World.ToLocal(new Transform(X)).Position and something for normals (don't now to make a good one)

[smt923]

oh man, this was really tripping me up for a couple days and I didn't realize it was cause of this, switching my code to the one from the post above completely fixed my issue, it might only come up in specific edge cases but this is definitely still broken I think

using PointToLocal:

hitObj.Transform.LocalPosition = obj.Transform.World.PointToLocal(traceLoc);

1.webm

vs the code above:

hitObj.Transform.LocalPosition = (traceLoc - obj.Transform.Position) * obj.Transform.Rotation.Inverse / obj.Transform.Scale;

2.webm

this tripped me up for a while because when the rigidbodies weren't moving it was totally fine (like, the current API doesn't produce this error if my objects aren't moving) but obviously once there was rotation this issue kicked in due to the shapes being scaled into rectangles

I read a bunch more and played with it and yeah, this just doesn't come up if there isn't both scale and rotation happening at once, at 1,1,1 scale PointToLocal looks like it works fine even on this rotating cube:

but obviously as soon as I apply scale to the rotating shapes the issue appears again like above, changing the code to the switched order above definitely does fix this for me

[aylaylay]

Sorry for the delay on this, I think everyone was scared to touch these as we use them a lot. But as figured out already I think this problem only pops up when using scale.

I have made the changes and also removed scale from NormalToLocal and NormalToWorld as scaling normals here doesn't make much sense to me but happy to be proven wrong.

[GavrilovNI]

I have made the changes and also removed scale from NormalToLocal and NormalToWorld as scaling normals here doesn't make much sense to me but happy to be proven wrong.

Thank you! Yeah, multiplying by scale doesn't make sense unless scale is 0, not sure if it's usefull to have scale equal to 0, but it can make problems to someone

[GavrilovNI]

unless scale is 0, not sure if it's usefull to have scale equal to 0, but it can make problems to someone

Feel free to close if said above is ok. Everything else seems fixed