Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 20 additions & 14 deletions crates/bevy_gltf/src/convert_coordinates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ impl ConvertCoordinates for [f32; 4] {
/// Options for converting scenes and assets from glTF's [standard coordinate system](https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#coordinate-system-and-units)
/// (+Z forward) to Bevy's coordinate system (-Z forward).
///
/// _CAUTION: This is an experimental feature. Behavior may change in future versions._
///
/// The exact coordinate system conversion is as follows:
///
/// - glTF:
/// - forward: +Z
/// - up: +Y
Expand All @@ -43,36 +46,39 @@ impl ConvertCoordinates for [f32; 4] {
/// - up: +Y
/// - right: +X
///
/// Note that some glTF files may not follow the glTF standard.
/// Cameras and lights are an exception - they already use Bevy's coordinate
/// system. This means cameras and lights will match Bevy's forward even if
/// conversion is disabled.
///
/// If your glTF scene is +Z forward and you want it converted to match Bevy's
/// `Transform::forward`, enable the `rotate_scene_entity` option. If you also want `Mesh`
/// assets to be converted, enable the `rotate_meshes` option.
/// If a glTF file uses the standard coordinate system, then the conversion
/// options will behave like so:
///
/// Cameras and lights in glTF files are an exception - they already use Bevy's
/// coordinate system. This means cameras and lights will match
/// `Transform::forward` even if conversion is disabled.
/// - `rotate_scene_entity` will make the glTF's scene forward align with the [`Transform::forward`]
/// of the entity with the [`SceneInstance`](bevy_scene::SceneInstance) component.
/// - `rotate_meshes` will do the same for entities with a `Mesh3d` component.
///
/// Other entities in the scene are not converted, so their forward may not
/// match `Transform::forward`. In particular, the entities that correspond to
/// glTF nodes are not converted.
#[derive(Copy, Clone, Default, Debug, Serialize, Deserialize)]
pub struct GltfConvertCoordinates {
/// If true, convert scenes by rotating the top-level transform of the scene entity.
///
/// This will ensure that [`Transform::forward`] of the "root" entity (the one with [`SceneInstance`](bevy_scene::SceneInstance))
/// aligns with the "forward" of the glTF scene.
///
/// The glTF loader creates an entity for each glTF scene. Entities are
/// then created for each node within the glTF scene. Nodes without a
/// parent in the glTF scene become children of the scene entity.
/// The scene entity is created by the glTF loader. Its parent is the entity
/// with the `SceneInstance` component, and its children are the root nodes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Every entity is created by the glTF loader, not just the "scene entity"
  • SceneInstance and SceneRoot are not part of a scene. If you spawn a scene using the SceneSpawner you will not have a SceneInstance or a SceneRoot.

Here's an alternative:

Bevy's glTF loader creates an Entity for each glTF Scene to contain the glTF Scene's nodes. This Scene Entity's children are the entities created from the top-level nodes in the glTF Scene definition. Each node can also continue to have its own child nodes. The Scene Entity that was created to contain the glTF Scene's nodes is the one that becomes rotated.

If you use SceneRoot to spawn the entire scene as a child, that means the Entity with the SceneRoot is parent of the full set of Scene nodes described in the previous paragraph, including the Scene Entity. The child Entity of the SceneRoot is the one that is rotated according to the previous paragraph.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every entity is created by the glTF loader, not just the "scene entity"

The parent of the scene entity is not created by the loader. I was worried that the user might interpret "scene entity" as the parent entity that they created, so it's worth emphasising that the loader creates the scene entity even if it's arguably redundant.

SceneInstance and SceneRoot are not part of a scene. If you spawn a scene using the SceneSpawner you will not have a SceneInstance or a SceneRoot.

The reference to SceneInstance was originally added by Cart. So you're correct and I'm open to changing it, but I'm reluctant to do so without a second opinion. Maybe it was an oversight and should be changed, or maybe there's an assumption that most people use SceneRoot/SceneInstance and that keeps the explanation simpler.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like you agree with the suggested rewrite but want to add SceneInstance into the second paragraph in addition to SceneRoot.

Copy link
Contributor Author

@greeble-dev greeble-dev Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree with the suggested rewrite. I think the current description is good enough, assuming most people are spawning scenes via components.

If I feel there's a consensus that the current description isn't good enough, I'd still need to take a different approach - the suggested rewrite doesn't cover another place that references the scene entity:

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. I think the PR is not sufficient, so I'll leave the change suggestion.

Different documentation sections that are incorrect should also be updated, which is separate from this suggestion and does not affect its relevance.

/// of the glTF scene.
///
/// This option only changes the transform of the scene entity. It does not
/// directly change the transforms of node entities - it only changes them
/// indirectly through transform inheritance.
pub rotate_scene_entity: bool,

/// If true, convert mesh assets. This includes skinned mesh bind poses.
/// If true, convert mesh assets and skinned mesh bind poses.
///
/// This option only changes mesh assets and the transforms of entities that
/// instance meshes. It does not change the transforms of entities that
/// correspond to glTF nodes.
/// instance meshes through [`Mesh3d`](bevy_mesh::Mesh3d).
pub rotate_meshes: bool,
}

Expand Down