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

Metal: Validation fails if a shader program samples from the same texture in both fragment and vertex shaders #3024

Open
rogual opened this issue Jan 19, 2023 · 8 comments

Comments

@rogual
Copy link
Contributor

rogual commented Jan 19, 2023

Describe the bug

Certain shaders cause Metal API Validation to fail. The condition seems to be:

  • The vertex shader samples from a texture, AND
  • The fragment shader samples from the same texture.

To Reproduce

I couldn't find any shader program in the BGFX examples that samples from a texture in both the vertex and fragment shaders, so I've modified 27-terrain to do so.

Steps to reproduce the behavior:

  1. Apply the patch to 27-terrain
  2. Compile the example.
  3. Compile the shader for Metal.
  4. Run the example with Metal API Validation enabled (steps to enable are below)

Expected behavior
The example should continue run as normal, but with the terrain textured with its own heightmap.

Observed behaviour
The program aborts with a Metal API Validation error:

failed assertion `Vertex Function(xlatMtlMain): missing sampler binding at index 0 for s_heightTextureSampler[0].'

Additional context

Why this is important

  • If Metal API Validation fails, you can't capture a frame in the Metal graphics debugger.
  • I've heard it also causes Apple to reject apps for the App Store.

To run the examples with Metal API Validation enabled

These instructions are for XCode 11.3.

  • Start a new empty XCode project
  • Product -> Scheme -> Edit Scheme...
  • Choose "Run" on the left
  • Choose "Info" tab on the right
    • Under "Executable", choose "Other" and then choose the bgfx examples.app
  • Choose "Options" tab on the right
    • Set "Metal API Validation" to "Enabled"
    • Set "Working Directory" to bgfx/examples/runtime
  • Close the scheme editor
  • Product -> Run
  • The examples app should run

Test Case

You can find the test case on my branch here: https://github.com/rogual/bgfx/tree/shader-validation-fail-test-case

The diff is here:

diff --git a/examples/27-terrain/fs_terrain.sc b/examples/27-terrain/fs_terrain.sc
index aff04bea0..586fae170 100644
--- a/examples/27-terrain/fs_terrain.sc
+++ b/examples/27-terrain/fs_terrain.sc
@@ -7,7 +7,9 @@ $input v_position, v_texcoord0
 
 #include "../common/common.sh"
 
+SAMPLER2D(s_heightTexture, 0);
+
 void main()
 {
-	gl_FragColor = vec4(v_texcoord0.x, v_texcoord0.y, v_position.y / 50.0, 1.0);
-}
+	gl_FragColor = texture2D(s_heightTexture, v_texcoord0.xy);
+}
\ No newline at end of file
diff --git a/examples/27-terrain/terrain.cpp b/examples/27-terrain/terrain.cpp
index 3b493304a..5afc36808 100644
--- a/examples/27-terrain/terrain.cpp
+++ b/examples/27-terrain/terrain.cpp
@@ -125,7 +125,7 @@ ExampleTerrain(const char* _name, const char* _description, const char* _url)
 
 		uint32_t num = s_terrainSize * s_terrainSize;
 
-		m_terrain.m_mode      = 0;
+		m_terrain.m_mode      = 2;
 		m_terrain.m_dirty     = true;
 		m_terrain.m_vertices  = (PosTexCoord0Vertex*)BX_ALLOC(entry::getAllocator(), num * sizeof(PosTexCoord0Vertex) );
 		m_terrain.m_indices   = (uint16_t*)BX_ALLOC(entry::getAllocator(), num * sizeof(uint16_t) * 6);
@@ -460,6 +460,7 @@ ExampleTerrain(const char* _name, const char* _description, const char* _url)
 			bgfx::setViewTransform(0, m_viewMtx, m_projMtx);
 			bgfx::setTransform(m_terrain.m_transform);
 
+			bgfx::setTexture(0, s_heightTexture, m_heightTexture);
 			switch (m_terrain.m_mode)
 			{
 			default:
@@ -477,7 +478,6 @@ ExampleTerrain(const char* _name, const char* _description, const char* _url)
 			case 2:
 				bgfx::setVertexBuffer(0, m_vbh);
 				bgfx::setIndexBuffer(m_ibh);
-				bgfx::setTexture(0, s_heightTexture, m_heightTexture);
 				bgfx::submit(0, m_terrainHeightTextureProgram);
 				break;
 			}
diff --git a/examples/runtime/shaders/metal/fs_terrain.bin b/examples/runtime/shaders/metal/fs_terrain.bin
index 7092e84b1..0edbc2363 100644
Binary files a/examples/runtime/shaders/metal/fs_terrain.bin and b/examples/runtime/shaders/metal/fs_terrain.bin differ
@rogual
Copy link
Contributor Author

rogual commented Jan 19, 2023

@attilaz seems to get CC'd on every Metal issue, so, cc: @attilaz :)

@rogual rogual closed this as completed Jan 19, 2023
@rogual
Copy link
Contributor Author

rogual commented Jan 19, 2023

Sorry, I keep making stupid mistakes and editing this. Now it's working. There is an issue here somewhere, but I'll reopen it when I can make a proper issue report.

@bkaradzic
Copy link
Owner

You should not open issues if you're not sure you have actual issue... Use Discussions or Discord instead.

@rogual
Copy link
Contributor Author

rogual commented Jan 19, 2023

I was sure! I was just wrong 🙁

@rogual rogual changed the title Metal: Validation fails if a shader program samples from textures in both fragment and vertex shaders Metal: Validation fails if a shader program samples from the same texture in both fragment and vertex shaders Jan 19, 2023
@rogual
Copy link
Contributor Author

rogual commented Jan 19, 2023

Alright, I've fixed my test case. It's when vertex and fragment shaders sample from specifically the same texture.

I won't reopen until I've confirmed on Discord that this is a proper bug.

@pezcode
Copy link
Contributor

pezcode commented Jan 19, 2023

Might be a similar issue to #2410 (which is fixed btw, that issue should be closed)

@rogual
Copy link
Contributor Author

rogual commented Jan 20, 2023

OK, it sounds like this is a bug then. Reopening.

@rogual rogual reopened this Jan 20, 2023
@nathan-stouffer-onx
Copy link

I am encountering the same issue. Has there been any more discussion on this issue?

attilaz added a commit to attilaz/bgfx that referenced this issue Dec 4, 2023
Fixes metal texture/buffer binding when when the same resource is bound to both (vertex and fragment) stages

bkaradzic#3024
bkaradzic pushed a commit that referenced this issue Dec 4, 2023
Fixes metal texture/buffer binding when when the same resource is bound to both (vertex and fragment) stages

#3024
jay3d pushed a commit to jay3d/bgfx that referenced this issue Dec 7, 2023
Fixes metal texture/buffer binding when when the same resource is bound to both (vertex and fragment) stages

bkaradzic#3024
mipek pushed a commit to mipek/bgfx that referenced this issue Mar 2, 2024
Fixes metal texture/buffer binding when when the same resource is bound to both (vertex and fragment) stages

bkaradzic#3024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants