godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.8k stars 21.13k forks source link

Vulkan: Normals don't update on skinned meshes when transformed by scaling or translating #53441

Open SavageMcCabbage opened 3 years ago

SavageMcCabbage commented 3 years ago

Godot version

4.0.dev.20211004.official.2e8cba0bd obtained from https://downloads.tuxfamily.org/godotengine/testing/4.0/4.0-dev.20211004/

System information

Windows 10, Vulkan Clustered, GTX 1070ti, Nvidia 471.96 driver

Issue description

If a model rigged with a skeleton has any of its bones translate or scale, the parts of the mesh affected by that bone will not update their normals, resulting in incorrect shading. Rotation doesn't seem to have this issue.

I didn't test other file types, but these models were imported as GLTF2 (.glb) files from blender 2.93.4.

Before moving the bone: untranslated

After moving: untranslated

The view here was set to display the normal buffer, so the color on the plane should have been different on either side of the peak.

Scaling also has this issue when the scaling isn't equal on all 3 axes, here the sphere on the left was scaled almost flat using a bone, but the sphere on the right was scaled about the same using the editor scaling control, which works without issue: untranslated

Steps to reproduce

Open the attached project and switch the editor viewport to display the normal buffer. The two tscn files contain models with skeletons that have already been deformed, one for translation and the other for scaling.

Alternatively, import any mesh with multiple bones and move one of them so it introduces shearing - moving a character model's spine off to the side will show this quite well.

For scale - any round surface scaled flat, like a character's head, will end up keeping its original normals rather than updating them.

Minimal reproduction project

Normals Bug.zip

clayjohn commented 1 year ago

I can't reproduce the issue with scaling. When scaling the normal does change appropriately for me. But in the testing scene with the plane I can confirm that the normal does not appear to change.

Upon further investigation it appears that the normals of the plane do change if the bone is rotated, but not if the bone is scaled. Which makes sense in some cases, i.e. if I have a jump animation, it shouldn't change the normals of my character. But it doesn't really make sense here where the is a bone for each vertex.

SavageMcCabbage commented 1 year ago

I just realized I titled this issue incorrectly - it should have been scaling and translation, not scaling and rotation.

Anyway, I tried this again on RC4 with all three renderers. As you said, translation isn't working still, but on my end scaling with bones is still not giving the correct normals:

Flat ballz

Both are scaled to 0.1 height. The one on the left used a bone in Godot, the one on the right was created by scaling a SphereMesh on a MeshInstance3D. Mobile and forward+ both give the result above, but in the compatibility renderer they both look incorrect.

Seeing as you mentioned it, I tried rotation out as well. It works in most circumstances, but I found it won't get the normals right if the effect of the rotation resembles a translation:

twistedMesh

Here, the rotating bone was way offscreen to the bottom left. The mesh on the left was rotated in Godot, and the one on the right had the deformation applied before exporting from Blender. I'm not sure this warrants a new issue, but I would guess it's related?

I agree that it doesn't make sense to fully recalculate normals when animating a jump, although I feel like you'd get the right result at the cost of an unnecessary perf hit, because presumably jumping doesn't deform the character. On the other hand, for something like face posing, being unable to move a bone to control eyebrows, lip movements and so on without weird shading is pretty limiting. Maybe a shader flag or a bool on GeometryInstance3D would do the job, and users could decide which meshes need the extra accuracy.

DDarby-Lewis commented 2 months ago

This is still and issue on 4.3, I have been looking for a solution since moving over from Unity (worth mentioning that this didn't work there either). It's a big difference though that godot already has this working with transform scaling (unity did not). Hopefully that means something like this wouldn't be too difficult to add to the skinning?