From bdcd44b340962c78c8392d657232b99cd62d6871 Mon Sep 17 00:00:00 2001 From: omigamedev Date: Mon, 1 Jun 2026 17:58:09 +0200 Subject: [PATCH] Extract OpenGL shader attribute bindings --- CMakeLists.txt | 3 +- docs/modernization/build-inventory.md | 3 +- docs/modernization/roadmap.md | 11 ++++-- src/renderer_gl/shader_bindings.cpp | 43 ++++++++++++++++++++++ src/renderer_gl/shader_bindings.h | 19 ++++++++++ src/shader.cpp | 20 +++------- tests/renderer_gl/capabilities_tests.cpp | 47 ++++++++++++++++++++++++ 7 files changed, 127 insertions(+), 19 deletions(-) create mode 100644 src/renderer_gl/shader_bindings.cpp create mode 100644 src/renderer_gl/shader_bindings.h diff --git a/CMakeLists.txt b/CMakeLists.txt index dbf34ea..b04c63a 100644 --- a/CMakeLists.txt +++ b/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") diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index c79091e..9d66eb7 100644 --- a/docs/modernization/build-inventory.md +++ b/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. diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 7371188..4d31116 100644 --- a/docs/modernization/roadmap.md +++ b/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 diff --git a/src/renderer_gl/shader_bindings.cpp b/src/renderer_gl/shader_bindings.cpp new file mode 100644 index 0000000..c264843 --- /dev/null +++ b/src/renderer_gl/shader_bindings.cpp @@ -0,0 +1,43 @@ +#include "renderer_gl/shader_bindings.h" + +#include +#include + +namespace pp::renderer::gl { + +std::span panopainter_shader_attribute_bindings() noexcept +{ + static constexpr std::array 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 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(); +} + +} diff --git a/src/renderer_gl/shader_bindings.h b/src/renderer_gl/shader_bindings.h new file mode 100644 index 0000000..bb1bbfb --- /dev/null +++ b/src/renderer_gl/shader_bindings.h @@ -0,0 +1,19 @@ +#pragma once + +#include "foundation/result.h" + +#include +#include + +namespace pp::renderer::gl { + +struct OpenGlAttributeBinding { + const char* name = ""; + std::uint32_t location = 0; +}; + +[[nodiscard]] std::span panopainter_shader_attribute_bindings() noexcept; +[[nodiscard]] pp::foundation::Status validate_shader_attribute_bindings( + std::span bindings) noexcept; + +} diff --git a/src/shader.cpp b/src/shader.cpp index c52e04e..2730ef0 100644 --- a/src/shader.cpp +++ b/src/shader.cpp @@ -3,6 +3,7 @@ #include "shader.h" #include "asset.h" #include "app.h" +#include "renderer_gl/shader_bindings.h" std::map 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(binding.location), binding.name); + } glLinkProgram(ps); glGetProgramiv(ps, GL_LINK_STATUS, &status); diff --git a/tests/renderer_gl/capabilities_tests.cpp b/tests/renderer_gl/capabilities_tests.cpp index 505dde9..400839d 100644 --- a/tests/renderer_gl/capabilities_tests.cpp +++ b/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 #include +#include #include 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 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(); }