rhi: metal: Skip inactive resources

The Quick3D-on-RHI PoC demonstrates a case which the Metal backend
fails to handle correctly: have an object with a lighting-enabled
material, but remove all lights from the scene.

Under the hood this means having a uniform block in the shader, but
without referencing it in any way in the actual shader code.

This leads to the resource being present (as far as shader reflection
is concerned), but with no native binding point available, meaning the
attempt to retrieve the Metal binding point for it returns -1, and that
is what the QShader carries in the nativeResourceBindingMap.

The backend should be prepared to silently skip the resource, whereas
 currently we end up in an assertion due to attempting to batch the (native)
binding "-1", which is invalid.

Correct this.

Change-Id: I85ee58145f589aca45d46c23e0cdce837d598850
Fixes: QTBUG-80668
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>

2 files changed, 81 insertions, 30 deletions

diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm
index 3aa68db585..b6ca40e08b 100644
--- a/src/gui/rhi/qrhimetal.mm
+++ b/src/gui/rhi/qrhimetal.mm
@@ -673,12 +673,17 @@ static inline int mapBinding(int binding,
                              BindingType type)
 {
     const QShader::NativeResourceBindingMap *map = nativeResourceBindingMaps[stageIndex];
-    if (map) {
-        auto it = map->constFind(binding);
-        if (it != map->cend())
-            return type == BindingType::Sampler ? it->second : it->first;
-    }
-    return binding;
+    if (!map)
+        return binding; // old QShader versions do not have this map, assume 1:1 mapping then
+
+    auto it = map->constFind(binding);
+    if (it != map->cend())
+        return type == BindingType::Sampler ? it->second : it->first; // may be -1, if the resource is inactive
+
+    // Hitting this path is normal too, is not given that the resource (e.g. a
+    // uniform block) is really present in the shaders for all the stages
+    // specified by the visibility mask in the QRhiShaderResourceBinding.
+    return -1;
 }
 
 void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD,
@@ -712,16 +717,25 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD
                 }
             }
             if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) {
-                res[VERTEX].buffers.feed(mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf);
-                res[VERTEX].bufferOffsets.feed(b->binding, offset);
+                const int nativeBinding = mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Buffer);
+                if (nativeBinding >= 0) {
+                    res[VERTEX].buffers.feed(nativeBinding, mtlbuf);
+                    res[VERTEX].bufferOffsets.feed(b->binding, offset);
+                }
             }
             if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) {
-                res[FRAGMENT].buffers.feed(mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf);
-                res[FRAGMENT].bufferOffsets.feed(b->binding, offset);
+                const int nativeBinding = mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Buffer);
+                if (nativeBinding >= 0) {
+                    res[FRAGMENT].buffers.feed(nativeBinding, mtlbuf);
+                    res[FRAGMENT].bufferOffsets.feed(b->binding, offset);
+                }
             }
             if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) {
-                res[COMPUTE].buffers.feed(mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf);
-                res[COMPUTE].bufferOffsets.feed(b->binding, offset);
+                const int nativeBinding = mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Buffer);
+                if (nativeBinding >= 0) {
+                    res[COMPUTE].buffers.feed(nativeBinding, mtlbuf);
+                    res[COMPUTE].bufferOffsets.feed(b->binding, offset);
+                }
             }
         }
             break;
@@ -730,16 +744,28 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD
             QMetalTexture *texD = QRHI_RES(QMetalTexture, b->u.stex.tex);
             QMetalSampler *samplerD = QRHI_RES(QMetalSampler, b->u.stex.sampler);
             if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) {
-                res[VERTEX].textures.feed(mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex);
-                res[VERTEX].samplers.feed(mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState);
+                const int nativeBindingTexture = mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Texture);
+                const int nativeBindingSampler = mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Sampler);
+                if (nativeBindingTexture >= 0 && nativeBindingSampler >= 0) {
+                    res[VERTEX].textures.feed(nativeBindingTexture, texD->d->tex);
+                    res[VERTEX].samplers.feed(nativeBindingSampler, samplerD->d->samplerState);
+                }
             }
             if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) {
-                res[FRAGMENT].textures.feed(mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex);
-                res[FRAGMENT].samplers.feed(mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState);
+                const int nativeBindingTexture = mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Texture);
+                const int nativeBindingSampler = mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Sampler);
+                if (nativeBindingTexture >= 0 && nativeBindingSampler >= 0) {
+                    res[FRAGMENT].textures.feed(nativeBindingTexture, texD->d->tex);
+                    res[FRAGMENT].samplers.feed(nativeBindingSampler, samplerD->d->samplerState);
+                }
             }
             if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) {
-                res[COMPUTE].textures.feed(mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Texture), texD->d->tex);
-                res[COMPUTE].samplers.feed(mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Sampler), samplerD->d->samplerState);
+                const int nativeBindingTexture = mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Texture);
+                const int nativeBindingSampler = mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Sampler);
+                if (nativeBindingTexture >= 0 && nativeBindingSampler >= 0) {
+                    res[COMPUTE].textures.feed(nativeBindingTexture, texD->d->tex);
+                    res[COMPUTE].samplers.feed(nativeBindingSampler, samplerD->d->samplerState);
+                }
             }
         }
             break;
@@ -751,12 +777,21 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD
         {
             QMetalTexture *texD = QRHI_RES(QMetalTexture, b->u.simage.tex);
             id<MTLTexture> t = texD->d->viewForLevel(b->u.simage.level);
-            if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage))
-                res[VERTEX].textures.feed(mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Texture), t);
-            if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage))
-                res[FRAGMENT].textures.feed(mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Texture), t);
-            if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage))
-                res[COMPUTE].textures.feed(mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Texture), t);
+            if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) {
+                const int nativeBinding = mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Texture);
+                if (nativeBinding >= 0)
+                    res[VERTEX].textures.feed(nativeBinding, t);
+            }
+            if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) {
+                const int nativeBinding = mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Texture);
+                if (nativeBinding >= 0)
+                    res[FRAGMENT].textures.feed(nativeBinding, t);
+            }
+            if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) {
+                const int nativeBinding = mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Texture);
+                if (nativeBinding >= 0)
+                    res[COMPUTE].textures.feed(nativeBinding, t);
+            }
         }
             break;
         case QRhiShaderResourceBinding::BufferLoad:
@@ -769,16 +804,25 @@ void QRhiMetal::enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD
             id<MTLBuffer> mtlbuf = bufD->d->buf[0];
             uint offset = uint(b->u.sbuf.offset);
             if (b->stage.testFlag(QRhiShaderResourceBinding::VertexStage)) {
-                res[VERTEX].buffers.feed(mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf);
-                res[VERTEX].bufferOffsets.feed(b->binding, offset);
+                const int nativeBinding = mapBinding(b->binding, VERTEX, nativeResourceBindingMaps, BindingType::Buffer);
+                if (nativeBinding >= 0) {
+                    res[VERTEX].buffers.feed(nativeBinding, mtlbuf);
+                    res[VERTEX].bufferOffsets.feed(b->binding, offset);
+                }
             }
             if (b->stage.testFlag(QRhiShaderResourceBinding::FragmentStage)) {
-                res[FRAGMENT].buffers.feed(mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf);
-                res[FRAGMENT].bufferOffsets.feed(b->binding, offset);
+                const int nativeBinding = mapBinding(b->binding, FRAGMENT, nativeResourceBindingMaps, BindingType::Buffer);
+                if (nativeBinding >= 0) {
+                    res[FRAGMENT].buffers.feed(nativeBinding, mtlbuf);
+                    res[FRAGMENT].bufferOffsets.feed(b->binding, offset);
+                }
             }
             if (b->stage.testFlag(QRhiShaderResourceBinding::ComputeStage)) {
-                res[COMPUTE].buffers.feed(mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Buffer), mtlbuf);
-                res[COMPUTE].bufferOffsets.feed(b->binding, offset);
+                const int nativeBinding = mapBinding(b->binding, COMPUTE, nativeResourceBindingMaps, BindingType::Buffer);
+                if (nativeBinding >= 0) {
+                    res[COMPUTE].buffers.feed(nativeBinding, mtlbuf);
+                    res[COMPUTE].bufferOffsets.feed(b->binding, offset);
+                }
             }
         }
             break;
diff --git a/src/gui/rhi/qshader.cpp b/src/gui/rhi/qshader.cpp
index dc6060f882..0b99281f08 100644
--- a/src/gui/rhi/qshader.cpp
+++ b/src/gui/rhi/qshader.cpp
@@ -660,6 +660,13 @@ QDebug operator<<(QDebug dbg, const QShaderVersion &v)
     pair, because combined image samplers may map to two native resources (a
     texture and a sampler) in some shading languages. In that case the second
     value refers to the sampler.
+
+    \note The native binding may be -1, in case there is no active binding for
+    the resource in the shader. (for example, there is a uniform block
+    declared, but it is not used in the shader code) The map is always
+    complete, meaning there is an entry for all declared uniform blocks,
+    storage blocks, image objects, and combined samplers, but the value will be
+    -1 for those that are not actually referenced in the shader functions.
 */
 
 /*!