Refactor shader uniform handling to use smart pointers

UntitledOutput · UntitledOutput · commit 07398b83274c · 2025-12-10T22:58:41.000-05:00
Replaces raw pointers with std::shared_ptr for GfxShaderValue and related uniform binding logic, improving memory management and safety. Updates GfxUniformList, GfxShaderBinding, and GfxMaterial to use smart pointers, and centralizes uniform application logic in GfxMaterial::applyUniform. Also adds bone world matrix calculation and skin initialization, and fixes GLM quaternion initialization for model nodes and bones.
diff --git a/include/borealis/gfx/engine.hpp b/include/borealis/gfx/engine.hpp
@@ -58,7 +58,7 @@ namespace brl
         void update();
         bool isRunning() {return mainWindow->isOpen();}
         void insertCall(GfxMaterial* material, GfxAttribBuffer* buffer, const glm::mat4& transform,
-                        std::vector<GfxShaderBinding>& uniqueOverrides, int instancingIdx = 0);
+                        GfxUniformList& uniqueOverrides, int instancingIdx = 0);
         int getFrameCount();
 
         float getAspectRatio();
diff --git a/include/borealis/gfx/model.hpp b/include/borealis/gfx/model.hpp
@@ -76,7 +76,7 @@ namespace brl
     struct GfxModelNode {
         std::string name;
         glm::vec3 position = glm::vec3(0);
-        glm::quat rotation = glm::quat(); 
+        glm::quat rotation = glm::identity<glm::quat>();
         glm::vec3 scale = glm::vec3(1);
 
         int mesh = -1;
@@ -115,7 +115,7 @@ namespace brl
 
         static GfxModel* loadModel(std::string path);
 
-        EcsEntity* createEntity();
+        GfxModelEntity* createEntity();
 
     private:
         friend GfxEngine;
@@ -152,9 +152,10 @@ namespace brl
     {
         std::string name;
         glm::vec3 position = glm::vec3(0);
-        glm::quat rotation = glm::quat();
+        glm::quat rotation = glm::identity<glm::quat>();
         glm::vec3 scale = glm::vec3(1);
         glm::mat4 inverseBindMatrix = glm::mat4(1.0);
+        glm::mat4 worldMatrix = glm::mat4(1.0);
 
         int parent = -1;
 
@@ -171,6 +172,11 @@ namespace brl
     {
         std::string name;
         std::vector<GfxBone*> bones;
+
+    private:
+        friend GfxModel;
+        void initialize();
+        void _calcTransform(brl::GfxBone* bone, const glm::mat4& parentTransform, int index);
     };
 
     struct GfxSkeleton : EcsEntity
@@ -184,7 +190,7 @@ namespace brl
     private:
 
         std::vector<glm::mat4> jointMatrices;
-        void _calcTransform(brl::GfxBone* bone, glm::mat4 parentTransform, int index,
+        void _calcTransform(brl::GfxBone* bone, const glm::mat4& parentTransform, int index,
                                  bool parentChanged);
 
     };
diff --git a/include/borealis/gfx/shader.hpp b/include/borealis/gfx/shader.hpp
@@ -5,6 +5,9 @@
 #include <borealis/util/util.h>
 #include <glad/glad.h>
 
+#include <memory>
+#include <vector>
+
 namespace brl
 {
     struct GfxShaderProgram;
@@ -62,29 +65,32 @@ namespace brl
         glm::vec2 v2value;
         glm::vec3 v3value;
         glm::vec4 v4value;
-        std::shared_ptr<std::vector<glm::mat4>> m4value = nullptr;
+        std::shared_ptr<std::vector<glm::mat4>> m4value; // Shared ownership
         GfxTexture* txValue = nullptr;
+
+        ~GfxShaderValue();
     };
 
     struct GfxShaderBinding 
     {
         brl::GfxShaderUniform* uniform = nullptr;
-        brl::GfxShaderValue* value = nullptr;
+        std::shared_ptr<GfxShaderValue> value = nullptr;
 
         GfxShaderBinding() = default;
-        GfxShaderBinding(brl::GfxShaderUniform* u, brl::GfxShaderValue* v) : uniform(u), value(v) {}
+        GfxShaderBinding(brl::GfxShaderUniform* u, std::shared_ptr<GfxShaderValue> v) : uniform(u), value(v) {}
 
         ~GfxShaderBinding();
+
+    private:
+        bool real = true;
     };
 
-    struct GfxUniformList : public std::vector<GfxShaderBinding>
+    struct GfxUniformList :  std::vector<GfxShaderBinding>
     {
         bool contains(GfxShaderUniform* uniform);
 
-        
-
 // Non-const version for read/write access
-        GfxShaderValue*& operator[](GfxShaderUniform* uniform)
+        std::shared_ptr<GfxShaderValue>& operator[](GfxShaderUniform* uniform)
         {
             for (auto& element1 : *this) // Non-const loop for non-const method
             {
@@ -97,7 +103,7 @@ namespace brl
         }
 
         // Const version for read-only access
-        GfxShaderValue* const& operator[](GfxShaderUniform* uniform) const
+        std::shared_ptr<GfxShaderValue> const& operator[](GfxShaderUniform* uniform) const
         {
             for (const auto& element1 : *this)
             {
@@ -164,6 +170,8 @@ namespace brl
 
     };
 
+#define CONVERT_UNIFORM_MAT4(m) std::make_shared<std::vector<glm::mat4>>(m)
+
     struct GfxMaterial
     {
 
@@ -182,9 +190,11 @@ namespace brl
             if (!shader->getUniform(name))
                 return;
 
-            GfxShaderValue* val = new GfxShaderValue();
+            auto val = std::make_shared<GfxShaderValue>();
             val->intValue = value;
-            setOverride({shader->getUniform(name), val});
+
+            auto override = GfxShaderBinding(shader->getUniform(name), val);
+            setOverride(std::move(override));
         }
 
         void setFloat(std::string name, float value)
@@ -194,9 +204,11 @@ namespace brl
             if (!shader->getUniform(name))
                 return;
 
-            GfxShaderValue* val = new GfxShaderValue();
+            auto val = std::make_shared<GfxShaderValue>();
             val->floatValue = value;
-            setOverride({shader->getUniform(name), val});
+
+                        auto override = GfxShaderBinding(shader->getUniform(name), val);
+            setOverride(std::move(override));
         }
 
         void setVec2(std::string name, glm::vec2 value)
@@ -206,9 +218,11 @@ namespace brl
             if (!shader->getUniform(name))
                 return;
 
-            GfxShaderValue* val = new GfxShaderValue();
+            auto val = std::make_shared<GfxShaderValue>();
             val->v2value = value;
-            setOverride({shader->getUniform(name), val});
+
+                        auto override = GfxShaderBinding(shader->getUniform(name), val);
+            setOverride(std::move(override));
         }
 
         void setVec3(std::string name, glm::vec3 value)
@@ -217,9 +231,11 @@ namespace brl
             if (shader->getUniform(name) == nullptr)
                 return;
 
-            GfxShaderValue* val = new GfxShaderValue();
+            auto val = std::make_shared<GfxShaderValue>();
             val->v3value = value;
-            setOverride({shader->getUniform(name), val});
+
+                        auto override = GfxShaderBinding(shader->getUniform(name), val);
+            setOverride(std::move(override));
         }
 
         void setVec4(std::string name, glm::vec4 value)
@@ -229,9 +245,11 @@ namespace brl
             if (!shader->getUniform(name))
                 return;
 
-            GfxShaderValue* val = new GfxShaderValue();
+            auto val = std::make_shared<GfxShaderValue>();
             val->v4value = value;
-            setOverride({shader->getUniform(name), val});
+
+                        auto override = GfxShaderBinding(shader->getUniform(name), val);
+            setOverride(std::move(override));
         }
 
         void setMat4(std::string name, glm::mat4 value, int index=0)
@@ -243,15 +261,17 @@ namespace brl
 
             if (overrides.contains(uniform))
             {
-                overrides[uniform]->m4value->operator[](index) = value;
+                overrides[uniform]->m4value->at(index) = value;
             }
             else
             {
 
-                GfxShaderValue* val = new GfxShaderValue();
+                auto val = std::make_shared<GfxShaderValue>();
                 val->m4value->reserve(uniform->count);
-                val->m4value->operator[](index) = value;
-                setOverride({uniform, val});
+                val->m4value->at(index) = value;
+
+                auto override = GfxShaderBinding(uniform, val);
+                setOverride((override));
             }
         }
 
@@ -264,14 +284,16 @@ namespace brl
 
             if (overrides.contains(uniform))
             {
-                overrides[uniform]->m4value = std::make_shared<std::vector<glm::mat4>>(value);
+                overrides[uniform]->m4value = CONVERT_UNIFORM_MAT4(value);
             }
             else
             {
 
-                GfxShaderValue* val = new GfxShaderValue();
-                val->m4value = std::make_shared<std::vector<glm::mat4>>(value);
-                setOverride({uniform, val});
+                auto val = std::make_shared<GfxShaderValue>();
+                val->m4value = CONVERT_UNIFORM_MAT4(value);
+                ;
+                auto override = GfxShaderBinding(uniform, val);
+                setOverride(std::move(override));
             }
         }
 
@@ -280,20 +302,31 @@ namespace brl
             if (!shader->getUniform(name))
                 return;
 
-            GfxShaderValue* val = new GfxShaderValue();
+            auto val = std::make_shared<GfxShaderValue>();
             val->txValue = value;
-            setOverride({shader->getUniform(name), val});
+
+            auto override = GfxShaderBinding(shader->getUniform(name), val);
+            setOverride(override);
         }
 
         GfxShaderValue* getUniform(std::string name)
         {
+            if (!shader->getUniform(name))
+                return nullptr;
+
             for (const auto& uniform : overrides)
             {
                 if (uniform.uniform->name == name)
-                    return uniform.value;
+                    return uniform.value.get();
             }
 
-            return {};
+            auto val = std::make_shared<GfxShaderValue>();
+
+
+            auto override = GfxShaderBinding(shader->getUniform(name), val);
+            setOverride(override);
+
+            return val.get();
 
         }
 
@@ -310,6 +343,7 @@ namespace brl
         unsigned int registryIndex = UINT_MAX;
 
         void setOverride(GfxShaderBinding pair);
+        void applyUniform(const brl::GfxShaderBinding& _override, brl::GfxShaderProgram* shader);
     };
 
     struct GfxMaterialMgr
diff --git a/src/brl/gfx/camera.cpp b/src/brl/gfx/camera.cpp
@@ -25,20 +25,20 @@ void brl::GfxCamera::draw(std::vector<GfxDrawCall>& calls, const GfxInstancedDra
     cachedFramebuffer->use();
     cachedFramebuffer->clear();
 
-    GfxShaderValue* viewValue = new GfxShaderValue;
-    GfxShaderValue* projValue = new GfxShaderValue;
-    GfxShaderValue* timeValue = new GfxShaderValue;
-    GfxShaderValue* cameraPosValue = new GfxShaderValue;
+    auto viewValue = std::make_shared<GfxShaderValue>();
+    auto projValue = std::make_shared<GfxShaderValue>();
+    auto timeValue = std::make_shared<GfxShaderValue>();
+    auto cameraPosValue = std::make_shared<GfxShaderValue>();
 
-    viewValue->m4value = std::make_shared<std::vector<glm::mat4>>(std::vector<glm::mat4>{GetViewMatrix()});
-    projValue->m4value = std::make_shared<std::vector<glm::mat4>>(std::vector<glm::mat4>{GetProjMatrix()});
+    viewValue->m4value = CONVERT_UNIFORM_MAT4(std::vector<glm::mat4>{GetViewMatrix()});
+    projValue->m4value = CONVERT_UNIFORM_MAT4(std::vector<glm::mat4>{GetProjMatrix()});
     timeValue->floatValue = glfwGetTime();
     cameraPosValue->v3value = position;
 
     for (GfxDrawCall& call : calls)
     {
-        GfxShaderValue* modelValue = new GfxShaderValue;
-        modelValue->m4value = std::make_shared<std::vector<glm::mat4>>(std::vector<glm::mat4>{call.transform});
+        auto modelValue = std::make_shared<GfxShaderValue>();
+        modelValue->m4value = CONVERT_UNIFORM_MAT4(std::vector<glm::mat4>{call.transform});
         GfxUniformList overrides;
         overrides.push_back({call.material->getShader()->getUniform("_internalView"), viewValue});
         overrides.push_back({call.material->getShader()->getUniform("_internalProj"), projValue});
diff --git a/src/brl/gfx/engine.cpp b/src/brl/gfx/engine.cpp
@@ -336,7 +336,7 @@ void brl::GfxEngine::update()
 }
 
 void brl::GfxEngine::insertCall(::brl::GfxMaterial* material, ::brl::GfxAttribBuffer* buffer, const glm::mat4& transform,
-                                std::vector<GfxShaderBinding>& uniqueOverrides, int instancingIdx)
+                                GfxUniformList& uniqueOverrides, int instancingIdx)
 {
     auto shader = material->getShader();
     if (shader->instancingEnabled)
diff --git a/src/brl/gfx/framebuffer.cpp b/src/brl/gfx/framebuffer.cpp
@@ -30,7 +30,7 @@ void brl::GfxFramebufferAttachment::draw(GfxMaterial* material)
 
     GfxUniformList uniforms = {};
 
-    GfxShaderValue* sourceValue = new GfxShaderValue;
+    auto sourceValue = std::make_shared<GfxShaderValue>();
     sourceValue->txValue = this;
     uniforms.push_back({material->getShader()->getUniform("_sourceTexture"), sourceValue});
 
diff --git a/src/brl/gfx/model.cpp b/src/brl/gfx/model.cpp
diff --git a/src/brl/gfx/shader.cpp b/src/brl/gfx/shader.cpp
diff --git a/src/brl/gfx/ui.cpp b/src/brl/gfx/ui.cpp

Original file line number	Diff line number	Diff line change
`@@ -336,7 +336,7 @@ void brl::GfxEngine::update()`
`336`	`336`	`}`
`337`	`337`
`338`	`338`	`void brl::GfxEngine::insertCall(::brl::GfxMaterial* material, ::brl::GfxAttribBuffer* buffer, const glm::mat4& transform,`
`339`		`- std::vector<GfxShaderBinding>& uniqueOverrides, int instancingIdx)`
	`339`	`+ GfxUniformList& uniqueOverrides, int instancingIdx)`
`340`	`340`	`{`
`341`	`341`	`auto shader = material->getShader();`
`342`	`342`	`if (shader->instancingEnabled)`