rhi: Reset resource state tracking tables upon layout-compatible updates

...just like create() would do when requesting a full rebuild.

Not relevant for the OpenGL backend, while D3D and Metal may get away
without doing this, but the Vulkan backend gives visible rendering
errors in Qt Quick once updateResources() is taken into use and a
scene manages to do the "right" amount and types of changes.

The most common source is a changing uniform buffer offset. Consider
how the Qt Quick scenegraph merges uniform data into a single buffer
in unmerged batches (i.e. when a set of geometry nodes cannot be
batched together in a single draw all). While each node gets its own
draw call, each associated srb refers to the same uniform buffer at
binding point 0, just with a different offset.

Without the layout-compatible-update optimization (that is
QRhiShaderResourceBindings::updateResources()) this is not something
that needs extra care: once an srb is built or rebuilt (by calling
create()), the offset, just like the resource itself is fixed and
cannot change. And once create() is called, it conveniently
invalidates all related data structures, leading to the subsequent
setShaderResources() rewrite descriptors (incl. the resource, the
offset, etc.) with Vulkan or do whatever is relevant with other
backends.

updateResources() only does the minimum amount of changes because we
know that the binding list layout has not changed. It turns out this
was a bit too minimal, because certain state tracking data structures
still need resetting, just as if we called create().

The problem is becoming apparent with non-layout data such as the
uniform buffer offset, storage buffer offset, or the storage image mip
level. It however works as expected when changing the resource itself.
E.g. if a binding point now references a QRhiBuffer different than
before, then there is no visible problems, regardless of the buffer
offset being different or the same. Hence being difficult to discover,
until the aforementioned Qt Quick renderer case triggers it.

Why is this?

Changing the resource (buffer, texture, sampler) itself works due to
the guarantees given by the QRhi resource model. Consider the
following:

ubuf is a uniform buffer
ubuf->create();
srb->setBindings({ references ubuf });
srb->create();
// use the srb in some draw calls
// ...
// later, when preparing the next frame we decide we need new data with
// a different size even:
ubuf->setSize(new_size)
ubuf->create();
// fill in new data to ubuf
// use the srb in some draw calls

at this point "magic" happens: note how there is no rebuilding of the
srb itself (no create(), no nothing), yet it will correctly pick up
the now-different native buffer objects underneath ubuf.

This works because there is a certain degree of state tracking
happening that allows recognizing that a resource referenced from an
srb got rebuilt and now has different native objects (e.g. a VkBuffer)
underneath, which in turn needs (with Vulkan) rewriting the associated
descriptor with the new native resource.

Incidentally, this also makes updateResources() work just fine as long
as it replaces the QRhiBuffer/Texture/Sampler reference for the
binding point. However, with the example snippet above there is no way
to change the other associated data such as the buffer offset. (that
would need rebuilding the srb with create(), and that resets all
related state tracking structures)

So once we encounter an updateResources() where the same QRhiBuffer is
now used with an offset different from before, that is not recognized
by setShaderResources() and (with Vulkan) it will not rewrite the
descriptor with the new offset. (unless the changes for another
resource in the binding list trigger it; the granularity here is quite
coarse, i.e. we either rewrite (with Vulkan) all descriptors or none
at all; this makes the problem even less apparent because now
rendering errors occur only when Qt Quick ends with an update where
only the uniform buffer offset, but nothing else changed)

Change-Id: I82ee43aa358947288135ff72ec213e091342e9cb
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
(cherry picked from commit 19febad9f2f9a9afe0128f7568f3cc41d759db1c)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

3 files changed, 22 insertions, 0 deletions

diff --git a/src/gui/rhi/qrhid3d11.cpp b/src/gui/rhi/qrhid3d11.cpp
index b2bc631569..71faece6c1 100644
--- a/src/gui/rhi/qrhid3d11.cpp
+++ b/src/gui/rhi/qrhid3d11.cpp
@@ -3634,6 +3634,10 @@ void QD3D11ShaderResourceBindings::updateResources(UpdateFlags flags)
         });
     }
 
+    Q_ASSERT(boundResourceData.count() == sortedBindings.count());
+    for (BoundResourceData &bd : boundResourceData)
+        memset(&bd, 0, sizeof(BoundResourceData));
+
     generation += 1;
 }
 
diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm
index 254f09ae08..e169550ee4 100644
--- a/src/gui/rhi/qrhimetal.mm
+++ b/src/gui/rhi/qrhimetal.mm
@@ -3191,6 +3191,9 @@ void QMetalShaderResourceBindings::updateResources(UpdateFlags flags)
         });
     }
 
+    for (BoundResourceData &bd : boundResourceData)
+        memset(&bd, 0, sizeof(BoundResourceData));
+
     generation += 1;
 }
 
diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp
index 43619d4299..45fb106a10 100644
--- a/src/gui/rhi/qrhivulkan.cpp
+++ b/src/gui/rhi/qrhivulkan.cpp
@@ -6637,6 +6637,21 @@ void QVkShaderResourceBindings::updateResources(UpdateFlags flags)
         });
     }
 
+    // Reset the state tracking table too - it can deal with assigning a
+    // different QRhiBuffer/Texture/Sampler for a binding point, but it cannot
+    // detect changes in the associated data, such as the buffer offset. And
+    // just like after a create(), a call to updateResources() may lead to now
+    // specifying a different offset for the same QRhiBuffer for a given binding
+    // point. The same applies to other type of associated data that is not part
+    // of the layout, such as the mip level for a StorageImage. Instead of
+    // complicating the checks in setShaderResources(), reset the table here
+    // just like we do in create().
+    for (int i = 0; i < QVK_FRAMES_IN_FLIGHT; ++i) {
+        Q_ASSERT(boundResourceData[i].count() == sortedBindings.count());
+        for (BoundResourceData &bd : boundResourceData[i])
+            memset(&bd, 0, sizeof(BoundResourceData));
+    }
+
     generation += 1;
 }