Add an import setting to rotate the imported scene by 180 degrees

[aaronfranke]

This PR implements and closes godotengine/godot-proposals#6198

This PR adds an import setting to rotate the imported scene by 180 degrees. This sets a negative X/Z scale that rotates by 180 degrees on the first level child nodes (same math as a Basis with rotate 180 deg Y, don't worry there are no negatively-scaled nodes in the final scene, even with apply root scale unchecked).

Bikeshedding: What should this option be called? I'm not totally sold on rotate_180_degrees since it doesn't describe how we're doing the rotation, but it is easy to guess that it's a rotation around Y (really on the XZ plane).

EDIT: Be aware that I changed the implementation since initially opening this PR. I collapsed several comments below as outdated. The new implementation does the rotation after most of the import logic, so it should be much safer.

[QbieShay]

Hey @aaronfranke , do you think it's possible to add a project settings flag to make this the default behaviour project wide?

[aaronfranke]

This is already possible, but I don't recommend it. Even for projects importing many characters and props facing +Z, it would be undesired to rotate other assets such as maps/terrain. Plus, even after testing and bug fixing, there will likely be edge cases broken with this enabled, so I would recommend users have it disabled by default and selectively enable for each asset they want to be rotated.

[fire]

Please do not merge until it works on all cases like the skeletons. You can see the previous attempts that were not completed. I believe the best way to do the flip is flip two axes of the scale and then apply the scale. Then there is some trickery with skins and its relation to the mesh transform.

[QbieShay]

Does it affect root motion?

[TokageItLab]

Also, since Godot has a glTF exporter, it should be necessary to implement rotation on that side as well. This option may cause that when a glTF exported from Godot is re-imported into Godot, the asset's orientation is flipped. @fire

In other words, if this is implemented for the sake of consistency, then it should be mandatory (at least a fundamental flag like Project/EditorSetting) not option in importer IMO.

[TokageItLab]

This is a Fix Silhouette issue, not Apply Node Transforms. As explained above, SkeletonProfile(reference rest pose) is based on +Z Forward, which means that if you want to apply rotation before PostImportPlugin, you also need to rotate SkeletonProfile.

So, as a simpler solution, I suggest creating a PostImportPlugin that does the rotation later than Retarget.

[aaronfranke]

I updated the PR with help from @TokageItLab. The fix ended up being simple, perform the rotation after all of the skeleton logic is run, so that the rotation does not interfere with the skeleton logic. It should be fully working now.

[TokageItLab]

This should work well, but a consensus is needed to decide if this is merged or not.

For now, it should be possible to create a PostImportPlugin add-on that provides the same functions, so we should provide that in the meantime.

---

I have described retargeting at length above, but I will summarize it again:

The retarget applies its parent's transform to match the direction of the bone and the direction of the movement animation with the visual (World coordinate).

At this point, the Forward axis of the asset must be defined to only one. Currently it is +Z, following the glTF specification.

At first this PR was trying to do a rotation before retargeting, which could cause the forward axis to not be defined as one. That is why I said it should be mandatory and not optional.

Now this PR does the rotation after the retarget, so there is no problem there.

---

However, this PR leaves the concept of +Z Forward on the asset.

I don't know what will be influenced by having a rotation on the parent node. (IK or PhysicsBone or something?)

Also, it makes the necessary hierarchy deeper due to the nodes for rotation.

So I still think we need +Z Forward support as well as #72795.

I think this PR and #72795 can coexist, but personally I want to give priority to #72795 because the rotation of the parent node still looks like a hack.

If we don't want the parent node to have rotation without #72795, we would have to force -Z Forward on all processes as I said above, but it would no longer be possible.

---

Well, this is what we can do now for now.

For now, it should be possible to create a PostImportPlugin add-on that provides the same functions, so we should provide that in the meantime.

[aaronfranke]

Also, it makes the necessary hierarchy deeper due to the nodes for rotation.

It doesn't, at least not in this PR. Since GLTF allows defining multiple root nodes, Godot already imports an extra root node already, so we can just rotate the GLTF scene's root node which doesn't mess with any nodes deeper down.

I think this PR and #72795 can coexist

I agree, they are not in conflict. If we accept both, we are giving users options. If we are picking one, we are enforcing a particular workflow. I would give priority to this PR, because it's the system I would rather use myself and it keeps a consistent forward direction, but I am not at all against your PR.

because the rotation of the parent node still looks like a hack.

To be honest, you're right, it kinda is. My initial thought was to rotate all the meshes/skeletons/etc, but that's much more complicated and I was not able to figure it out. This setting could be improved in the future for Godot 5.0 or 6.0 or whatever so that it doesn't rotate the nodes and instead rotates the meshes/skeletons/etc. But this PR should work for nearly every use case, or maybe every use case, even if it's not theoretically the most optimal solution.

[TokageItLab]

This still appears to be hard coded, so this logic needs to be separated as PostImportPlugin and properly imported.

Also it could be made into an add-on that could be easily distributed in the asset library without having to discussion whether to merge it into the core or not. Will see if I can work on that later.

[aaronfranke]

@TokageItLab I don't think there is a point to abstracting this code out to a plugin with a whole other class. It's a very small amount of code. At most I could turn it into a method. Also, since it's a very small amount of code that many people would find useful, I think this should be in core.

[TokageItLab]

@lyuma and I agreed that having this only there as a checkbox is too little information and rather it makes more confusing.

After the #76060 or #76082 merge, this should no longer be necessary for those who follow the correct export/import process.

If it is a useful case for most people - if you have a model in the asset store that is facing the other way and you are using it to put it back in the right direction, you will need to rotate the Node "before" retargeting¹, not after retargeting.

But this PR stuff do it after retargeting, it means that this PR is focused on a special workflow, so this should be an Add-on.

And you need to note in the Add-on description that this is a hacky workflow in the right-hand coordinate system, and that when anyone export the model as glTF from Godot, the assets need to be reoriented to +Z.

This is the view from the glTF pipeline team.

Footnotes

1. And even then, it should be allowed to specify the rotation of each xyz in 90 degree steps. That would also help with fbx that are oriented up or down ↩

[akien-mga]

Is this still wanted after the other changes merged to handle this issue?

[aaronfranke]

I won't be using this option personally. I still think this is handy to have in core to give users the freedom to alter the rotation of their assets at the end of the import process, but I won't be sad if this PR is closed.

[Zireael07]

The project I would need it for relies on some stuff coming in 4.1, so I can't check before 4.1 :(

[aaronfranke]

While this option may be useful for some users, since I will not be using it personally, and as Tokage explained this is not used in the "files are +Z, game logic is +Z" workflow and it does not help the "files are -Z" case whenever skeletons are involved. Also as Tokage explained, the "files are +Z, game logic is -Z" case is considered an unsupported / special workflow and Tokage says this should be an add-on (and really, it can be, there is no reason this can't be done in user code, it's just a question of if it's convenient to have in core). Therefore, I am closing this PR.

For the record with the above discussion, handedness is a mathematical concept, it is not related to how the default orientations of cameras and assets are laid out relative to each other.