addressed comments

filament/backend/include/backend/DriverEnums.h

@@ -31,6 +31,7 @@
 
 #include <array>        // FIXME: STL headers are not allowed in public headers
 #include <type_traits>  // FIXME: STL headers are not allowed in public headers
+#include <variant>      // FIXME: STL headers are not allowed in public headers
 
 #include <stddef.h>
 #include <stdint.h>
@@ -1222,19 +1223,7 @@ struct StencilState {
     uint8_t padding = 0;
 };
 
-struct PushConstant {
-    using Variant = std::variant<int, float, bool>;
-    char const* name = nullptr;
-    Variant value;
-};
-
-using PushConstantArray = std::array<PushConstant, MAX_PUSH_CONSTANT_COUNT>;
-
-struct PushConstantStruct {
-    char const* name = nullptr;
-    ShaderStage stage;
-    PushConstantArray constants;
-};
+using PushConstantVariant = std::variant<int32_t, float, bool>;
 
 static_assert(sizeof(StencilState::StencilOperations) == 5u,
         "StencilOperations size not what was intended");

filament/backend/include/backend/Program.h

@@ -62,9 +62,6 @@ class Program {
     using SamplerGroupInfo = std::array<SamplerGroupData, SAMPLER_BINDING_COUNT>;
     using ShaderBlob = utils::FixedCapacityVector<uint8_t>;
     using ShaderSource = std::array<ShaderBlob, SHADER_TYPE_COUNT>;
-    using PushConstantArray = filament::backend::PushConstantArray;
-    using PushConstantStruct = filament::backend::PushConstantStruct;
-    using PushConstantStructArray = std::array<PushConstantStruct, SHADER_TYPE_COUNT>;
 
     Program() noexcept;
 
@@ -120,7 +117,8 @@ class Program {
     Program& specializationConstants(
             utils::FixedCapacityVector<SpecializationConstant> specConstants) noexcept;
 
-    Program& pushConstants(PushConstantStruct const& pushConstants) noexcept;
+    Program& pushConstants(ShaderStage stage,
+            utils::FixedCapacityVector<char const*> constants) noexcept;
 
     Program& cacheId(uint64_t cacheId) noexcept;
 
@@ -153,9 +151,14 @@ class Program {
         return mSpecializationConstants;
     }
 
-    PushConstantStructArray const& getPushConstants() const noexcept { return mPushConstants; }
+    utils::FixedCapacityVector<char const*> const& getPushConstants(
+            ShaderStage stage) const noexcept {
+        return mPushConstants[static_cast<uint8_t>(stage)];
+    }
 
-    PushConstantStructArray& getPushConstants() noexcept { return mPushConstants; }
+    utils::FixedCapacityVector<char const*>& getPushConstants(ShaderStage stage) noexcept {
+        return mPushConstants[static_cast<uint8_t>(stage)];
+    }
 
     uint64_t getCacheId() const noexcept { return mCacheId; }
 
@@ -174,7 +177,7 @@ class Program {
     uint64_t mCacheId{};
     utils::Invocable<utils::io::ostream&(utils::io::ostream& out)> mLogger;
     utils::FixedCapacityVector<SpecializationConstant> mSpecializationConstants;
-    PushConstantStructArray mPushConstants;
+    std::array<utils::FixedCapacityVector<char const*>, SHADER_TYPE_COUNT> mPushConstants;
     utils::FixedCapacityVector<std::pair<utils::CString, uint8_t>> mAttributes;
     std::array<UniformInfo, Program::UNIFORM_BINDING_COUNT> mBindingUniformInfo;
     CompilerPriorityQueue mPriorityQueue = CompilerPriorityQueue::HIGH;

filament/backend/include/private/backend/DriverAPI.inc

@@ -434,9 +434,10 @@ DECL_DRIVER_API_N(bindSamplers,
         uint32_t, index,
         backend::SamplerGroupHandle, sbh)
 
-DECL_DRIVER_API_N(writePushConstants,
+DECL_DRIVER_API_N(setPushConstant,
         backend::ShaderStage, stage,
-        backend::PushConstantArray, pushConstants)
+        uint8_t, index,
+        backend::PushConstantVariant, value)
 
 DECL_DRIVER_API_N(insertEventMarker,
         const char*, string,

filament/backend/src/Program.cpp

@@ -91,9 +91,9 @@ Program& Program::specializationConstants(
     return *this;
 }
 
-
-Program& Program::pushConstants(PushConstantStruct const& pushConstants) noexcept {
-    mPushConstants[static_cast<int>(pushConstants.stage)] = pushConstants;
+Program& Program::pushConstants(ShaderStage stage,
+        utils::FixedCapacityVector<char const*> constants) noexcept {
+    mPushConstants[static_cast<uint8_t>(stage)] = constants;
     return *this;
 }
 

filament/backend/src/metal/MetalDriver.mm

@@ -1241,8 +1241,8 @@
     mContext->samplerBindings[index] = sb;
 }
 
-void MetalDriver::writePushConstants(backend::ShaderStage stage,
-    backend::PushConstantArray pushConstants) {}
+void MetalDriver::setPushConstant(backend::ShaderStage stage, uint8_t index,
+        backend::PushConstantVariant value) {}
 
 void MetalDriver::insertEventMarker(const char* string, uint32_t len) {
 

filament/backend/src/noop/NoopDriver.cpp

@@ -312,8 +312,9 @@ void NoopDriver::unbindBuffer(BufferObjectBinding bindingType, uint32_t index) {
 void NoopDriver::bindSamplers(uint32_t index, Handle<HwSamplerGroup> sbh) {
 }
 
-void NoopDriver::writePushConstants(backend::ShaderStage stage,
-        backend::PushConstantArray pushConstants) {}
+void NoopDriver::setPushConstant(backend::ShaderStage stage, uint8_t index,
+        backend::PushConstantVariant value) {
+}
 
 void NoopDriver::insertEventMarker(char const* string, uint32_t len) {
 }

filament/backend/src/opengl/OpenGLDriver.cpp

@@ -285,8 +285,9 @@ void OpenGLDriver::bindSampler(GLuint unit, GLuint sampler) noexcept {
     mContext.bindSampler(unit, sampler);
 }
 
-void OpenGLDriver::writePushConstants(backend::ShaderStage stage,
-        backend::PushConstantArray pushConstants) {}
+void OpenGLDriver::setPushConstant(backend::ShaderStage stage, uint8_t index,
+        backend::PushConstantVariant value) {
+}
 
 void OpenGLDriver::bindTexture(GLuint unit, GLTexture const* t) noexcept {
     assert_invariant(t != nullptr);

filament/backend/src/vulkan/VulkanDriver.cpp

@@ -1572,12 +1572,12 @@ void VulkanDriver::bindSamplers(uint32_t index, Handle<HwSamplerGroup> sbh) {
     mSamplerBindings[index] = hwsb;
 }
 
-void VulkanDriver::writePushConstants(backend::ShaderStage stage,
-        backend::PushConstantArray pushConstants) {
+void VulkanDriver::setPushConstant(backend::ShaderStage stage, uint8_t index,
+        backend::PushConstantVariant value) {
     assert_invariant(mBoundPipeline.program && "Expect a program when writing to push constants");
     VulkanCommands* commands = &mCommands;
-    mBoundPipeline.program->writePushConstant(commands, mBoundPipeline.pipelineLayout, stage,
-            pushConstants);
+    mBoundPipeline.program->writePushConstant(commands, mBoundPipeline.pipelineLayout, stage, index,
+            value);
 }
 
 void VulkanDriver::insertEventMarker(char const* string, uint32_t len) {

filament/backend/src/vulkan/VulkanHandles.cpp

@@ -126,77 +126,55 @@ inline VkShaderStageFlags getVkStage(backend::ShaderStage stage) {
 } // anonymous namespace
 
 
-VulkanDescriptorSetLayout::VulkanDescriptorSetLayout(VkDevice device, VkDescriptorSetLayoutCreateInfo const& info,
-        Bitmask const& bitmask)
+VulkanDescriptorSetLayout::VulkanDescriptorSetLayout(VkDevice device,
+        VkDescriptorSetLayoutCreateInfo const& info, Bitmask const& bitmask)
     : VulkanResource(VulkanResourceType::DESCRIPTOR_SET_LAYOUT),
       mDevice(device),
       vklayout(createDescriptorSetLayout(device, info)),
       bitmask(bitmask),
       bindings(getBindings(bitmask)),
-      count(Count::fromLayoutBitmask(bitmask)) {
-}
+      count(Count::fromLayoutBitmask(bitmask)) {}
 
 VulkanDescriptorSetLayout::~VulkanDescriptorSetLayout() {
     vkDestroyDescriptorSetLayout(mDevice, vklayout, VKALLOC);
 }
 
-PushConstantDescription::PushConstantDescription(
-        Program::PushConstantStructArray const& pushConstants) {
+PushConstantDescription::PushConstantDescription(backend::Program const& program) noexcept {
     mRangeCount = 0;
-    for (uint8_t i = 0; i < pushConstants.size(); ++i) {
-        if (!pushConstants[i].name) {
+    for (auto stage : { ShaderStage::VERTEX, ShaderStage::FRAGMENT, ShaderStage::COMPUTE }) {
+        auto const& names = program.getPushConstants(stage);
+        if (names.empty()) {
             continue;
         }
-        auto const& constants = pushConstants[i].constants;
-        NameOffsetMap& nameOffsets = mOffsets[i];
-        for (size_t j = 0; j < constants.size(); j++) {
-            if (!constants[j].name) {
-                break;
-            }
-            nameOffsets[constants[j].name] = j;
-        }
-
         mRanges[mRangeCount++] = {
-            .stageFlags = getVkStage(static_cast<backend::ShaderStage>(i)),
+            .stageFlags = getVkStage(stage),
             .offset = 0,
-            .size = (uint32_t) nameOffsets.size() * 4,
+            .size = (uint32_t) names.size() * 4,
         };
     }
 }
 
 void PushConstantDescription::write(VulkanCommands* commands, VkPipelineLayout layout,
-        backend::ShaderStage stage, backend::PushConstantArray const& constants) {
+        backend::ShaderStage stage, uint8_t index, backend::PushConstantVariant const& value) {
     VulkanCommandBuffer* cmdbuf = &(commands->get());
-    NameOffsetMap const& nameOffsets = mOffsets[static_cast<int>(stage)];
-    for (auto const& constant: constants) {
-        if (!constant.name) {
-            break;
-        }
-        auto res = nameOffsets.find(constant.name);
-        assert_invariant(res != nameOffsets.end());
-
-        uint32_t const offset = res->second;
-        auto const& value = constant.value;
-        uint32_t binaryValue = 0;
-        if (std::holds_alternative<bool>(value)) {
-            bool const bval = std::get<bool>(value);
-            binaryValue = reinterpret_cast<uint32_t const>(bval ? VK_TRUE : VK_FALSE);
-        } else if (std::holds_alternative<float>(value)) {
-            float const fval = std::get<float>(value);
-            binaryValue = *reinterpret_cast<uint32_t const*>(&fval);
-        } else {
-            int const ival = std::get<int>(value);
-            binaryValue = *reinterpret_cast<uint32_t const*>(&ival);
-        }
-        vkCmdPushConstants(cmdbuf->buffer(), layout, getVkStage(stage), offset * 4, 4,
-                &binaryValue);
+    uint32_t binaryValue = 0;
+    if (std::holds_alternative<bool>(value)) {
+        bool const bval = std::get<bool>(value);
+        binaryValue = reinterpret_cast<uint32_t const>(bval ? VK_TRUE : VK_FALSE);
+    } else if (std::holds_alternative<float>(value)) {
+        float const fval = std::get<float>(value);
+        binaryValue = *reinterpret_cast<uint32_t const*>(&fval);
+    } else {
+        int const ival = std::get<int>(value);
+        binaryValue = *reinterpret_cast<uint32_t const*>(&ival);
     }
+    vkCmdPushConstants(cmdbuf->buffer(), layout, getVkStage(stage), index * 4, 4, &binaryValue);
 }
 
 VulkanProgram::VulkanProgram(VkDevice device, Program const& builder) noexcept
     : HwProgram(builder.getName()),
       VulkanResource(VulkanResourceType::PROGRAM),
-      mInfo(new PipelineInfo(builder.getPushConstants())),
+      mInfo(new PipelineInfo(builder)),
       mDevice(device) {
 
     constexpr uint8_t UBO_MODULE_OFFSET = (sizeof(UniformBufferBitmask) * 8) / MAX_SHADER_MODULES;

filament/backend/src/vulkan/VulkanHandles.h

@@ -180,23 +180,20 @@ struct VulkanDescriptorSet : public VulkanResource {
 using VulkanDescriptorSetList = std::array<Handle<VulkanDescriptorSet>,
         VulkanDescriptorSetLayout::UNIQUE_DESCRIPTOR_SET_COUNT>;
 
+using PushConstantNameArray = utils::FixedCapacityVector<char const*>;
+using PushConstantNameByStage = std::array<PushConstantNameArray, Program::SHADER_TYPE_COUNT>;
+
 struct PushConstantDescription {
-    PushConstantDescription(Program::PushConstantStructArray const& pushConstants);
+    explicit PushConstantDescription(backend::Program const& program) noexcept;
 
-    VkPushConstantRange const* getVkRanges() const {
-        return mRanges;
-    }
+    VkPushConstantRange const* getVkRanges() const noexcept { return mRanges; }
 
-    uint32_t getVkRangeCount() const {
-        return mRangeCount;
-    }
+    uint32_t getVkRangeCount() const noexcept { return mRangeCount; }
 
     void write(VulkanCommands* commands, VkPipelineLayout layout, backend::ShaderStage stage,
-            backend::PushConstantArray const& constants);
+            uint8_t index, backend::PushConstantVariant const& value);
 
 private:
-    using NameOffsetMap = std::unordered_map<std::string_view, uint8_t>;
-    std::array<NameOffsetMap, Program::SHADER_TYPE_COUNT> mOffsets;
     VkPushConstantRange mRanges[Program::SHADER_TYPE_COUNT];
     uint32_t mRangeCount;
 };
@@ -242,8 +239,8 @@ struct VulkanProgram : public HwProgram, VulkanResource {
     }
 
     inline void writePushConstant(VulkanCommands* commands, VkPipelineLayout layout,
-            backend::ShaderStage stage, backend::PushConstantArray const& pushConstants) {
-        mInfo->pushConstantDescription.write(commands, layout, stage, pushConstants);
+            backend::ShaderStage stage, uint8_t index, backend::PushConstantVariant const& value) {
+        mInfo->pushConstantDescription.write(commands, layout, stage, index, value);
     }
 
 #if FVK_ENABLED_DEBUG_SAMPLER_NAME
@@ -258,9 +255,9 @@ struct VulkanProgram : public HwProgram, VulkanResource {
 
 private:
     struct PipelineInfo {
-        PipelineInfo(Program::PushConstantStructArray const& pushConstants)
+        explicit PipelineInfo(backend::Program const& program) noexcept
             : bindingToSamplerIndex(MAX_SAMPLER_COUNT, 0xffff),
-              pushConstantDescription(pushConstants)
+              pushConstantDescription(program)
 #if FVK_ENABLED_DEBUG_SAMPLER_NAME
             , bindingToName(MAX_SAMPLER_COUNT, "")
 #endif

filament/backend/src/vulkan/caching/VulkanPipelineLayoutCache.h

@@ -43,10 +43,9 @@ class VulkanPipelineLayoutCache {
     };
 
     struct PipelineLayoutKey {
-        using DescriptorSetLayoutArray = std::array<VkDescriptorSetLayout, VulkanDescriptorSetLayout::UNIQUE_DESCRIPTOR_SET_COUNT>;
+        using DescriptorSetLayoutArray = std::array<VkDescriptorSetLayout,
+                VulkanDescriptorSetLayout::UNIQUE_DESCRIPTOR_SET_COUNT>;
         DescriptorSetLayoutArray descSetLayouts = {};
-
-        // We use the shader handles as a shorthand for the push constant ranges.
         PushConstantKey pushConstant[Program::SHADER_TYPE_COUNT] = {};
     };
 

filament/src/details/Material.cpp

@@ -576,8 +576,8 @@ Program FMaterial::getProgramWithVariants(
 
     program.specializationConstants(mSpecializationConstants);
 
-    program.pushConstants(mFragmentPushConstants);
-    program.pushConstants(mVertexPushConstants);
+    program.pushConstants(ShaderStage::VERTEX, mPushConstants[(uint8_t) ShaderStage::VERTEX]);
+    program.pushConstants(ShaderStage::FRAGMENT, mPushConstants[(uint8_t) ShaderStage::FRAGMENT]);
 
     program.cacheId(utils::hash::combine(size_t(mCacheId), variant.key));
 
@@ -933,7 +933,7 @@ void FMaterial::processSpecializationConstants(FEngine& engine, Material::Builde
 
 void FMaterial::processPushConstants(FEngine& engine, Material::Builder const& builder) {
     // TODO: for testing and illustrating push constants. To be removed.
-    // mVertexPushConstants = SKINNING_PUSH_CONSTANTS;
+    // mPushConstants[(uint8_t) ShaderStage::VERTEX] = SKINNING_VERTEX_PUSH_CONSTANTS.names;
 }
 
 void FMaterial::processDepthVariants(FEngine& engine, MaterialParser const* const parser) {

filament/src/details/Material.h

@@ -323,17 +323,8 @@ class FMaterial : public Material {
     utils::FixedCapacityVector<backend::Program::SpecializationConstant> mSpecializationConstants;
 
     // current push constants for the HwProgram
-    backend::Program::PushConstantStruct mFragmentPushConstants = {
-            .name = nullptr,
-            .stage = backend::ShaderStage::FRAGMENT,
-            .constants = {},
-    };
-
-    backend::Program::PushConstantStruct mVertexPushConstants = {
-            .name = nullptr,
-            .stage = backend::ShaderStage::VERTEX,
-            .constants = {},
-    };
+    std::array<utils::FixedCapacityVector<char const*>, backend::Program::SHADER_TYPE_COUNT>
+            mPushConstants;
 
 #if FILAMENT_ENABLE_MATDBG
     matdbg::MaterialKey mDebuggerId;