From d01b29fa01cf55c6ae0d71e06098eb8ceccc0a56 Mon Sep 17 00:00:00 2001
From: Powei Feng <powei@google.com>
Date: Fri, 1 Sep 2023 10:59:30 -0700
Subject: [PATCH] vulkan: properly set bool spec onst (#7125)

We wrote a bool directly into 4 bytes (as the first byte). This has two issues:
 - the other 3 bytes are not initialized
 - should be writing VK_TRUE/FALSE instead
---
 filament/backend/include/backend/Program.h    |  3 +-
 filament/backend/src/vulkan/VulkanHandles.cpp | 33 +++++++++++--------
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/filament/backend/include/backend/Program.h b/filament/backend/include/backend/Program.h
index 2a491959e9..72bcf01e42 100644
--- a/filament/backend/include/backend/Program.h
+++ b/filament/backend/include/backend/Program.h
@@ -104,8 +104,9 @@ public:
             Sampler const* samplers, size_t count) noexcept;
 
     struct SpecializationConstant {
+        using Type = std::variant<int32_t, float, bool>;
         uint32_t id;                                // id set in glsl
-        std::variant<int32_t, float, bool> value;   // value and type
+        Type value;                                 // value and type
     };
 
     Program& specializationConstants(
diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp
index e3d4579cd3..29a9f023ba 100644
--- a/filament/backend/src/vulkan/VulkanHandles.cpp
+++ b/filament/backend/src/vulkan/VulkanHandles.cpp
@@ -87,21 +87,26 @@ VulkanProgram::VulkanProgram(VkDevice device, const Program& builder) noexcept :
         };
 
         for (size_t i = 0; i < specializationConstants.size(); i++) {
-            const uint32_t offset = uint32_t(i) * 4;
-            std::visit([&](auto&& arg) {
-                using T = std::decay_t<decltype(arg)>;
-                pEntries[i] = {
-                        .constantID = specializationConstants[i].id,
-                        .offset = offset,
-                        // Turns out vulkan expects the size of bool to be 4 (verified through
-                        // validation layer). So all expected types are of 4 bytes.
-                        .size = 4,
-                };
-                T* const addr = (T*)((char*)pData + offset);
-                *addr = arg;
-            }, specializationConstants[i].value);
-        }
+            uint32_t const offset = uint32_t(i) * 4;
+            pEntries[i] = {
+                .constantID = specializationConstants[i].id,
+                .offset = offset,
+                // Note that bools are 4-bytes in Vulkan
+                // https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkBool32.html
+                .size = 4,
+            };
 
+            using SpecConstant = Program::SpecializationConstant::Type;
+            char const* addr = (char*)pData + offset;
+            SpecConstant const& arg = specializationConstants[i].value;
+            if (std::holds_alternative<bool>(arg)) {
+                *((VkBool32*)addr) = std::get<bool>(arg) ? VK_TRUE : VK_FALSE;
+            } else if (std::holds_alternative<float>(arg)) {
+                *((float*)addr) = std::get<float>(arg);
+            } else {
+                *((int32_t*)addr) = std::get<int32_t>(arg);
+            }
+        }
         bundle.specializationInfos = pInfo;
     }