Refactor

.github/workflows/cmake.yml

@@ -4,71 +4,68 @@ on:
   push:
     branches:
       - master
+      - development
       - c-i
   pull_request:
     branches:
       - master
-env:
-  # Customize the CMake build type here (Release, Debug, RelWithDebInfo, etc.)
-  BUILD_TYPE: Release
+      - development
 
 jobs:
   build:
-    # The CMake configure and build commands are platform agnostic and should work equally
-    # well on Windows or Mac.  You can convert this to a matrix build if you need
-    # cross-platform coverage.
-    # See: https://docs.github.com/en/free-pro-team@latest/actions/learn-github-actions/managing-complex-workflows#using-a-build-matrix
+    name: build-${{ matrix.os }}
     runs-on: ${{ matrix.os }}
     strategy:
+      fail-fast: false
       matrix:
-        os: [windows-latest]
-
+        os: [windows-latest, ubuntu-latest]
+    env:
+      BUILD_TYPE: Release
     steps:
-    - uses: actions/checkout@v2
-    
-    - name: Linux dev lib
+    - uses: actions/checkout@v4
+
+    - name: Install Linux dependencies
       if: runner.os == 'Linux'
-      run:  |
-            sudo apt-get update
-            sudo add-apt-repository ppa:ubuntu-toolchain-r/test
-            sudo apt-get update
-            sudo apt-get install -y gcc-10 g++-10
-            sudo apt-get install -y libxrandr-dev xorg-dev libglu1-mesa-dev
-            sudo apt-get install -y libgtk-3-dev
-            export CC=gcc-10
-            export CXX=g++-10
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y libxrandr-dev xorg-dev libglu1-mesa-dev libgtk-3-dev
 
-            sudo apt-get install libgtest-dev
-            cd /usr/src/gtest
-            sudo mkdir build
-            cd build
-            sudo cmake ..
-            sudo make
-
-    - name: Create Build Environment
-      # Some projects don't allow in-source building, so create a separate build directory
-      # We'll use this as our working directory for all subsequent commands
-      run: cmake -E make_directory ${{runner.workspace}}/build
     - name: Configure CMake
-      # Use a bash shell so we can use the same syntax for environment variable
-      # access regardless of the host operating system
-      shell: bash
-      working-directory: ${{runner.workspace}}/build
-      # Note the current convention is to use the -S and -B options here to specify source 
-      # and build directories, but this is only available with CMake 3.13 and higher.  
-      # The CMake binaries on the Github Actions machines are (as of this writing) 3.12
-      run: cmake $GITHUB_WORKSPACE -DCMAKE_BUILD_TYPE=$BUILD_TYPE
+      run: cmake -S . -B build -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }}
 
     - name: Build
-      working-directory: ${{runner.workspace}}/build
-      shell: bash
-      # Execute the build.  You can specify a specific target with "--target <NAME>"
-      run: cmake --build . --config $BUILD_TYPE
+      run: cmake --build build --config ${{ env.BUILD_TYPE }}
 
     - name: Test
-      working-directory: ${{runner.workspace}}/build
-      shell: bash
-      # Execute tests defined by the CMake configuration.  
-      # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
-      run: ctest -C $BUILD_TYPE --rerun-failed --output-on-failure
+      working-directory: build
+      run: ctest -C ${{ env.BUILD_TYPE }} --output-on-failure
+
+  sanitizers:
+    name: asan-ubsan (ubuntu)
+    runs-on: ubuntu-latest
+    env:
+      BUILD_TYPE: Debug
+      CC: clang
+      CXX: clang++
+      CXXFLAGS: -fsanitize=address,undefined -fno-omit-frame-pointer
+      LDFLAGS: -fsanitize=address,undefined
+    steps:
+    - uses: actions/checkout@v4
+
+    - name: Install Linux dependencies
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y libxrandr-dev xorg-dev libglu1-mesa-dev libgtk-3-dev
 
+    - name: Configure CMake
+      run: cmake -S . -B build -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }}
+
+    - name: Build
+      run: cmake --build build --config ${{ env.BUILD_TYPE }}
+
+    - name: Test
+      working-directory: build
+      env:
+        ASAN_OPTIONS: detect_leaks=1:halt_on_error=0
+        UBSAN_OPTIONS: print_stacktrace=1
+      run: ctest -C ${{ env.BUILD_TYPE }} --output-on-failure

.gitignore

@@ -6,3 +6,5 @@ build/
 
 .vscode/
 /.vs
+
+docs/REMEDIATION_PLAN.md

Assets/Shaders/glsl/pbr.fs

@@ -96,7 +96,7 @@ void main() {
     vec3 albedo     = pow(material.hasBaseColorMap ? texture(material.baseColorMap, ftex_coords).rgb : material.baseColor, vec3(2.2));
     float metallic  = material.hasMetallicMap ? texture(material.metallicMap, ftex_coords).b : material.metallic;
     float roughness = material.hasRoughnessMap ? texture(material.roughnessMap, ftex_coords).g : material.roughness;
-    float ao        = material.hasAoMap ? texture(material.aoMap, ftex_coords).length() : material.ao;
+    float ao        = material.hasAoMap ? texture(material.aoMap, ftex_coords).r : material.ao;
 
     vec3 N = getNormalFromMap();
     vec3 V = normalize(fview - fposition);
@@ -109,11 +109,19 @@ void main() {
     // reflectance equation
     vec3 Lo = vec3(0.0);
     for(int i = 0; i < light_count; ++i) {
-        // calculate per-light radiance
-        vec3 L = normalize(lights[i].position - fposition);
+        // calculate per-light radiance. Directional lights (type 1) come from a fixed
+        // direction with no distance attenuation; point/spot use position + falloff.
+        vec3 L;
+        float attenuation;
+        if(lights[i].type == 1) {
+            L = normalize(-lights[i].rotation);
+            attenuation = 1.0;
+        } else {
+            L = normalize(lights[i].position - fposition);
+            float distance = length(lights[i].position - fposition);
+            attenuation = 1.0 / (1 + lights[i].distance_dropoff * distance * distance);
+        }
         vec3 H = normalize(V + L);
-        float distance = length(lights[i].position - fposition);
-        float attenuation = 1.0 / (1 + lights[i].distance_dropoff * distance * distance);
         vec3 radiance = lights[i].color * attenuation;
 
         // Cook-Torrance BRDF

Assets/Shaders/glsl/phong.fs

@@ -22,6 +22,8 @@ struct Material {
 uniform Material material;
 
 in vec3 fnormal;
+in vec3 ftangent;
+in vec3 fbitangent;
 in vec3 fposition;
 in vec3 fview;
 in vec2 ftex_coords;
@@ -64,16 +66,27 @@ vec4 applyLight(Light light) {
     } else if(light.type == 2) {
         //TODO: Spot lights
     }
+    // Non-void function must return on every path (spot/unknown types fell off the end -> UB).
+    return vec4(0.0);
 }
 
-vec3 colorToNormal(vec3 color) {
-    return normalize(vec3(color.x * 2 - 1, color.y * 2 - 1, color.z * 2 - 1));
+// Transform the tangent-space normal-map sample into world space using a proper TBN basis
+// (Gram-Schmidt re-orthogonalized), matching pbr.fs. The previous math was not a TBN
+// transform at all.
+vec3 getNormalFromMap() {
+    vec3 tangentNormal = texture(material.normal_map, ftex_coords).xyz * 2.0 - 1.0;
+    vec3 N = normalize(fnormal);
+    vec3 T = normalize(ftangent);
+    T = normalize(T - dot(T, N) * N);
+    vec3 B = cross(N, T);
+    mat3 TBN = mat3(T, B, N);
+    return normalize(TBN * tangentNormal);
 }
 
 
 void main() {
     if(material.use_normal_map) {
-        normal = normalize(fnormal + (fnormal * colorToNormal(texture(material.normal_map, ftex_coords).xyz)));
+        normal = getNormalFromMap();
     } else {
         normal = fnormal;
     }

Assets/Shaders/glsl/picking.fs

@@ -5,5 +5,11 @@ uniform int objectID;
 out vec4 frag_color;
 
 void main() {
-    frag_color = vec4((objectID & 0xFF) / 255.0, ((objectID & 0xFF00) >> 8) / 255, ((objectID & 0xFF0000) >> 16 / 255), 1.0);
+    // Encode the 24-bit object id across R,G,B (decoded as R + G<<8 + B<<16). The old
+    // code used integer division (/255) and had an operator-precedence bug (>> 16 / 255),
+    // so ids >= 256 could not round-trip.
+    frag_color = vec4(float(objectID & 0xFF) / 255.0,
+                      float((objectID >> 8) & 0xFF) / 255.0,
+                      float((objectID >> 16) & 0xFF) / 255.0,
+                      1.0);
 }

Assets/Shaders/glsl/picking.vs

@@ -0,0 +1,32 @@
+#version 420 core
+
+#include "vert_uniforms.glsl"
+#define MAX_BONES 100
+#define MAX_BONE_INFLUENCE 4
+
+layout (location = 0) in vec3 vertex;
+layout (location = 5) in ivec4 bone_ids;
+layout (location = 6) in vec4 bone_weights;
+
+// Picking is not instanced, so the model matrix is a uniform (skinning.vs takes it as a
+// per-instance vertex attribute). Skinning is applied with the same logic as skinning.vs
+// so an animated model is picked at its current pose, matching what is rendered on screen.
+uniform mat4 model;
+uniform mat4 bonesTransformMatrices[MAX_BONES];
+
+void main() {
+    vec4 totalPosition = vec4(0.0);
+    if (bone_ids == ivec4(-1)) {
+        totalPosition = vec4(vertex, 1.0);
+    } else {
+        for (int i = 0; i < MAX_BONE_INFLUENCE; i++) {
+            if (bone_ids[i] == -1) continue;
+            if (bone_ids[i] >= MAX_BONES) {
+                totalPosition = vec4(vertex, 1.0);
+                break;
+            }
+            totalPosition += bonesTransformMatrices[bone_ids[i]] * vec4(vertex, 1.0) * bone_weights[i];
+        }
+    }
+    gl_Position = uProjection * uView * model * totalPosition;
+}

Assets/Shaders/glsl/skinning.vs

@@ -11,8 +11,8 @@ layout (location = 3) in vec3 tangent;
 layout (location = 4) in vec3 bitangent;
 layout (location = 5) in ivec4 bone_ids;
 layout (location = 6) in vec4 bone_weights;
+layout (location = 7) in mat4 model;
 
-uniform mat4 model;
 uniform mat4 bonesTransformMatrices[MAX_BONES];
 
 out vec3 fnormal;
@@ -46,10 +46,12 @@ void main() {
 			}
 
 			mat4 finalBonesMatrix = bonesTransformMatrices[bone_ids[i]];
-			
+
 			totalPosition += finalBonesMatrix * vec4(vertex, 1.0f) * bone_weights[i];
-
-			mat3 normalMatrix = mat3(transpose(inverse(finalBonesMatrix)));
+
+			// Skeletal bones are rigid (rotation + translation), so the 3x3 already is the
+			// correct normal transform -- no need for a per-bone, per-vertex inverse-transpose.
+			mat3 normalMatrix = mat3(finalBonesMatrix);
 			totalNormal += normalMatrix * normal * bone_weights[i];
 			totalTangent += normalMatrix * tangent * bone_weights[i];
 			totalBitangent += normalMatrix * bitangent * bone_weights[i];
@@ -65,6 +67,7 @@ void main() {
 
     gl_Position = uProjection * uView * model * totalPosition;
 
-    fview = (inverse(uView) * vec4(0, 0, 0, 1)).xyz;
+    // Camera world position comes from the UBO instead of a per-vertex inverse(uView).
+    fview = uCameraPos.xyz;
     ftex_coords = tex_coords;
 }

Assets/Shaders/glsl/skybox.vs

@@ -11,5 +11,9 @@ out vec3 TexCoords;
 void main()
 {
     TexCoords = aPos;
-    gl_Position = uProjection * uView * vec4(aPos, 1.0);
+    // Strip translation from the view so the skybox stays centered on the camera, and use
+    // the z=w trick so its depth is always 1.0 -- drawn behind everything with GL_LEQUAL.
+    mat4 rotView = mat4(mat3(uView));
+    vec4 clipPos = uProjection * rotView * vec4(aPos, 1.0);
+    gl_Position = clipPos.xyww;
 }

Assets/Shaders/glsl/ui.fs

@@ -0,0 +1,21 @@
+#version 420 core
+
+out vec4 fragColor;
+
+in vec2 fUV;
+
+uniform vec4 uColor;
+uniform sampler2D uTexture;
+
+// 0 = solid colour, 1 = colour modulated by an RGBA texture, 2 = text (texture .r is coverage).
+uniform int uMode;
+
+void main() {
+    if (uMode == 2) {
+        fragColor = vec4(uColor.rgb, uColor.a * texture(uTexture, fUV).r);
+    } else if (uMode == 1) {
+        fragColor = uColor * texture(uTexture, fUV);
+    } else {
+        fragColor = uColor;
+    }
+}

Assets/Shaders/glsl/ui.vs

@@ -0,0 +1,17 @@
+#version 420 core
+layout (location = 0) in vec2 aPos;
+
+out vec2 fUV;
+
+uniform mat4 projection;
+uniform mat4 model;
+
+// Sub-rectangle of the bound texture to sample: the whole texture by default, a single glyph's
+// cell when drawing text from the font atlas.
+uniform vec2 uUVOffset;
+uniform vec2 uUVScale;
+
+void main() {
+    gl_Position = projection * model * vec4(aPos, 0.0, 1.0);
+    fUV = uUVOffset + aPos * uUVScale;
+}

Assets/Shaders/glsl/vert_uniforms.glsl

@@ -1,4 +1,5 @@
 layout(std140, binding = 0) uniform SceneData {
     mat4 uProjection;
     mat4 uView;
+    vec4 uCameraPos;  // world-space camera position (xyz)
 };

Assets/Shaders/picking.shader.json

@@ -1,7 +1,7 @@
 [
   {
     "stage": "vertex",
-    "source": "glsl/skinning.vs"
+    "source": "glsl/picking.vs"
   },
   {
     "stage": "fragment",

Assets/Shaders/ui.shader.json

@@ -0,0 +1,10 @@
+[
+  {
+    "stage": "vertex",
+    "source": "glsl/ui.vs"
+  },
+  {
+    "stage": "fragment",
+    "source": "glsl/ui.fs"
+  }
+]

CMakeLists.txt

@@ -1,14 +1,21 @@
 cmake_minimum_required(VERSION 3.19)
 project(ICE_ROOT)
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++20")
 set(CMAKE_CXX_STANDARD 20)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(CMAKE_CXX_EXTENSIONS OFF)
 
-include(cmake/fetch_dependencies.cmake)
-enable_testing()
 set(GLFW_INSTALL OFF CACHE BOOL "" FORCE)
 set(GLFW_BUILD_DOCS OFF CACHE BOOL "" FORCE)
 set(GLFW_BUILD_TESTS OFF CACHE BOOL "" FORCE)
 set(GLFW_BUILD_EXAMPLES OFF CACHE BOOL "" FORCE)
+set(BUILD_IMXML_EXAMPLE OFF CACHE BOOL "" FORCE)
+
+add_compile_definitions(NOMINMAX)
+
+
+include(cmake/fetch_dependencies.cmake)
+enable_testing()
+
 
 if(APPLE)
 	find_library(COCOA_LIBRARY Cocoa REQUIRED)
@@ -35,4 +42,10 @@ set(GL3W_SRC
 
 add_subdirectory(ICE)
 add_subdirectory(ICEBERG)
-add_subdirectory(ICEFIELD)
+add_subdirectory(ICEFIELD)
+
+# Architectural guard: fail the test run if a new inter-module dependency cycle is introduced.
+# Enforces the target module DAG (see cmake/module_dependency_check.cmake and
+# docs/module_dependencies.md). Runs standalone -- no build required -- so it can also gate CI early.
+add_test(NAME module_cycle_check
+    COMMAND ${CMAKE_COMMAND} -DICE_DIR=${CMAKE_SOURCE_DIR}/ICE -P ${CMAKE_SOURCE_DIR}/cmake/module_dependency_check.cmake)