rhi: Sanity check the srb in debug builds

Instead of cryptic assertions and crashes depending on the backend,
show some useful warnings (in debug builds only) when one tries to
create an srb with a list where there are duplicated bindings. (a
mistake that happens relatively often during the development of
frameworks, such as Quick 3D, on top)

Change-Id: If1b50a2e8165b001878ad566e048f146e636514f
Reviewed-by: Andy Nichols <andy.nichols@qt.io>

6 files changed, 91 insertions, 2 deletions

diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp
index 632bfd0b29..b052a2f140 100644
--- a/src/gui/rhi/qrhi.cpp
+++ b/src/gui/rhi/qrhi.cpp
@@ -3400,7 +3400,7 @@ QDebug operator<<(QDebug dbg, const QRhiShaderResourceBinding &b)
                       << ')';
         break;
     default:
-        Q_UNREACHABLE();
+        dbg.nospace() << " UNKNOWN()";
         break;
     }
     dbg.nospace() << ')';
@@ -4350,6 +4350,79 @@ bool QRhiImplementation::sanityCheckGraphicsPipeline(QRhiGraphicsPipeline *ps)
     return true;
 }
 
+bool QRhiImplementation::sanityCheckShaderResourceBindings(QRhiShaderResourceBindings *srb)
+{
+#ifndef QT_NO_DEBUG
+    bool bindingsOk = true;
+    const int CHECKED_BINDINGS_COUNT = 64;
+    bool bindingSeen[CHECKED_BINDINGS_COUNT] = {};
+    for (auto it = srb->cbeginBindings(), end = srb->cendBindings(); it != end; ++it) {
+        const int binding = it->data()->binding;
+        if (binding >= CHECKED_BINDINGS_COUNT)
+            continue;
+        if (binding < 0) {
+            qWarning("Invalid binding number %d", binding);
+            bindingsOk = false;
+            continue;
+        }
+        switch (it->data()->type) {
+        case QRhiShaderResourceBinding::UniformBuffer:
+            if (!bindingSeen[binding]) {
+                bindingSeen[binding] = true;
+            } else {
+                qWarning("Uniform buffer duplicates an existing binding number %d", binding);
+                bindingsOk = false;
+            }
+            break;
+        case QRhiShaderResourceBinding::SampledTexture:
+            if (!bindingSeen[binding]) {
+                bindingSeen[binding] = true;
+            } else {
+                qWarning("Combined image sampler duplicates an existing binding number %d", binding);
+                bindingsOk = false;
+            }
+            break;
+        case QRhiShaderResourceBinding::ImageLoad:
+            Q_FALLTHROUGH();
+        case QRhiShaderResourceBinding::ImageStore:
+            Q_FALLTHROUGH();
+        case QRhiShaderResourceBinding::ImageLoadStore:
+            if (!bindingSeen[binding]) {
+                bindingSeen[binding] = true;
+            } else {
+                qWarning("Image duplicates an existing binding number %d", binding);
+                bindingsOk = false;
+            }
+            break;
+        case QRhiShaderResourceBinding::BufferLoad:
+            Q_FALLTHROUGH();
+        case QRhiShaderResourceBinding::BufferStore:
+            Q_FALLTHROUGH();
+        case QRhiShaderResourceBinding::BufferLoadStore:
+            if (!bindingSeen[binding]) {
+                bindingSeen[binding] = true;
+            } else {
+                qWarning("Buffer duplicates an existing binding number %d", binding);
+                bindingsOk = false;
+            }
+            break;
+        default:
+            qWarning("Unknown binding type %d", int(it->data()->type));
+            bindingsOk = false;
+            break;
+        }
+    }
+
+    if (!bindingsOk) {
+        qWarning() << *srb;
+        return false;
+    }
+#else
+    Q_UNUSED(srb);
+#endif
+    return true;
+}
+
 /*!
     \internal
  */
diff --git a/src/gui/rhi/qrhi_p_p.h b/src/gui/rhi/qrhi_p_p.h
index b5cf660409..1dbb2b9ab9 100644
--- a/src/gui/rhi/qrhi_p_p.h
+++ b/src/gui/rhi/qrhi_p_p.h
@@ -210,6 +210,7 @@ public:
     }
 
     bool sanityCheckGraphicsPipeline(QRhiGraphicsPipeline *ps);
+    bool sanityCheckShaderResourceBindings(QRhiShaderResourceBindings *srb);
 
     QRhi *q;
 
diff --git a/src/gui/rhi/qrhid3d11.cpp b/src/gui/rhi/qrhid3d11.cpp
index 71b8e48da0..d5c32cde2c 100644
--- a/src/gui/rhi/qrhid3d11.cpp
+++ b/src/gui/rhi/qrhid3d11.cpp
@@ -3421,6 +3421,10 @@ bool QD3D11ShaderResourceBindings::create()
     if (!sortedBindings.isEmpty())
         destroy();
 
+    QRHI_RES_RHI(QRhiD3D11);
+    if (!rhiD->sanityCheckShaderResourceBindings(this))
+        return false;
+
     std::copy(m_bindings.cbegin(), m_bindings.cend(), std::back_inserter(sortedBindings));
     std::sort(sortedBindings.begin(), sortedBindings.end(),
               [](const QRhiShaderResourceBinding &a, const QRhiShaderResourceBinding &b)
diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp
index f8a63d482f..380baf91b1 100644
--- a/src/gui/rhi/qrhigles2.cpp
+++ b/src/gui/rhi/qrhigles2.cpp
@@ -4192,6 +4192,10 @@ void QGles2ShaderResourceBindings::destroy()
 
 bool QGles2ShaderResourceBindings::create()
 {
+    QRHI_RES_RHI(QRhiGles2);
+    if (!rhiD->sanityCheckShaderResourceBindings(this))
+        return false;
+
     generation += 1;
     return true;
 }
diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm
index 7d2b1982f0..fbb2003fb2 100644
--- a/src/gui/rhi/qrhimetal.mm
+++ b/src/gui/rhi/qrhimetal.mm
@@ -2993,6 +2993,10 @@ bool QMetalShaderResourceBindings::create()
     if (!sortedBindings.isEmpty())
         destroy();
 
+    QRHI_RES_RHI(QRhiMetal);
+    if (!rhiD->sanityCheckShaderResourceBindings(this))
+        return false;
+
     std::copy(m_bindings.cbegin(), m_bindings.cend(), std::back_inserter(sortedBindings));
     std::sort(sortedBindings.begin(), sortedBindings.end(),
               [](const QRhiShaderResourceBinding &a, const QRhiShaderResourceBinding &b)
diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp
index 28463398bb..b172a8b16b 100644
--- a/src/gui/rhi/qrhivulkan.cpp
+++ b/src/gui/rhi/qrhivulkan.cpp
@@ -6155,6 +6155,10 @@ bool QVkShaderResourceBindings::create()
     if (layout)
         destroy();
 
+    QRHI_RES_RHI(QRhiVulkan);
+    if (!rhiD->sanityCheckShaderResourceBindings(this))
+        return false;
+
     for (int i = 0; i < QVK_FRAMES_IN_FLIGHT; ++i)
         descSets[i] = VK_NULL_HANDLE;
 
@@ -6187,7 +6191,6 @@ bool QVkShaderResourceBindings::create()
     layoutInfo.bindingCount = uint32_t(vkbindings.count());
     layoutInfo.pBindings = vkbindings.constData();
 
-    QRHI_RES_RHI(QRhiVulkan);
     VkResult err = rhiD->df->vkCreateDescriptorSetLayout(rhiD->dev, &layoutInfo, nullptr, &layout);
     if (err != VK_SUCCESS) {
         qWarning("Failed to create descriptor set layout: %d", err);