Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] iOS shader validation crash #42516

Open
naithar opened this issue Oct 2, 2020 · 8 comments
Open

[4.0] iOS shader validation crash #42516

naithar opened this issue Oct 2, 2020 · 8 comments

Comments

@naithar
Copy link
Contributor

naithar commented Oct 2, 2020

Godot version:
master branch.
Bisected the issue down to this commit: d0bddf5

OS/device including version:
iOS device with Vulkan renderer

Issue description:

Important note: crashes only on debug mode with Metal API Validation enabled. Disabling Metal API Validation in schema allows to run application and doesn't result in a crash.

Crash on application startup after specific shader compiles:

Crash log:

2020-10-02 21:16:04.467528+0300 godot_app[29739:11131332] [Metal Compiler Warning] Warning: Compilation succeeded with: 

program_source:221:12: warning: unused variable 'shadow_vertex'
    float2 shadow_vertex = vertex0;
           ^
program_source:222:11: warning: unused variable 'normal_depth'
    float normal_depth = 1.0;
          ^
program_source:220:12: warning: unused variable 'light_vertex'
    float3 light_vertex = float3(vertex0, 0.0);
           ^
program_source:219:12: warning: unused variable 'screen_uv'
    float2 screen_uv = float2(0.0);
           ^
program_source:229:12: warning: unused variable 'base_color'
    float4 base_color = color;
           ^
validateNewTexture:74: failed assertion `Offset of a buffer-backed texture with pixelFormat(MTLPixelFormatRGBA32Float) must be aligned to 64 bytes, found offset(1086800)'

Steps to reproduce:

  • Create a new clean exported iOS App for any project from current master
  • Run it in Xcode with default settings.
@naithar naithar changed the title [4.0] iOS crash [4.0] iOS shader validation crash Oct 2, 2020
@naithar
Copy link
Contributor Author

naithar commented Oct 2, 2020

Update: this seems to be happening only when application is launched in debug mode through Xcode
Disabling Metal API Validation in scheme settings also helps.

@akien-mga
Copy link
Member

Indeed, @bruvzg mentioned that issue on the PR too: #41810 (comment)

@naithar
Copy link
Contributor Author

naithar commented Oct 2, 2020

@akien-mga yeah, I've seen same errors running Godot Editor by Xcode, but it wasn't crashing.
My old 4.0 iOS project which I used to check my PRs also wasn't crashing, due to it having many default settings changed :|

@bruvzg
Copy link
Member

bruvzg commented Oct 2, 2020

It's definitely not crashing on macOS, just printing errors and features related to the failing sharer are broken.

And the errors are different, but similar unused variables errors do pop up on macOS too, but only when using MoltenVK debug build (see #40833).

Also, I was messing with the shaders that failing on macOS, and following patch seems to fix compilation and runtime errors in particle and SDFGI shaders (not the debug unused variable errors), but I'm not sure what effect it will have on performance, and if it's correct at all (also not tested it with latest MoltenVK release, some errors might be already fixed, or will be when Metal 3 is fully used):

diff --git a/servers/rendering/rasterizer_rd/rasterizer_scene_rd.cpp b/servers/rendering/rasterizer_rd/rasterizer_scene_rd.cpp
index 958d8eac1f..494cc7006b 100644
--- a/servers/rendering/rasterizer_rd/rasterizer_scene_rd.cpp
+++ b/servers/rendering/rasterizer_rd/rasterizer_scene_rd.cpp
@@ -317,9 +317,9 @@ void RasterizerSceneRD::sdfgi_update(RID p_render_buffers, RID p_environment, co
 		}
 
 		RD::TextureFormat tf_occlusion = tf_sdf;
-		tf_occlusion.format = RD::DATA_FORMAT_R16_UINT;
-		tf_occlusion.shareable_formats.push_back(RD::DATA_FORMAT_R16_UINT);
-		tf_occlusion.shareable_formats.push_back(RD::DATA_FORMAT_R4G4B4A4_UNORM_PACK16);
+		tf_occlusion.format = RD::DATA_FORMAT_R32_UINT;
+		tf_occlusion.shareable_formats.push_back(RD::DATA_FORMAT_R32_UINT);
+		tf_occlusion.shareable_formats.push_back(RD::DATA_FORMAT_A8B8G8R8_UNORM_PACK32);
 		tf_occlusion.depth *= sdfgi->cascades.size(); //use depth for occlusion slices
 		tf_occlusion.width *= 2; //use width for the other half
 
@@ -389,7 +389,7 @@ void RasterizerSceneRD::sdfgi_update(RID p_render_buffers, RID p_environment, co
 		sdfgi->occlusion_data = RD::get_singleton()->texture_create(tf_occlusion, RD::TextureView());
 		{
 			RD::TextureView tv;
-			tv.format_override = RD::DATA_FORMAT_R4G4B4A4_UNORM_PACK16;
+			tv.format_override = RD::DATA_FORMAT_A8B8G8R8_UNORM_PACK32;
 			sdfgi->occlusion_texture = RD::get_singleton()->texture_create_shared(tv, sdfgi->occlusion_data);
 		}
 
diff --git a/servers/rendering/rasterizer_rd/shaders/particles.glsl b/servers/rendering/rasterizer_rd/shaders/particles.glsl
index a924509771..2cadb387a1 100644
--- a/servers/rendering/rasterizer_rd/shaders/particles.glsl
+++ b/servers/rendering/rasterizer_rd/shaders/particles.glsl
@@ -80,7 +80,7 @@ struct ParticleEmission {
 	vec4 custom;
 };
 
-layout(set = 1, binding = 2, std430) restrict volatile coherent buffer SourceEmission {
+layout(set = 1, binding = 2, std430) restrict buffer SourceEmission {
 	int particle_count;
 	uint pad0;
 	uint pad1;
diff --git a/servers/rendering/rasterizer_rd/shaders/scene_high_end.glsl b/servers/rendering/rasterizer_rd/shaders/scene_high_end.glsl
index e11f3983c5..42f89b0f55 100644
--- a/servers/rendering/rasterizer_rd/shaders/scene_high_end.glsl
+++ b/servers/rendering/rasterizer_rd/shaders/scene_high_end.glsl
@@ -2580,7 +2580,7 @@ FRAGMENT_SHADER_CODE
 			}
 		}
 
-		imageAtomicOr(geom_facing_grid, grid_pos, facing_bits); //store facing bits
+		imageStore(geom_facing_grid, grid_pos, imageLoad(geom_facing_grid, grid_pos) | facing_bits); //store facing bits
 
 		if (length(emission) > 0.001) {
 			float lumas[6];

@naithar
Copy link
Contributor Author

naithar commented Oct 2, 2020

Applying this patch doesn't fix the crash, but it seems like there are a lot less errors with it.

@megalobyte
Copy link
Contributor

Not sure if related, but I got this error when running Godot. Please note that I'm still able to open Godot. Also, it very likely could be a setup issue 🙃

(base) ~/workspace/godot(master ✔) ./bin/godot.osx.tools.64
arguments
0: ./bin/godot.osx.tools.64
Current path: /Users/megalobyte/workspace/godot
Godot Engine v4.0.dev.custom_build.081c1ceda - https://godotengine.org
Vulkan API 1.2.189 - Using Vulkan Device #0: AMD - AMD Radeon Pro 5300M
[mvk-error] VK_ERROR_INITIALIZATION_FAILED: Shader library compile failed (Error code 3):
program_source:198:96: error: array subscript is not an integer
            uint _491 = atomic_fetch_add_explicit((device atomic_uint*)&density_only_map_atomic[pos], uint(final_density), memory_order_relaxed);
                                                                                               ^~~~
program_source:203:94: error: array subscript is not an integer
            uint _534 = atomic_fetch_add_explicit((device atomic_uint*)&light_only_map_atomic[pos], final_scattering, memory_order_relaxed);
                                                                                             ^~~~
program_source:213:97: error: array subscript is not an integer
                uint _590 = atomic_fetch_or_explicit((device atomic_uint*)&light_only_map_atomic[pos], force_max, memory_order_relaxed);
                                                                                                ^~~~
.
ERROR:  - Message Id Number: 0 | Message Id Name:
	VK_ERROR_INITIALIZATION_FAILED: Shader library compile failed (Error code 3):
program_source:198:96: error: array subscript is not an integer
            uint _491 = atomic_fetch_add_explicit((device atomic_uint*)&density_only_map_atomic[pos], uint(final_density), memory_order_relaxed);
                                                                                               ^~~~
program_source:203:94: error: array subscript is not an integer
            uint _534 = atomic_fetch_add_explicit((device atomic_uint*)&light_only_map_atomic[pos], final_scattering, memory_order_relaxed);
                                                                                             ^~~~
program_source:213:97: error: array subscript is not an integer
                uint _590 = atomic_fetch_or_explicit((device atomic_uint*)&light_only_map_atomic[pos], force_max, memory_order_relaxed);
                                                                                                ^~~~
.
	Objects - 1
		Object[0] - VK_OBJECT_TYPE_SHADER_MODULE, Handle 140612549813328
   at: _debug_messenger_callback (drivers/vulkan/vulkan_context.cpp:157)
[mvk-error] VK_ERROR_INVALID_SHADER_NV: Compute shader function could not be compiled into pipeline. See previous logged error.
ERROR:  - Message Id Number: 0 | Message Id Name:
	VK_ERROR_INVALID_SHADER_NV: Compute shader function could not be compiled into pipeline. See previous logged error.
	Objects - 1
		Object[0] - VK_OBJECT_TYPE_PIPELINE, Handle 140612549866496
   at: _debug_messenger_callback (drivers/vulkan/vulkan_context.cpp:157)
ERROR: vkCreateComputePipelines failed with error -1000012000.
   at: compute_pipeline_create (drivers/vulkan/rendering_device_vulkan.cpp:6629)

Registered camera FaceTime HD Camera (Built-in) with id 1 position 0 at index 0

@N0hbdy
Copy link
Contributor

N0hbdy commented Nov 19, 2021

@megalobyte Able to repro as well on mac.

It seems to originate from the volumetric_fog.glsl shader (commit 1b2cd9f), specifically failing on the imageAtomicAdd and friends. I'm no expert, but wondering if the molten rewrite is causing an issue in that it doesn't support a vec3 for pos? Seems valid glsl otherwise...

@Calinou
Copy link
Member

Calinou commented Nov 19, 2021

@N0hbdy See #53353 (comment).

@clayjohn clayjohn modified the milestones: 4.0, 4.x Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants