From da8fa2d08e3b21d81255f982a9e26ba127585be4 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Wed, 8 Feb 2023 16:23:42 +0100
Subject: [PATCH] QRhi: fix potential ODR issue (operators for foreign types)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We mustn't define operators or hash functions for types we don't own,
esp. not in a header file, because when someone else gets the same
idea, we have an ODR violation, unless they get it token-for-token the
same as our implementation.

One option would be to replace the QHash with an STL container. Those
can take per-container Hash and Equal function objects, so we wouldn't
need to declare the global ones.

But let's use a wrapper around the type on which we define the missing
operators instead.

As a drive-by, rename the arguments to the idiomatic lhs/rhs/key, from
a/b/s.

Change-Id: Ibbc2083bcd7423c5d443a0ca1b820cbecb241865
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
---
 src/gui/rhi/qrhid3d12.cpp   |  4 +--
 src/gui/rhi/qrhid3d12_p_p.h | 53 ++++++++++++++++++++++++++++---------
 2 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/src/gui/rhi/qrhid3d12.cpp b/src/gui/rhi/qrhid3d12.cpp
index 3e8596bf99..5630aa852b 100644
--- a/src/gui/rhi/qrhid3d12.cpp
+++ b/src/gui/rhi/qrhid3d12.cpp
@@ -2362,14 +2362,14 @@ void QD3D12SamplerManager::destroy()
 
 QD3D12Descriptor QD3D12SamplerManager::getShaderVisibleDescriptor(const D3D12_SAMPLER_DESC &desc)
 {
-    auto it = gpuMap.constFind(desc);
+    auto it = gpuMap.constFind({desc});
     if (it != gpuMap.cend())
         return *it;
 
     QD3D12Descriptor descriptor = shaderVisibleSamplerHeap.heap.get(1);
     if (descriptor.isValid()) {
         device->CreateSampler(&desc, descriptor.cpuHandle);
-        gpuMap.insert(desc, descriptor);
+        gpuMap.insert({desc}, descriptor);
     } else {
         qWarning("Out of shader-visible SAMPLER descriptor heap space,"
                  " this should not happen, maximum number of unique samplers is %u",
diff --git a/src/gui/rhi/qrhid3d12_p_p.h b/src/gui/rhi/qrhid3d12_p_p.h
index 4f1717e2bd..1048e66142 100644
--- a/src/gui/rhi/qrhid3d12_p_p.h
+++ b/src/gui/rhi/qrhid3d12_p_p.h
@@ -20,6 +20,7 @@
 #include "qshaderdescription_p.h"
 #include <QWindow>
 #include <QBitArray>
+
 #include <optional>
 #include <array>
 
@@ -30,11 +31,6 @@
 
 #include "D3D12MemAlloc.h"
 
-inline size_t qHash(const D3D12_SAMPLER_DESC &s, size_t seed = 0) noexcept
-{
-    return QT_PREPEND_NAMESPACE(qHashBits)(&s, sizeof(s), seed);
-}
-
 QT_BEGIN_NAMESPACE
 
 static const int QD3D12_FRAMES_IN_FLIGHT = 2;
@@ -457,15 +453,46 @@ struct QD3D12ShaderVisibleDescriptorHeap
     QD3D12DescriptorHeap perFrameHeapSlice[QD3D12_FRAMES_IN_FLIGHT];
 };
 
-inline bool operator==(const D3D12_SAMPLER_DESC &a, const D3D12_SAMPLER_DESC &b) noexcept
+// wrap foreign struct so we can legally supply equality operators and qHash:
+struct Q_D3D12_SAMPLER_DESC
 {
-    return !memcmp(&a, &b, sizeof(D3D12_SAMPLER_DESC));
-}
+    D3D12_SAMPLER_DESC desc;
 
-inline bool operator!=(const D3D12_SAMPLER_DESC &a, const D3D12_SAMPLER_DESC &b) noexcept
-{
-    return !(a == b);
-}
+    friend bool operator==(const Q_D3D12_SAMPLER_DESC &lhs, const Q_D3D12_SAMPLER_DESC &rhs) noexcept
+    {
+        return lhs.desc.Filter == rhs.desc.Filter
+                && lhs.desc.AddressU == rhs.desc.AddressU
+                && lhs.desc.AddressV == rhs.desc.AddressV
+                && lhs.desc.AddressW == rhs.desc.AddressW
+                && lhs.desc.MipLODBias == rhs.desc.MipLODBias
+                && lhs.desc.MaxAnisotropy == rhs.desc.MaxAnisotropy
+                && lhs.desc.ComparisonFunc == rhs.desc.ComparisonFunc
+                // BorderColor is never used, skip it
+                && lhs.desc.MinLOD == rhs.desc.MinLOD
+                && lhs.desc.MaxLOD == rhs.desc.MaxLOD;
+    }
+
+    friend bool operator!=(const Q_D3D12_SAMPLER_DESC &lhs, const Q_D3D12_SAMPLER_DESC &rhs) noexcept
+    {
+        return !(lhs == rhs);
+    }
+
+    friend size_t qHash(const Q_D3D12_SAMPLER_DESC &key, size_t seed = 0) noexcept
+    {
+        QtPrivate::QHashCombine hash;
+        seed = hash(seed, key.desc.Filter);
+        seed = hash(seed, key.desc.AddressU);
+        seed = hash(seed, key.desc.AddressV);
+        seed = hash(seed, key.desc.AddressW);
+        seed = hash(seed, key.desc.MipLODBias);
+        seed = hash(seed, key.desc.MaxAnisotropy);
+        seed = hash(seed, key.desc.ComparisonFunc);
+        // BorderColor is never used, skip it
+        seed = hash(seed, key.desc.MinLOD);
+        seed = hash(seed, key.desc.MaxLOD);
+        return seed;
+    }
+};
 
 struct QD3D12SamplerManager
 {
@@ -478,7 +505,7 @@ struct QD3D12SamplerManager
 
     ID3D12Device *device = nullptr;
     QD3D12ShaderVisibleDescriptorHeap shaderVisibleSamplerHeap;
-    QHash<D3D12_SAMPLER_DESC, QD3D12Descriptor> gpuMap;
+    QHash<Q_D3D12_SAMPLER_DESC, QD3D12Descriptor> gpuMap;
 };
 
 enum QD3D12Stage { VS = 0, HS, DS, GS, PS, CS };