Clarify `Transform` and `GlobalTransform` documentation #9182

Selene-Amanita · 2023-07-17T01:17:54Z

Objective

The doc about Transform and GlobalTransform mention "the reference frame", which can be confusing for two reasons:

a reference frame is y definition relative, Transform is in the reference frame of the parent,
not everybody is familiar with this term, and some people might see "frame" and think it is talking about viewport coordinates. (I actually helped someone on discord who was confused because of that, hence this PR)

Also:

some links are broken
the documentation doesn't detail the meaning of translation, rotation and scale
the documentation uses the term "position" which is misleading

This PR tries to fix that.

Solution

Specify "local" and "absolute"
Specify "in space" (I could have said "in the world" but that could be confusing because of the ECS world, I would also have said "2D/3D space")
Specify "main reference frame"
fix the links
add a section to detail the meaning of translation, rotation and scale
use the term "transform", after defining it, instead of "position"

james7132 · 2023-07-19T03:37:34Z

crates/bevy_transform/src/components/global_transform.rs

@@ -5,7 +5,8 @@ use bevy_ecs::{component::Component, reflect::ReflectComponent};
 use bevy_math::{Affine3A, Mat4, Quat, Vec3, Vec3A};
 use bevy_reflect::{std_traits::ReflectDefault, Reflect};

-/// Describe the position of an entity relative to the reference frame.
+/// Describe the absolute position of an entity in space,


While we're at it, I don't particularly like the use of "position" here. Unity uses Position, rotation and scale of an object in their docs. I'm pretty partial to https://en.wikipedia.org/wiki/Pose_(computer_vision), but that might not common enough jargon to use here.

I agree, I've thought about it too, and it's used on the first line of the doc too, actually translation/rotation/scale are never really explained.
But I don't have a good enough replacement except enumerating the three (with translation) which is a bit annoying to put every time.
"positioning" might be a bit better as it implies rotation too (in my mind at least)?
I could use "pose" by defining it at the beginning (which I think I should do anyway)? Also "The term “pose” is largely synonymous with the term “transform”, but a transform may often include scale, whereas pose does not.".
Maybe just use the enumeration at first then just "transform" but it might be confusing?

Edit: I used "transform" here : 9c91876 but I detailed what a transform is. I used "position" to talk about the translation

crates/bevy_transform/src/components/global_transform.rs

rparrett

I added some suggestions for the GlobalTransform docs.

Some of those suggestions are applicable to bits of text that are repeated in the Transform docs as well, but I haven't repeated the suggestions there.

I am a bit worried about drift in these docs. I think it's worth discussing whether most of them should be moved to either GlobalTransform or Transform so they don't need to be kept in sync.

Side Note: The links to GlobalTransform in the GlobalTransform section and Transform in the Transform section seem a bit distracting / useless.

I am having a hard time reviewing because the docs are so similar.

rparrett · 2024-01-17T14:32:52Z

crates/bevy_transform/src/components/global_transform.rs

@@ -5,34 +5,62 @@ use bevy_ecs::{component::Component, reflect::ReflectComponent};
 use bevy_math::{Affine3A, Mat4, Quat, Vec3, Vec3A};
 use bevy_reflect::{std_traits::ReflectDefault, Reflect};

-/// Describe the position of an entity relative to the reference frame.
+/// Describe the absolute transform (translation, rotation, scale) of an entity in space,


Suggested change

/// Describe the absolute transform (translation, rotation, scale) of an entity in space,

/// Describes the absolute transform (translation, rotation, scale) of an entity in space,

rparrett · 2024-01-17T14:33:45Z

crates/bevy_transform/src/components/global_transform.rs

+/// [`GlobalTransform`] represents:
+/// - The position of an entity in space, defined as a `translation` from the origin,
+/// [`Vec3::ZERO`]. The unit doesn't have a predefined meaning, but the convention
+/// is usually to use `1.0` is equivalent to 1 meter.


Suggested change

/// is usually to use `1.0` is equivalent to 1 meter.

/// is usually to consider `1.0` equivalent to 1 meter.

This is the language used later in this PR in the Transform docs and sounds better to me.

rparrett · 2024-01-17T14:35:59Z

crates/bevy_transform/src/components/global_transform.rs

+/// matching the main reference frame coordinates.
+/// It can be seen as a ratio of the main reference frame length unit to
+/// the entity's length unit.
+/// For example if the scale is `1.0` for a `Mesh`, its vertices position attribute


Suggested change

/// For example if the scale is `1.0` for a `Mesh`, its vertices position attribute

/// For example if the scale is `1.0` for a `Mesh`, its vertex position attributes

rparrett · 2024-01-17T14:45:40Z

crates/bevy_transform/src/components/global_transform.rs

-/// [`Transform`] is the position of an entity relative to its parent position, or the reference
-/// frame if it doesn't have a [`Parent`](bevy_hierarchy::Parent).
+/// [`Transform`] is the local transform of an entity in space relative to its parent transform,
+/// or the global transform relative to the main reference frame if it doesn't have a [`Parent`].


I think it's a bit confusing to say "transform is ... the global transform."

I'd suggest something like

relative to its parent transform or if it doesn't have a parent, the main reference frame

Or some language like "which is equivalent to the global transform if it doesn't have a parent" if you are trying to push that angle.

There's actually a very similar sentence at the top of the Transform docs that seems a bit better.

rparrett · 2024-01-17T14:49:58Z

crates/bevy_transform/src/components/global_transform.rs

+/// [`TransformPropagate`]: crate::TransformSystem::TransformPropagate
+/// [`PostUpdate`]: bevy_app::PostUpdate
+/// [`Parent`]: bevy_hierarchy::Parent
+/// [`transform example`]: https://github.com/bevyengine/bevy/blob/latest/examples/transforms/transform.rs


I'm not sure which link is preferred, but we should be consistent about linking to either the example showcase or the git repository.

rparrett · 2024-01-17T14:50:54Z

crates/bevy_transform/src/components/global_transform.rs

+/// [`Parent`]: bevy_hierarchy::Parent
+/// [`transform example`]: https://github.com/bevyengine/bevy/blob/latest/examples/transforms/transform.rs
+/// [`transforms folder`]: https://github.com/bevyengine/bevy/tree/latest/examples/transforms
+/// [`parenting example`]: https://bevyengine.org/examples/3d/parenting/


This link is broken

rparrett · 2024-01-17T14:52:17Z

crates/bevy_transform/src/components/transform.rs

@@ -5,36 +5,73 @@ use bevy_reflect::prelude::*;
 use bevy_reflect::Reflect;
 use std::ops::Mul;

-/// Describe the position of an entity. If the entity has a parent, the position is relative
-/// to its parent position.
+/// Describe the local transform (translation, rotation, scale) of an entity in space.


Suggested change

/// Describe the local transform (translation, rotation, scale) of an entity in space.

/// Describes the local transform (translation, rotation, scale) of an entity in space.

NthTensor · 2024-03-12T18:19:39Z

Looks like this got lost in limbo. @Selene-Amanita is this still an active PR for you? I can adopt if not.

Selene-Amanita · 2024-03-12T22:02:33Z

@NthTensor I'm not active on Bevy lately, sorry, you can adopt it if you want, no problem!

Edit: btw thank you @rparrett for your review, sorry to not have reacted sooner

bas-ie · 2024-09-22T06:04:21Z

@alice-i-cecile adopted at #15358.

Selene-Amanita added 2 commits July 17, 2023 02:09

Clarify Transform and GlobalTransform documentation

15b6aab

global -> absolute

7937244

Selene-Amanita added C-Docs An addition or correction to our documentation A-Transform Translations, rotations and scales labels Jul 17, 2023

global -> absolute 2

a3a2c8b

james7132 reviewed Jul 19, 2023

View reviewed changes

Selene-Amanita commented Jul 19, 2023

View reviewed changes

crates/bevy_transform/src/components/global_transform.rs Outdated Show resolved Hide resolved

Improve transform and global transform documentation

9c91876

Selene-Amanita added this to the 0.11.1 milestone Jul 19, 2023

cart removed this from the 0.11.1 milestone Aug 9, 2023

Selene-Amanita added this to the 0.12 milestone Aug 28, 2023

Selene-Amanita requested a review from alice-i-cecile August 28, 2023 19:17

alice-i-cecile approved these changes Aug 28, 2023

View reviewed changes

alice-i-cecile removed this from the 0.12 milestone Sep 29, 2023

Selene-Amanita added this to the 0.13 milestone Sep 29, 2023

rparrett reviewed Jan 17, 2024

View reviewed changes

alice-i-cecile removed this from the 0.13 milestone Jan 24, 2024

alice-i-cecile added the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label Mar 13, 2024

bas-ie mentioned this pull request Sep 22, 2024

Clarify Transform and GlobalTransform documentation (adopted) #15358

Open

alice-i-cecile closed this Sep 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Clarify `Transform` and `GlobalTransform` documentation #9182

Clarify `Transform` and `GlobalTransform` documentation #9182

Uh oh!

Selene-Amanita commented Jul 17, 2023 •

edited

Loading

Uh oh!

james7132 Jul 19, 2023

Uh oh!

Selene-Amanita Jul 19, 2023 •

edited

Loading

Uh oh!

Uh oh!

rparrett left a comment •

edited

Loading

Uh oh!

rparrett Jan 17, 2024

Uh oh!

rparrett Jan 17, 2024

Uh oh!

rparrett Jan 17, 2024

Uh oh!

rparrett Jan 17, 2024

Uh oh!

rparrett Jan 17, 2024

Uh oh!

rparrett Jan 17, 2024

Uh oh!

rparrett Jan 17, 2024

Uh oh!

rparrett Jan 17, 2024

Uh oh!

NthTensor commented Mar 12, 2024

Uh oh!

Selene-Amanita commented Mar 12, 2024 •

edited

Loading

Uh oh!

bas-ie commented Sep 22, 2024

Uh oh!

Uh oh!

	/// Describe the absolute transform (translation, rotation, scale) of an entity in space,
	/// Describes the absolute transform (translation, rotation, scale) of an entity in space,

	/// is usually to use `1.0` is equivalent to 1 meter.
	/// is usually to consider `1.0` equivalent to 1 meter.

	/// For example if the scale is `1.0` for a `Mesh`, its vertices position attribute
	/// For example if the scale is `1.0` for a `Mesh`, its vertex position attributes

	/// Describe the local transform (translation, rotation, scale) of an entity in space.
	/// Describes the local transform (translation, rotation, scale) of an entity in space.

Uh oh!

Clarify Transform and GlobalTransform documentation #9182

Clarify Transform and GlobalTransform documentation #9182

Uh oh!

Conversation

Selene-Amanita commented Jul 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Selene-Amanita Jul 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rparrett left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NthTensor commented Mar 12, 2024

Uh oh!

Selene-Amanita commented Mar 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bas-ie commented Sep 22, 2024

Uh oh!

Uh oh!

Clarify `Transform` and `GlobalTransform` documentation #9182

Clarify `Transform` and `GlobalTransform` documentation #9182

Selene-Amanita commented Jul 17, 2023 •

edited

Loading

Selene-Amanita Jul 19, 2023 •

edited

Loading

rparrett left a comment •

edited

Loading

Selene-Amanita commented Mar 12, 2024 •

edited

Loading