From a71baef5d3728787bc02ccf6b6f277bbaa53dbc0 Mon Sep 17 00:00:00 2001
From: Mathias Agopian <mathias@google.com>
Date: Wed, 17 May 2023 22:41:01 -0700
Subject: [PATCH] improve internal documentation of per-view sampler interface
 blocks

---
 libs/filamat/src/shaders/SibGenerator.cpp | 49 ++++++++++++++---------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/libs/filamat/src/shaders/SibGenerator.cpp b/libs/filamat/src/shaders/SibGenerator.cpp
index b780599296..18472a3ff3 100644
--- a/libs/filamat/src/shaders/SibGenerator.cpp
+++ b/libs/filamat/src/shaders/SibGenerator.cpp
@@ -28,7 +28,19 @@ SamplerInterfaceBlock const& SibGenerator::getPerViewSib(Variant variant) noexce
     using Format = SamplerInterfaceBlock::Format;
     using Precision = SamplerInterfaceBlock::Precision;
 
-    static SamplerInterfaceBlock sibPcf = SamplerInterfaceBlock::Builder()
+    // What is happening here is that depending on the variant, some samplers' type or format
+    // can change (e.g.: when VSM is used the shadowmap sampler is a regular float sampler),
+    // so we return a different SamplerInterfaceBlock based on the variant.
+    //
+    // The samplers' name and binding (i.e. ordering) must match in all SamplerInterfaceBlocks
+    // because this information is stored per-material and not per-shader.
+    //
+    // For the SSR (reflections) SamplerInterfaceBlock, only two samplers are ever used, for this
+    // reason we name them "unused*" to ensure we're not using them by mistake (type/format don't
+    // matter). This will not affect SamplerBindingMap because it always uses the default variant,
+    // and so the correct information will be stored in the material file.
+
+    static SamplerInterfaceBlock const sibPcf{ SamplerInterfaceBlock::Builder()
             .name("Light")
             .stageFlags(backend::ShaderStageFlags::FRAGMENT)
             .add(  {{ "shadowMap",   Type::SAMPLER_2D_ARRAY, Format::SHADOW, Precision::MEDIUM },
@@ -39,9 +51,9 @@ SamplerInterfaceBlock const& SibGenerator::getPerViewSib(Variant variant) noexce
                     { "ssr",         Type::SAMPLER_2D_ARRAY, Format::FLOAT,  Precision::MEDIUM },
                     { "structure",   Type::SAMPLER_2D,       Format::FLOAT,  Precision::HIGH   }}
             )
-            .build();
+            .build() };
 
-    static SamplerInterfaceBlock sibVsm = SamplerInterfaceBlock::Builder()
+    static SamplerInterfaceBlock const sibVsm{ SamplerInterfaceBlock::Builder()
             .name("Light")
             .stageFlags(backend::ShaderStageFlags::FRAGMENT)
             .add(  {{ "shadowMap",   Type::SAMPLER_2D_ARRAY, Format::FLOAT,  Precision::HIGH   },
@@ -52,26 +64,24 @@ SamplerInterfaceBlock const& SibGenerator::getPerViewSib(Variant variant) noexce
                     { "ssr",         Type::SAMPLER_2D_ARRAY, Format::FLOAT,  Precision::MEDIUM },
                     { "structure",   Type::SAMPLER_2D,       Format::FLOAT,  Precision::HIGH   }}
             )
-            .build();
+            .build() };
 
-    static SamplerInterfaceBlock sibSsr = SamplerInterfaceBlock::Builder()
+    static SamplerInterfaceBlock const sibSsr{ SamplerInterfaceBlock::Builder()
             .name("Light")
             .stageFlags(backend::ShaderStageFlags::FRAGMENT)
-            .add(  {{ "shadowMap",   Type::SAMPLER_2D_ARRAY, Format::SHADOW, Precision::MEDIUM },
-                    { "froxels",     Type::SAMPLER_2D,       Format::UINT,   Precision::MEDIUM },
-                    { "iblDFG",      Type::SAMPLER_2D,       Format::FLOAT,  Precision::MEDIUM },
-                    { "iblSpecular", Type::SAMPLER_CUBEMAP,  Format::FLOAT,  Precision::MEDIUM },
-                    { "ssao",        Type::SAMPLER_2D_ARRAY, Format::FLOAT,  Precision::MEDIUM },
+            .add(  {{ "unused0" },
+                    { "unused1" },
+                    { "unused2" },
+                    { "unused3" },
+                    { "unused4" },
                     { "ssr",         Type::SAMPLER_2D,       Format::FLOAT,  Precision::MEDIUM },
                     { "structure",   Type::SAMPLER_2D,       Format::FLOAT,  Precision::HIGH   }}
             )
-            .build();
+            .build() };
 
-    // SamplerBindingMap relies on the assumption that Sibs have the same names and offsets
-    // regardless of variant.
-    assert(sibPcf.getSize() == PerViewSib::SAMPLER_COUNT);
-    assert(sibVsm.getSize() == PerViewSib::SAMPLER_COUNT);
-    assert(sibSsr.getSize() == PerViewSib::SAMPLER_COUNT);
+    assert_invariant(sibPcf.getSize() == PerViewSib::SAMPLER_COUNT);
+    assert_invariant(sibVsm.getSize() == PerViewSib::SAMPLER_COUNT);
+    assert_invariant(sibSsr.getSize() == PerViewSib::SAMPLER_COUNT);
 
     if (Variant::isSSRVariant(variant)) {
         return sibSsr;
@@ -82,12 +92,12 @@ SamplerInterfaceBlock const& SibGenerator::getPerViewSib(Variant variant) noexce
     }
 }
 
-SamplerInterfaceBlock const& SibGenerator::getPerRenderPrimitiveMorphingSib(Variant variant) noexcept {
+SamplerInterfaceBlock const& SibGenerator::getPerRenderPrimitiveMorphingSib(Variant) noexcept {
     using Type = SamplerInterfaceBlock::Type;
     using Format = SamplerInterfaceBlock::Format;
     using Precision = SamplerInterfaceBlock::Precision;
 
-    static SamplerInterfaceBlock sib = SamplerInterfaceBlock::Builder()
+    static SamplerInterfaceBlock const sib = SamplerInterfaceBlock::Builder()
             .name("MorphTargetBuffer")
             .stageFlags(backend::ShaderStageFlags::VERTEX)
             .add({  { "positions", Type::SAMPLER_2D_ARRAY, Format::FLOAT, Precision::HIGH },
@@ -97,7 +107,8 @@ SamplerInterfaceBlock const& SibGenerator::getPerRenderPrimitiveMorphingSib(Vari
     return sib;
 }
 
-SamplerInterfaceBlock const* SibGenerator::getSib(SamplerBindingPoints bindingPoint, Variant variant) noexcept {
+SamplerInterfaceBlock const* SibGenerator::getSib(
+        SamplerBindingPoints bindingPoint, Variant variant) noexcept {
     switch (bindingPoint) {
         case SamplerBindingPoints::PER_VIEW:
             return &getPerViewSib(variant);