Capture command callstacks for debugging

NEW_RELEASE_NOTES.md

@@ -13,4 +13,3 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md).
   dark fringes to appear (recompile materials)
 - Allow glTF materials with transmission/volume extensions to choose their alpha mode
   instead of forcing `MASKED`
-- Add support for 2 additional material variables [⚠️ **Recompile materials**]

android/filamat-android/src/main/java/com/google/android/filament/filamat/MaterialBuilder.java

@@ -102,9 +102,7 @@ public class MaterialBuilder {
         CUSTOM0,
         CUSTOM1,
         CUSTOM2,
-        CUSTOM3,
-        CUSTOM4,
-        CUSTOM5
+        CUSTOM3
     }
 
     public enum VertexAttribute {

docs/Materials.md.html

@@ -1299,7 +1299,7 @@ Type
 :    array of `string`
 
 Value
-:     Up to 6 strings, each must be a valid GLSL identifier.
+:     Up to 4 strings, each must be a valid GLSL identifier.
 
 Description
 :     Defines custom interpolants (or variables) that are output by the material's vertex shader.

filament/backend/include/private/backend/CommandStream.h

@@ -73,14 +73,32 @@ public:
         // a cost here (writing and reading the stack at each iteration), in the end it's
         // probably better to pay the cost at just one location.
         intptr_t next;
+        driver.mCurrentExecutingCommand = this;
         mExecute(driver, this, &next);
         return reinterpret_cast<CommandBase*>(reinterpret_cast<intptr_t>(this) + next);
     }
 
+    inline void captureCallstack() noexcept {
+        auto c = utils::CallStack::unwind(4);
+        size_t i = 0;
+        for (; i < c.getFrameCount() && i < 16; i++) {
+            mCallstack[i] = c[i];
+        }
+        for (; i < 16; i++) {
+            mCallstack[i] = 0;
+        }
+    }
+
+    void printCallstack() noexcept {
+        auto c = utils::CallStack(mCallstack);
+        utils::slog.d << c << utils::io::endl;
+    }
+
     inline ~CommandBase() noexcept = default;
 
 private:
     Execute mExecute;
+    std::array<intptr_t, 16> mCallstack = {0};
 };
 
 // ------------------------------------------------------------------------------------------------
@@ -218,6 +236,7 @@ public:
         using Cmd = COMMAND_TYPE(methodName);                                                   \
         void* const p = allocateCommand(CommandBase::align(sizeof(Cmd)));                       \
         new(p) Cmd(mDispatcher.methodName##_, APPLY(std::move, params));                        \
+        ((Cmd*)p)->captureCallstack();                                                          \
         DEBUG_COMMAND_END(methodName, false);                                                   \
     }
 
@@ -237,6 +256,7 @@ public:
         using Cmd = COMMAND_TYPE(methodName##R);                                                \
         void* const p = allocateCommand(CommandBase::align(sizeof(Cmd)));                       \
         new(p) Cmd(mDispatcher.methodName##_, RetType(result), APPLY(std::move, params));       \
+        ((Cmd*)p)->captureCallstack();                                                          \
         DEBUG_COMMAND_END(methodName, false);                                                   \
         return result;                                                                          \
     }

filament/backend/include/private/backend/Driver.h

@@ -53,6 +53,7 @@ template<typename T>
 class ConcreteDispatcher;
 class Dispatcher;
 class CommandStream;
+class CommandBase;
 
 class Driver {
 public:
@@ -83,6 +84,8 @@ public:
     virtual void debugCommandEnd(CommandStream* cmds,
             bool synchronous, const char* methodName) noexcept = 0;
 
+    CommandBase* mCurrentExecutingCommand = nullptr;
+
     /*
      * Asynchronous calls here only to provide a type to CommandStream. They must be non-virtual
      * so that calling the concrete implementation won't go through a vtable.

libs/filamat/include/filamat/MaterialBuilder.h

@@ -208,14 +208,12 @@ public:
     MaterialBuilder(MaterialBuilder&& rhs) noexcept = default;
     MaterialBuilder& operator=(MaterialBuilder&& rhs) noexcept = default;
 
-    static constexpr size_t MATERIAL_VARIABLES_COUNT = 6;
+    static constexpr size_t MATERIAL_VARIABLES_COUNT = 4;
     enum class Variable : uint8_t {
         CUSTOM0,
         CUSTOM1,
         CUSTOM2,
-        CUSTOM3,
-        CUSTOM4,
-        CUSTOM5
+        CUSTOM3
         // when adding more variables, make sure to update MATERIAL_VARIABLES_COUNT
     };
 

libs/filamat/src/MaterialBuilder.cpp

@@ -205,8 +205,6 @@ MaterialBuilder& MaterialBuilder::variable(Variable v, const char* name) noexcep
         case Variable::CUSTOM1:
         case Variable::CUSTOM2:
         case Variable::CUSTOM3:
-        case Variable::CUSTOM4:
-        case Variable::CUSTOM5:
             assert(size_t(v) < MATERIAL_VARIABLES_COUNT);
             mVariables[size_t(v)] = CString(name);
             break;

libs/utils/include/utils/CallStack.h

@@ -36,6 +36,13 @@ public:
      * @see CallStack::capture()
      */
     CallStack() = default;
+    template <unsigned long N>
+    explicit CallStack(const std::array<intptr_t, N>& symbols)
+        : m_frame_count(N) {
+        for (size_t i = 0; i < N; i++) {
+            m_stack[i] = symbols[i];
+        }
+    }
     CallStack(const CallStack&) = default;
     ~CallStack() = default;
 
@@ -114,12 +121,10 @@ private:
 
     static constexpr size_t NUM_FRAMES = 20;
 
-    struct StackFrameInfo {
-        intptr_t pc;
-    };
+    using StackFrameInfo = intptr_t;
 
     size_t m_frame_count = 0;
-    StackFrameInfo m_stack[NUM_FRAMES];
+    StackFrameInfo m_stack[NUM_FRAMES] = {0};
 };
 
 } // namespace utils

libs/utils/src/CallStack.cpp

@@ -70,7 +70,7 @@ intptr_t CallStack::operator[](size_t index) const {
 #endif
         std::abort();
     }
-    return m_stack[index].pc;
+    return m_stack[index];
 }
 
 size_t CallStack::getFrameCount() const noexcept {
@@ -91,7 +91,7 @@ void CallStack::update_gcc(size_t ignore) noexcept {
     size -= ignore;
 #endif
     for (ssize_t i = 0; i < size; i++) {
-        m_stack[i].pc = intptr_t(array[ignore + i]);
+        m_stack[i] = intptr_t(array[ignore + i]);
     }
     size--; // the last one seems to always be 0x0
 

shaders/src/varyings.glsl

@@ -2,37 +2,34 @@
 // Varyings
 //------------------------------------------------------------------------------
 
-// Varying locations [0, MaterialBuilder::MATERIAL_VARIABLES_COUNT) are reserved for user material
-// variables.
-
-LAYOUT_LOCATION(6) VARYING highp vec4 vertex_worldPosition;
+LAYOUT_LOCATION(4) VARYING highp vec4 vertex_worldPosition;
 
 #if defined(HAS_ATTRIBUTE_TANGENTS)
-LAYOUT_LOCATION(7) SHADING_INTERPOLATION VARYING mediump vec3 vertex_worldNormal;
+LAYOUT_LOCATION(5) SHADING_INTERPOLATION VARYING mediump vec3 vertex_worldNormal;
 #if defined(MATERIAL_NEEDS_TBN)
-LAYOUT_LOCATION(8) SHADING_INTERPOLATION VARYING mediump vec4 vertex_worldTangent;
+LAYOUT_LOCATION(6) SHADING_INTERPOLATION VARYING mediump vec4 vertex_worldTangent;
 #endif
 #endif
 
-LAYOUT_LOCATION(9) VARYING highp vec4 vertex_position;
+LAYOUT_LOCATION(7) VARYING highp vec4 vertex_position;
 
 #if defined(FILAMENT_HAS_FEATURE_INSTANCING)
-LAYOUT_LOCATION(10) flat VARYING highp int instance_index;
+LAYOUT_LOCATION(8) flat VARYING highp int instance_index;
 highp int logical_instance_index;
 #endif
 
 #if defined(HAS_ATTRIBUTE_COLOR)
-LAYOUT_LOCATION(11) VARYING mediump vec4 vertex_color;
+LAYOUT_LOCATION(9) VARYING mediump vec4 vertex_color;
 #endif
 
 #if defined(HAS_ATTRIBUTE_UV0) && !defined(HAS_ATTRIBUTE_UV1)
-LAYOUT_LOCATION(12) VARYING highp vec2 vertex_uv01;
+LAYOUT_LOCATION(10) VARYING highp vec2 vertex_uv01;
 #elif defined(HAS_ATTRIBUTE_UV1)
-LAYOUT_LOCATION(12) VARYING highp vec4 vertex_uv01;
+LAYOUT_LOCATION(10) VARYING highp vec4 vertex_uv01;
 #endif
 
 #if defined(VARIANT_HAS_SHADOWING) && defined(VARIANT_HAS_DIRECTIONAL_LIGHTING)
-LAYOUT_LOCATION(13) VARYING highp vec4 vertex_lightSpacePosition;
+LAYOUT_LOCATION(11) VARYING highp vec4 vertex_lightSpacePosition;
 #endif
 
 // Note that fragColor is an output and is not declared here; see main.fs and depth_main.fs

tools/matc/src/matc/ParametersProcessor.cpp

@@ -66,8 +66,6 @@ static MaterialBuilder::Variable intToVariable(size_t i) noexcept {
         case 1:  return MaterialBuilder::Variable::CUSTOM1;
         case 2:  return MaterialBuilder::Variable::CUSTOM2;
         case 3:  return MaterialBuilder::Variable::CUSTOM3;
-        case 4:  return MaterialBuilder::Variable::CUSTOM4;
-        case 5:  return MaterialBuilder::Variable::CUSTOM5;
         default: return MaterialBuilder::Variable::CUSTOM0;
     }
 }
@@ -609,9 +607,8 @@ static bool processVariables(MaterialBuilder& builder, const JsonishValue& value
     const JsonishArray* jsonArray = value.toJsonArray();
     const auto& elements = jsonArray->getElements();
 
-    if (elements.size() > MaterialBuilder::MATERIAL_VARIABLES_COUNT) {
-        std::cerr << "variables: Max array size is " << MaterialBuilder::MATERIAL_VARIABLES_COUNT
-                  << "." << std::endl;
+    if (elements.size() > 4) {
+        std::cerr << "variables: Max array size is 4." << std::endl;
         return false;
     }