Extract OpenGL shader attribute bindings

CMakeLists.txt

@@ -164,7 +164,8 @@ target_link_libraries(pp_renderer_api
 
 if(PP_ENABLE_OPENGL)
     add_library(pp_renderer_gl STATIC
-        src/renderer_gl/opengl_capabilities.cpp)
+        src/renderer_gl/opengl_capabilities.cpp
+        src/renderer_gl/shader_bindings.cpp)
     target_include_directories(pp_renderer_gl
         PUBLIC
             "${CMAKE_CURRENT_SOURCE_DIR}/src")

docs/modernization/build-inventory.md

@@ -132,7 +132,8 @@ Known local toolchain state:
   invalid channel counts rejected by `Texture2D::create(Image)`, and
   framebuffer status naming used by `RTT` diagnostics. It also validates
   Shape index-type and fill/stroke primitive-mode mapping used by the legacy
-  mesh draw path.
+  mesh draw path, plus the PanoPainter shader attribute binding catalog used
+  by legacy `Shader` creation.
 - `windows-msvc-vcpkg-headless` validates manifest install/configure/build/test
   for the current headless component matrix; see DEBT-0007 for remaining app
   and platform triplet migration.

docs/modernization/roadmap.md

@@ -393,8 +393,10 @@ format mapping for `Texture2D` image uploads and framebuffer status naming for
 `RTT` diagnostics. Mesh index-type and primitive-mode decisions used by legacy
 `Shape` drawing also live in `pp_renderer_gl`. The legacy app delegates
 extension, upload-format, framebuffer diagnostic, and mesh draw-mode
-interpretation to that backend library, but the existing renderer classes are
-not yet fully behind the renderer interfaces.
+interpretation to that backend library. The PanoPainter shader attribute
+binding catalog also lives in `pp_renderer_gl` and is consumed by legacy
+`Shader` creation. The existing renderer classes are not yet fully behind the
+renderer interfaces.
 
 Implementation tasks:
 
@@ -636,7 +638,10 @@ Results:
   WebGL exclusion behavior, upload types for RGBA8/RGBA16F/RGBA32F internal
   formats, image channel-count format mapping including invalid counts, and
   framebuffer status names, plus Shape index-type and fill/stroke primitive
-  mode mapping.
+  mode mapping. Shader attribute binding catalog validation covers the current
+  `pos`, `uvs`, `uvs2`, `col`, and `nor` bindings and rejects empty, unnamed,
+  null-name, and duplicate-name catalogs while preserving legacy shared
+  locations.
 - PowerShell analyze automation returns JSON summaries and includes the shader
   validation target.
 - `windows-msvc-vcpkg-headless` configured through the Visual Studio bundled

src/renderer_gl/shader_bindings.cpp

@@ -0,0 +1,43 @@
+#include "renderer_gl/shader_bindings.h"
+
+#include <array>
+#include <cstring>
+
+namespace pp::renderer::gl {
+
+std::span<const OpenGlAttributeBinding> panopainter_shader_attribute_bindings() noexcept
+{
+    static constexpr std::array<OpenGlAttributeBinding, 5> bindings {
+        OpenGlAttributeBinding { .name = "pos", .location = 0 },
+        OpenGlAttributeBinding { .name = "uvs", .location = 1 },
+        OpenGlAttributeBinding { .name = "uvs2", .location = 2 },
+        OpenGlAttributeBinding { .name = "col", .location = 3 },
+        OpenGlAttributeBinding { .name = "nor", .location = 3 },
+    };
+
+    return bindings;
+}
+
+pp::foundation::Status validate_shader_attribute_bindings(
+    std::span<const OpenGlAttributeBinding> bindings) noexcept
+{
+    if (bindings.empty()) {
+        return pp::foundation::Status::invalid_argument("shader attribute binding catalog is empty");
+    }
+
+    for (std::size_t i = 0; i < bindings.size(); ++i) {
+        if (bindings[i].name == nullptr || bindings[i].name[0] == '\0') {
+            return pp::foundation::Status::invalid_argument("shader attribute binding has no name");
+        }
+
+        for (std::size_t j = i + 1; j < bindings.size(); ++j) {
+            if (bindings[j].name != nullptr && std::strcmp(bindings[i].name, bindings[j].name) == 0) {
+                return pp::foundation::Status::invalid_argument("shader attribute binding name is duplicated");
+            }
+        }
+    }
+
+    return pp::foundation::Status::success();
+}
+
+}

src/renderer_gl/shader_bindings.h

@@ -0,0 +1,19 @@
+#pragma once
+
+#include "foundation/result.h"
+
+#include <cstdint>
+#include <span>
+
+namespace pp::renderer::gl {
+
+struct OpenGlAttributeBinding {
+    const char* name = "";
+    std::uint32_t location = 0;
+};
+
+[[nodiscard]] std::span<const OpenGlAttributeBinding> panopainter_shader_attribute_bindings() noexcept;
+[[nodiscard]] pp::foundation::Status validate_shader_attribute_bindings(
+    std::span<const OpenGlAttributeBinding> bindings) noexcept;
+
+}

src/shader.cpp

@@ -3,6 +3,7 @@
 #include "shader.h"
 #include "asset.h"
 #include "app.h"
+#include "renderer_gl/shader_bindings.h"
 
 std::map<kShader, Shader> ShaderManager::m_shaders;
 Shader* ShaderManager::m_current;
@@ -230,20 +231,11 @@ bool Shader::create(const std::string& vertex, const std::string& fragment)
         glDeleteShader(fs);
 
         glLinkProgram(ps);
-        if (glGetAttribLocation(ps, "pos") != -1)
-            glBindAttribLocation(ps, 0, "pos");
-
-        if (glGetAttribLocation(ps, "uvs") != -1)
-            glBindAttribLocation(ps, 1, "uvs");
-
-        if (glGetAttribLocation(ps, "uvs2") != -1)
-            glBindAttribLocation(ps, 2, "uvs2");
-
-        if (glGetAttribLocation(ps, "col") != -1)
-            glBindAttribLocation(ps, 3, "col");
-
-        if (glGetAttribLocation(ps, "nor") != -1)
-            glBindAttribLocation(ps, 3, "nor");
+        for (const auto& binding : pp::renderer::gl::panopainter_shader_attribute_bindings())
+        {
+            if (glGetAttribLocation(ps, binding.name) != -1)
+                glBindAttribLocation(ps, static_cast<GLuint>(binding.location), binding.name);
+        }
 
         glLinkProgram(ps);
         glGetProgramiv(ps, GL_LINK_STATUS, &status);

tests/renderer_gl/capabilities_tests.cpp

@@ -1,8 +1,10 @@
 #include "renderer_gl/opengl_capabilities.h"
+#include "renderer_gl/shader_bindings.h"
 #include "test_harness.h"
 
 #include <array>
 #include <cstdint>
+#include <cstring>
 #include <string_view>
 
 namespace {
@@ -156,6 +158,49 @@ void maps_shape_index_and_primitive_modes(pp::tests::Harness& h)
     PP_EXPECT(h, pp::renderer::gl::primitive_mode_for_stroke_count(99U) == gl_lines);
 }
 
+void exposes_shader_attribute_binding_catalog(pp::tests::Harness& h)
+{
+    const auto bindings = pp::renderer::gl::panopainter_shader_attribute_bindings();
+
+    PP_EXPECT(h, bindings.size() == 5U);
+    PP_EXPECT(h, pp::renderer::gl::validate_shader_attribute_bindings(bindings).ok());
+    PP_EXPECT(h, std::strcmp(bindings[0].name, "pos") == 0);
+    PP_EXPECT(h, bindings[0].location == 0U);
+    PP_EXPECT(h, std::strcmp(bindings[1].name, "uvs") == 0);
+    PP_EXPECT(h, bindings[1].location == 1U);
+    PP_EXPECT(h, std::strcmp(bindings[2].name, "uvs2") == 0);
+    PP_EXPECT(h, bindings[2].location == 2U);
+    PP_EXPECT(h, std::strcmp(bindings[3].name, "col") == 0);
+    PP_EXPECT(h, bindings[3].location == 3U);
+    PP_EXPECT(h, std::strcmp(bindings[4].name, "nor") == 0);
+    PP_EXPECT(h, bindings[4].location == 3U);
+}
+
+void rejects_invalid_shader_attribute_binding_catalogs(pp::tests::Harness& h)
+{
+    const std::array<pp::renderer::gl::OpenGlAttributeBinding, 0> empty {};
+    const std::array unnamed {
+        pp::renderer::gl::OpenGlAttributeBinding { .name = "", .location = 0 },
+    };
+    const std::array null_named {
+        pp::renderer::gl::OpenGlAttributeBinding { .name = nullptr, .location = 0 },
+    };
+    const std::array duplicate_name {
+        pp::renderer::gl::OpenGlAttributeBinding { .name = "pos", .location = 0 },
+        pp::renderer::gl::OpenGlAttributeBinding { .name = "pos", .location = 1 },
+    };
+    const std::array duplicate_location {
+        pp::renderer::gl::OpenGlAttributeBinding { .name = "col", .location = 3 },
+        pp::renderer::gl::OpenGlAttributeBinding { .name = "nor", .location = 3 },
+    };
+
+    PP_EXPECT(h, !pp::renderer::gl::validate_shader_attribute_bindings(empty).ok());
+    PP_EXPECT(h, !pp::renderer::gl::validate_shader_attribute_bindings(unnamed).ok());
+    PP_EXPECT(h, !pp::renderer::gl::validate_shader_attribute_bindings(null_named).ok());
+    PP_EXPECT(h, !pp::renderer::gl::validate_shader_attribute_bindings(duplicate_name).ok());
+    PP_EXPECT(h, pp::renderer::gl::validate_shader_attribute_bindings(duplicate_location).ok());
+}
+
 }
 
 int main()
@@ -169,5 +214,7 @@ int main()
     harness.run("maps_image_channel_count_to_texture_format", maps_image_channel_count_to_texture_format);
     harness.run("names_framebuffer_status_codes", names_framebuffer_status_codes);
     harness.run("maps_shape_index_and_primitive_modes", maps_shape_index_and_primitive_modes);
+    harness.run("exposes_shader_attribute_binding_catalog", exposes_shader_attribute_binding_catalog);
+    harness.run("rejects_invalid_shader_attribute_binding_catalogs", rejects_invalid_shader_attribute_binding_catalogs);
     return harness.finish();
 }