bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
15.02k stars 1.95k forks source link

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

Open rogual opened 1 year ago

rogual commented 1 year ago

Describe the bug

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

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

To run the examples with Metal API Validation enabled

These instructions are for XCode 11.3.

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 commented 1 year ago

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

rogual commented 1 year ago

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 commented 1 year ago

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

rogual commented 1 year ago

I was sure! I was just wrong 🙁

rogual commented 1 year ago

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 commented 1 year ago

Might be a similar issue to https://github.com/bkaradzic/bgfx/issues/2410 (which is fixed btw, that issue should be closed)

rogual commented 1 year ago

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

nathan-stouffer-onx commented 1 year ago

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