bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
34.27k stars 3.35k forks source link

Docs for `transform_point` on `Transform` and `GlobalTransform` contain mistakes #11078

Open Utsira opened 7 months ago

Utsira commented 7 months ago

The docs for transform_point on Transform and GlobalTransform contain mistakes.

The latter has the line:

This moves point into the local space of this [GlobalTransform].

https://github.com/bevyengine/bevy/blob/eca7924bb44fdf28664f5ae82dea7ea4e44f08b6/crates/bevy_transform/src/components/global_transform.rs#L188

similarly Transform states that:

If this [Transform] has a parent, this will transform a point that is relative to the parent's [Transform] into one relative to this [Transform]. If this [Transform] does not have a parent, this will transform a point that is in global space into one relative to this [Transform].

https://github.com/bevyengine/bevy/blob/eca7924bb44fdf28664f5ae82dea7ea4e44f08b6/crates/bevy_transform/src/components/transform.rs#L383

All of these statements get the conversion the wrong way round. transform_point transforms the point FROM the local space of the transform TO the space of the parent/ the reference frame. It is a localToParent/World operation.

If you wanted to go from global to local, you need to call

global_xform.affine().inverse().transform_point(point)

outside of the scope of a docs issue, but I'd argue that the world to local transform is a common enough scenario that it'd be handy to have it as method directly on GlobalTransform and Transform, maybe another method for vectors too, eg:

fn transform_point_to_local() fn transform_vector_to_local()

You could also extend the name of transform_point to further clear up the confusion in the docs (and add a method for vectors too)

fn transform_point_from_local() fn transform_vector_from_local()

irate-devil commented 7 months ago

I think the docs on Transform are correct, but the GlobalTransform is indeed a local-to-world transform so the docs are incorrect there, that's my bad.

Utsira commented 7 months ago

they're both wrong. The one for Transform says If this [Transform] does not have a parent, this will transform a point that is in global space into one relative to this [Transform]., when it actually does the opposite, should say something like If this [Transform] does not have a parent, this will transform a point that is in local space into global space.

irate-devil commented 7 months ago

Yeah, nvm I should just give up on trying to comprehend math :v

I'll work on a PR to fix this while actually adding some tests for this stuff.

masonk commented 1 week ago

I just noticed this open issue. I believe my patch fixes it for GlobalTransform::transform_point.

https://github.com/bevyengine/bevy/pull/14292