Godot 3.4.3 Projects created with last minor version do not show materials properly

[Snaker1]

Godot version

3.4.3

System information

Linux, GeForce RTX 2060, GLES3

Issue description

When I run projects I had formerly created in 3.4.2 on the new 3.4.3 bugfix release some objects appear to be "greyed out".

Sometimes they look good in viewport but bad when running the game. They sometimes only look bad when looked at a certain angle or flicker.

Speculation: Normal maps might be messed up? When I disable the normal map, the material is visible again.

Note that projects created in 3.4.3 look fine in 3.4.2 but not the other way round. Importing the original gltf again in 3.4.3 looks fine but is not practical for a bigger project.

Steps to reproduce

Run attached project in both in 3.4.2 and then 3.4.3 and note the difference.

Note that I have not found a way to reliably reproduce the bug as some things created in 3.4.2 look fine on 3.4.3

3.4.2

3.4.3

Minimal reproduction project

Material_Bug_Example.zip

[timothyqiu]

Related to #56161. Looks fine after reverting it.

[clayjohn]

Related to #56161. Looks fine after reverting it.

Ooooh, in that case reimporting the mesh may fix the issue

@Snaker1 have you tried reimporting your meshes?

[Snaker1]

Related to #56161. Looks fine after reverting it.

Ooooh, in that case reimporting the mesh may fix the issue

@Snaker1 have you tried reimporting your meshes?

I just tried importing my player character in a completely new project created with 3.4.3 and I am still getting the bug.

Some other meshes are fixed when importing fresh but that is not a good solution for me as my project is a bit more complex and I am not aware of a way to automatically reimport all meshes.

[Calinou]

and I am not aware of a way to automatically reimport all meshes.

Move the (normally hidden) .import folder to another location while the editor is closed, then open the editor. This will force all resources to reimport.

[Snaker1]

and I am not aware of a way to automatically reimport all meshes.

Move the (normally hidden) .import folder to another location while the editor is closed, then open the editor. This will force all resources to reimport.

Thanks, good to know.

Just tried this. It did NOT fix anything for my project.

[akien-mga]

CC @The-O-King

[The-O-King]

Just took a quick look and I wonder if in the blend shapes change I enable the octahedral compression flag in ALL cases rather than just cases where octahedral was being used

Specifically this line here: https://github.com/godotengine/godot/pull/56161/files#diff-f917aca210cab3aab9ba53811c7fda61df213876b31d9d90df84de0edc716b3cR4303

It probably needs an if statement to check if the surface actually has octahedral compression enabled, I have no idea how I missed that, probably I was tunnel visioning on getting it to work on fresh imports (which have octahedral enabled by default) haha

I'll see if that fixes things later this week

Thanks for bringing this to my attention! Apologies for any trouble caused!

[TokisanGames]

There's no way to turn it off within the mesh settings in the inspector. That should be fixed. When we import glbs, we convert them to Godot's resources and delete the glbs. If such an object is in the scene, it affects everything. Here the model has blendshapes. Take her out of the scene and reload it, and the lighting is fine. With her in the scene it affects every single mesh and material, lights or no, shaders or standard material, imported meshes or godot primitives.

3.4.3 is unusable for us, and maybe should be pulled @akien-mga.

[Calinou]

When we import glbs, we convert them to Godot's resources and delete the glbs.

This seems like an unusual workflow. Is there any reason you're doing that? Since Godot 3.0, it's expected that you always keep the source files around.
If reducing the Git history size of your project repository is a goal, maybe look into using a separate repository to store Blender and glTF files.

3.4.3 is unusable for us, and maybe should be pulled

Releases are never pulled, as it's considered bad practice to remove releases that were already made. We also don't use a concept of "yanking" existing releases, as the current regression does not affect all projects (and is not a security issue).

[TokisanGames]

When we import glbs, we convert them to Godot's resources and delete the glbs.
This seems like an unusual workflow. Is there any reason you're doing that? Since Godot 3.0, it's expected that you always keep the source files around. If reducing the Git history size of your project repository is a goal, maybe look into using a separate repository to store Blender and glTF files.

We definitely do not store our blender files in the repo. Some of them are a few gb. Nor our glbs, as they will contain embedded textures and animations, which we do not want embedded. We don't want any locked resources as we need to move things around in the scene tree, adding child nodes, bone attachments, introducing our LOD plugin. We convert textures to DDS, with text materials. Binary materials are no good for git as it can't see changes. Animations we save separately, as text. The glb just duplicates all of this separated data. Our current export is 4gb w/ binary resources. Including the glbs would be much bigger.

Our current workflow is not great. However, the "expected" workflow is more restrictive, which is why we do it this way. We need control over the assets and storage space.

[Calinou]

Our current export is 4gb w/ binary resources. Including the glbs would be much bigger.

Exported projects do not include .glb files as they're considered source assets. The same thing happens with any resource imported by Godot: only the resource within the .import/ folder is included in the export PCK.

[The-O-King]

Just following up with this - I found the issue, it was not caused by what I thought previously (I was properly setting the conditionals for the blend shapes shader)

But rather the issue was that I was setting the scene shader's state outside of the primary rendering function for blend shapes, and that function keeps track of the previous render state to try to avoid unnecessary state changes; this caused the previous state to be stored incorrectly in some scenarios, and things get out of sync, and result in the rendering behavior you all have reported

I submitted a fix for it that has been linked here (I tested with the minimal project shared here and things looked good with the character scene)

[akien-mga]

Fixed by #58808.

[akien-mga]

The fix for this issue is included in 3.4.4 RC 1 released today: https://godotengine.org/article/release-candidate-godot-3-4-4-rc-1
It would be great if you could confirm that it works fine on your main projects.

[Snaker1]

The fix for this issue is included in 3.4.4 RC 1 released today: https://godotengine.org/article/release-candidate-godot-3-4-4-rc-1 It would be great if you could confirm that it works fine on your main projects.

Yes, can confirm. For me everything works fine now on 3.4.4 RC 1 👍

[TokisanGames]

Thanks, but no the issue is not resolved. It fixes the normals on other meshes, however the normals are still messed up for the meshes with blendshapes. See this video for a demonstration.

https://youtu.be/3UvPn_6miHI

[akien-mga]

@tinmanjuggernaut Can you make a reproduction project that exhibits the error?

[TokisanGames]

Here you go. https://we.tl/t-SgawumyYj8

This model looks vastly different between 3.4.2 and 3.4.4-rc1. You can also play the animation in the player to rotate her.

I also noted that on her back at various points the model looks messed up even in 3.4.2, just like what this was originally trying to fix, as shown in #55633.

[clayjohn]

@tinmanjuggernaut Looking at your project, it looks like you have baked an imported mesh into an ArrayMesh (saved as a ".res") without octahedral compression and are now opening it a new version that defaults to octahedral compression. Running your scene I can see that the associated surface formats for your mesh have octahedral compression enabled.

Perhaps we need a way to change surface formats for meshes that have been baked into ArrayMeshes. While we may not expect users to bake assets like that, it seems like there is no way around this issue for projects like yours.

[The-O-King]

hmmmm wait question - perhaps I have a misunderstanding of how .res files work - but if the .res file was saved in a previous version of the engine, does it not have the surface formats flags alongside it? If it doesn't have the surface format flags does that mean it just uses whatever the default in the engine is set to?

And if it does save the original surface formats flag, I wonder why/how it would add the octahedral compression flag when loaded into the updated version of the engine (since it currently is another, new flag added to the surface format flag enum)? That feels like something that can be fixed at the engine level?

[TokisanGames]

@tinmanjuggernaut Looking at your project, it looks like you have baked an imported mesh into an ArrayMesh (saved as a ".res") without octahedral compression and are now opening it a new version that defaults to octahedral compression.

I have never heard of octahedral compression until my meshes started turning black. If you look at her back in 3.4.2, you'll see similar artifacts shown in #55633 for meshes with octahedral compression.

Regardless, it's not something we chose. As indicated before, our workflow is to either save the arraymesh to a file as a binary res, or for small meshes, we leave it as an arraymesh in the scene file, stored as text. We never leave a scene inheriting from the import format.

Unlike what has been suggested twice now, I doubt our workflow is unusual. I suspect it is more common then some of you think.

When anyone imports a glb or fbx, and finds it locked, some will manually clear inheritance so they can work with it. Or they might change the parent node type in the scene, say from spatial to a physicsbody, which automatically clears inheritance. Or they might open the file, copy the mesh resource, switch to an existing scene, and paste the resource. I've done that countless times when reimporting.

All scenarios will create an arraymesh resource which is stored as text in the scene file. The import file is then no longer used and free to be deleted, thus the import settings are now permanent until it is manually reimported.

We are just more explicit about it, by saving the resource out of the scene file. Either way, the arraymesh is easily, and automatically baked with the user unaware of the baking or the settings chosen, which cannot be changed later.

This model is only a development iteration and we will be importing a new version. So reimporting this one (the only one right now with blend shapes) is no big deal if needed. But I'm not going to reimport my hundreds of other models (without blend shapes). I will switch my team to a custom build without this patch before doing that.

[Zireael07]

When anyone imports a glb or fbx, and finds it locked, some will manually clear inheritance so they can work with it. Or they might change the parent node type in the scene, say from spatial to a physicsbody, which automatically clears inheritance. Or they might open the file, copy the mesh resource, switch to an existing scene, and paste the resource. I've done that countless times when reimporting.

Changing parent type and copy/pasting happened to me, so yep!
I never paid much attention to what happens under the hood, and I probably have the source files still, but I would find this behavior very annoying if it caused my arraymeshes to suddenly behave differently.

[TokisanGames]

Or one might have an existing scene, drag the new imported glb into the scene and make the node local so you can replace the old meshinstance with the new. Or they'd need to clear the inheritance if they use a LOD system and need to set up the scene properly, or add in an animation tree. Even in Op's screenshots here, and in #58789 both have cleared their inheritance. I expect 99% of the projects out there are using generated arraymeshes stored in the scene files.

@akien-mga The bottom line is that you guys introduced a non-backwards compatible, game-breaking change to engine defaults in a maintenance patch release, without the ability to change it back, and without notice. Any one of those is enough reason to pull 3.4.3, and/or release 3.4.4 reverting this patch entirely.

It could then be pushed to 3.5, hopefully with the added feature of adjusting the setting directly in the arraymesh so we don't have to reimport. Then in the release notes providing notice that "the engine defaults are changing and we have to do X and why, and it's not backwards compatible." Even after all of this discussion I still don't understand what I need to do or why, except try some stuff with reimporting in different versions with different settings until it looks correct.

For us it's only one development mesh w/ blendshapes, but if we were further along it could potentially be many complex AAA character models w/ animations that need to be adjusted and added to an animation tree, LODS and other custom settings that need to be set up within Godot. It can be very time-consuming to reimport.

[The-O-King]

ok so I found some time to test things just now - and while yes I do see the issues you are referring to in the screenshots and example project that you provided using the 3.4.4-rc1 release, I do NOT see the issue opening the example project you provided with a build of the engine with the 3.x branch here on github - perhaps we missed including a fix we submitted recently in the rc?

Let me know if perhaps I'm missing something with regards to setting up the project to reproduce the issue though

[TokisanGames]

I used 3.4.4-rc1 based on what @akien-mga said, and produced a project that demonstrates the issue there. 3.4.3 and 3.4.4-rc1 are definitely broken.

I just built 3.x 8975470. Yes both the demo project and my development scene posted above look fine. Great, whatever the current settings are now in 3.x work fine for me. Thanks.

[akien-mga]

That's great news :) I'll check the 3.4 commits and see what I might have missed from the 3.x changes. Maybe I messed up a merge conflict resolution.

[akien-mga]

@The-O-King I compared the commits from you on both 3.x and 3.4 branches and it seems like I didn't miss any to cherry-pick:
https://github.com/godotengine/godot/commits/3.x?author=The-O-King
https://github.com/godotengine/godot/commits/3.4?author=The-O-King
(It's the 5 top commits, older ones were made prior to the 3.4 branching.)

The commits look to have the exact same changes. So I assume there have been other changes in 3.x which make the fixes work, while something is still off in 3.4. It's a bit outside my area of expertise, would you mind having a look to see what differs in the two branches for the relevant blend shapes code?

Also relevant to review, drivers/gles3 commit history for 3.x and 3.4:
https://github.com/godotengine/godot/commits/3.x/drivers/gles3
https://github.com/godotengine/godot/commits/3.4/drivers/gles3

A major difference is of course all the async shader compilation / ubershader code which was merged for 3.5.

[TokisanGames]

I just built 3.4 6c3cfc7 and can confirm that this branch definitely exhibits the issues I showed and built the test project for on blend shape meshes. 3.4 broken, 3.x fine.
Thanks.

[RandomShaper]

Would you be so kind as to try this patch?
patch.txt

diff --git a/drivers/gles3/shaders/blend_shape.glsl b/drivers/gles3/shaders/blend_shape.glsl
index 2a0aa6a06d..3efb6483d7 100644
--- a/drivers/gles3/shaders/blend_shape.glsl
+++ b/drivers/gles3/shaders/blend_shape.glsl
@@ -26,7 +26,7 @@ ARRAY_INDEX=8,
 layout(location = 0) in highp VFORMAT vertex_attrib;
 /* clang-format on */
 #ifdef ENABLE_OCTAHEDRAL_COMPRESSION
-layout(location = 2) in vec4 normal_tangent_attrib;
+layout(location = 1) in vec4 normal_tangent_attrib;
 #else
 layout(location = 1) in vec3 normal_attrib;
 #endif

[akien-mga]

@RandomShaper You rock! This fixes it improves the situation for the MRP in #58592 (comment)

It's worth noting that the 3.x branch also uses location = 2 for this - so that fix might be needed in 3.x too, though the bug isn't reproducible?

---

Edit: It might only be part of the solution, I see there's still a difference between the results on each branch with the above MRP.

• Current 3.4 branch (0ae6150):

• 3.4 branch + attribute location = 1 patch:

• Current 3.x branch (c2c0c80):

• 3.x branch + attribute location = 1 patch:

All screenshots made right after opening the editor, so the lighting difference comes from the renderer code, not scene setup.

[RandomShaper]

It's worth noting that the 3.x branch also uses location = 2 for this - so that fix might be needed in 3.x too, though the bug isn't reproducible?

In 3.x location = 2 is correct. (Explanation: for the ubershader locations where juggled so each one had one an only one type across all "conditionings". If location 1 is vec3 under some #if, it must be as well for its #else, so to speak.)

Now, regarding these screenshots, I'd say 3.4 branch + attribute location = 1 patch and Current 3.x branch look already correct, considering the lighting difference is a matter of different defaults across Godot versions. Or am I overlooking something?

[clayjohn]

It's worth noting that the 3.x branch also uses location = 2 for this - so that fix might be needed in 3.x too, though the bug isn't reproducible?

In 3.x location = 2 is correct. (Explanation: for the ubershader locations where juggled so each one had one an only one type across all "conditionings". If location 1 is vec3 under some #if, it must be as well for its #else, so to speak.)

Now, regarding these screenshots, I'd say 3.4 branch + attribute location = 1 patch and Current 3.x branch look already correct, considering the lighting difference is a matter of different defaults across Godot versions. Or am I overlooking something?

3.4 + your patch indeed looks correct for normals, but the IBL is clearly broken. So the patch probably needs another similar change to ensure the environment map is correct.

[RandomShaper]

I've built latest 3.4 + the location patch and it looks this way to me:

[TokisanGames]

It seems @akien-mga and @RandomShaper got different results with 3.4 and this patch. I built 3.4 0ae6150 plus this patch location=1 and the MRP and my scene look fine. I then went back to location=2 and see the problem again, and then back again and can confirm this is definitely a fix.

3.4 + your patch indeed looks correct for normals, but the IBL is clearly broken.

Unlike what Remi showed, I do see environment lighting in the scene along with the directional light.

Edit: Also I don't know if 3.4 has shader caching or I just have saved the settings in my project file, but both enabled and disabled were fine.

[akien-mga]

I'll check again, might be a driver specific bug unrelated to this issue. Either way, feel free to PR your fix @RandomShaper :)

[akien-mga]

Tested again, it seems to work fine for me with the patch now, I can't reproduce the broken IBL I got yesterday. ¯\_(ツ)_/¯

[akien-mga]

Fixed in 3.x by #58808 and in 3.4 by #59159.

[akien-mga]

This fix is in 3.4.4 RC 2: https://godotengine.org/article/release-candidate-godot-3-4-4-rc-2

CC @SkanerSoft if you can check if that also fixes your octahedral compression issues.

[The-O-King]

Hey thanks for helping look into things @RandomShaper and everyone else! Excellent stuff!

Sorry I wasn't able to look myself between the branches things got busy this past week. I'm glad to see it seems to be all sorted out here though :)

[Cheeseness]

I'm a bit late to the party, but for whatever it's worth, I can confirm that the 3.4.4 RC2 resolves bad normals on shape keyed meshes in my 3.4.3 projects as well. Many thanks!