diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index fe5cc80b..e73c1dd6 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -230,8 +230,18 @@ namespace MobileGL::MG_Impl::GLImpl { // input primitive (GL 4.6 core 11.3.1); anything else is INVALID_OPERATION. GL_PATCHES // is the tessellation pipeline's input and reaches the geometry stage already // converted, so it is not constrained here. - const GLenum gsInput = currentProgram ? currentProgram->GetGeometryInputType() : GL_NONE; - if (gsInput != GL_NONE && mode != GL_PATCHES) { + // + // "Is there a geometry stage at all" has to be asked of the STAGE, never of the input + // primitive: GL_NONE and GL_POINTS are both 0, so a `layout(points) in` geometry shader + // is indistinguishable from no geometry shader by its reflected input type alone. The + // sentinel test this replaces therefore skipped the whole rule for exactly the geometry + // shaders whose input is the most restrictive one - every mode but GL_POINTS was + // accepted (KHR-GL43.transform_feedback.api_errors_test draws a points-in geometry + // program with GL_LINES and requires INVALID_OPERATION). + const Bool geometryActive = + currentProgram && currentProgram->GetShaderIndexByStage(ShaderStage::Geometry) >= 0; + const GLenum gsInput = geometryActive ? currentProgram->GetGeometryInputType() : GL_NONE; + if (geometryActive && mode != GL_PATCHES) { Bool compatible = false; switch (gsInput) { case GL_POINTS: diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 04a76493..7abdeec1 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -87,6 +87,7 @@ add_executable(MobileGLIntegrationTest Scenarios/Glsl420DeclarationScenario.cpp Scenarios/IoBlockNameCollisionScenario.cpp Scenarios/TessellationDrawModeScenario.cpp + Scenarios/GeometryDrawModeScenario.cpp Scenarios/FragmentOutputArrayIndexScenario.cpp Scenarios/BufferTextureScenario.cpp Scenarios/VertexAttribBindingScenario.cpp diff --git a/MobileGL/MG_IntegrationTest/Scenarios/GeometryDrawModeScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/GeometryDrawModeScenario.cpp new file mode 100644 index 00000000..bf8a57c4 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/GeometryDrawModeScenario.cpp @@ -0,0 +1,298 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/GeometryDrawModeScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// Scenario - A GEOMETRY SHADER'S INPUT PRIMITIVE CONSTRAINS THE DRAW MODE, AND +// GL_NONE IS NOT A USABLE "NO GEOMETRY SHADER" SENTINEL. +// +// GL 4.6 core 11.3.1: mode must be one of the primitive types that decomposes into the +// geometry shader's declared input primitive, or the draw is GL_INVALID_OPERATION. The +// validator asked "is there a geometry stage?" by comparing the REFLECTED INPUT PRIMITIVE +// against GL_NONE - and GL_NONE and GL_POINTS are both 0, so a `layout(points) in` geometry +// shader answered "no geometry stage" and every mode sailed through. The rule was therefore +// dead for exactly the geometry shaders whose input primitive rejects the most modes. +// +// KHR-GL43.transform_feedback.api_errors_test is where it showed: it draws a points-in +// geometry program with GL_LINES through glDrawTransformFeedbackInstanced and requires +// INVALID_OPERATION. The bug is not specific to that entry point - every draw shares this +// validator - so the ordinary glDrawArrays spelling is pinned here too, and the lines-in +// program is the control that proves the rule was not simply widened. +// +// Needs a real context: the validator returns before this rule when no backend object is +// active, so the GPU-free negative-API suite cannot reach it. + +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + const char* const kVertexSource = R"(#version 420 core +void main() +{ + gl_Position = vec4(0.0, 0.0, 0.0, 1.0); +} +)"; + + // The input primitive the CTS case uses, and the one the GL_NONE sentinel erased. + // `result` is here so the same program can be captured with transform feedback. + const char* const kPointsInGeometrySource = R"(#version 420 core +layout(points) in; +layout(points, max_vertices = 1) out; +out float result; +void main() +{ + gl_Position = gl_in[0].gl_Position; + result = 1.0; + EmitVertex(); +} +)"; + + const char* const kLinesInGeometrySource = R"(#version 420 core +layout(lines) in; +layout(points, max_vertices = 1) out; +void main() +{ + gl_Position = gl_in[0].gl_Position; + EmitVertex(); +} +)"; + + const char* const kFragmentSource = R"(#version 420 core +out vec4 fragColor; +void main() +{ + fragColor = vec4(0.0, 1.0, 0.0, 1.0); +} +)"; + + class GeometryDrawModeScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + glGenVertexArrays(1, &m_vao); + glBindVertexArray(m_vao); + if (!BackendHostsGeometry()) { + GTEST_SKIP() << "no geometry stage on " << Gl().BackendName() << " (" + << Gl().RendererString() << "); there is no input primitive to validate"; + } + } + + void TearDown() override { + if (!Ready()) return; + glUseProgram(0); + for (const GLuint program : m_programs) { + glDeleteProgram(program); + } + m_programs.clear(); + glBindVertexArray(0); + if (m_vao != 0) glDeleteVertexArrays(1, &m_vao); + m_vao = 0; + } + + // The same real-backend probe IoBlockNameCollisionScenario uses: 0 on a DirectGLES + // driver without GL_EXT_geometry_shader and on a DirectVulkan device without the + // geometryShader feature. + static bool BackendHostsGeometry() { + GLint maxGeometryOutputVertices = 0; + glGetIntegerv(GL_MAX_GEOMETRY_OUTPUT_VERTICES, &maxGeometryOutputVertices); + DrainErrors(); + return maxGeometryOutputVertices >= 4; + } + + static void DrainErrors() { + for (int i = 0; i < 16 && glGetError() != GL_NO_ERROR; ++i) { + } + } + + GLuint BuildProgram(const char* geometrySource, const char* capturedVarying = nullptr) { + const std::vector> stages = { + {GL_VERTEX_SHADER, kVertexSource}, + {GL_GEOMETRY_SHADER, geometrySource}, + {GL_FRAGMENT_SHADER, kFragmentSource}}; + + std::vector shaders; + bool ok = true; + for (const auto& [stage, source] : stages) { + const GLuint shader = glCreateShader(stage); + glShaderSource(shader, 1, &source, nullptr); + glCompileShader(shader); + GLint compiled = 0; + glGetShaderiv(shader, GL_COMPILE_STATUS, &compiled); + shaders.push_back(shader); + if (!compiled) { + m_buildLog = InfoLog(shader, true); + ok = false; + break; + } + } + if (!ok) { + for (const GLuint shader : shaders) glDeleteShader(shader); + return 0; + } + + const GLuint program = glCreateProgram(); + for (const GLuint shader : shaders) glAttachShader(program, shader); + if (capturedVarying != nullptr) { + glTransformFeedbackVaryings(program, 1, &capturedVarying, GL_INTERLEAVED_ATTRIBS); + } + glLinkProgram(program); + GLint linked = 0; + glGetProgramiv(program, GL_LINK_STATUS, &linked); + for (const GLuint shader : shaders) glDeleteShader(shader); + if (!linked) { + m_buildLog = InfoLog(program, false); + glDeleteProgram(program); + return 0; + } + m_programs.push_back(program); + return program; + } + + static std::string InfoLog(GLuint object, bool isShader) { + GLint length = 0; + if (isShader) { + glGetShaderiv(object, GL_INFO_LOG_LENGTH, &length); + } else { + glGetProgramiv(object, GL_INFO_LOG_LENGTH, &length); + } + std::vector buffer(static_cast(length) + 1, '\0'); + if (isShader) { + glGetShaderInfoLog(object, length + 1, nullptr, buffer.data()); + } else { + glGetProgramInfoLog(object, length + 1, nullptr, buffer.data()); + } + return buffer.data(); + } + + const std::string& BuildLog() const { return m_buildLog; } + + GLuint m_vao = 0; + std::vector m_programs; + std::string m_buildLog; + }; + + // GL_POINTS is the only mode that decomposes into a points input primitive. + TEST_F(GeometryDrawModeScenario, PointsInGeometryProgramRejectsEveryOtherMode) { + if (!Ready()) GTEST_SKIP(); + + const GLuint program = BuildProgram(kPointsInGeometrySource); + ASSERT_NE(program, 0u) << "the points-in geometry program did not build: " << BuildLog(); + + glUseProgram(program); + DrainErrors(); + + for (const GLenum mode : + {static_cast(GL_LINES), static_cast(GL_LINE_STRIP), + static_cast(GL_TRIANGLES), static_cast(GL_TRIANGLE_STRIP)}) { + glDrawArrays(mode, 0, 3); + EXPECT_EQ(glGetError(), static_cast(GL_INVALID_OPERATION)) + << "mode " << mode << " does not decompose into the geometry shader's points input"; + DrainErrors(); + } + + // The one mode that IS compatible still draws. + glDrawArrays(GL_POINTS, 0, 1); + EXPECT_EQ(glGetError(), static_cast(GL_NO_ERROR)); + DrainErrors(); + } + + // The same rule reached through glDrawTransformFeedback*, which is the spelling the CTS + // case asks about. The capture span is really completed first, so GL_POINTS comes back + // GL_NO_ERROR: without that the draw would report INVALID_OPERATION for the + // never-ended-a-span reason instead and the case could not tell the two apart. + TEST_F(GeometryDrawModeScenario, PointsInGeometryProgramRejectsNonPointModesOnFeedbackDraws) { + if (!Ready()) GTEST_SKIP(); + + const GLuint program = BuildProgram(kPointsInGeometrySource, "result"); + ASSERT_NE(program, 0u) << "the points-in geometry program did not build: " << BuildLog(); + + GLuint feedback = 0; + glGenTransformFeedbacks(1, &feedback); + glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, feedback); + GLuint captureBuffer = 0; + glGenBuffers(1, &captureBuffer); + glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, captureBuffer); + glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, 64, nullptr, GL_STATIC_DRAW); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, captureBuffer); + glUseProgram(program); + DrainErrors(); + + glBeginTransformFeedback(GL_POINTS); + glDrawArrays(GL_POINTS, 0, 1); + glEndTransformFeedback(); + ASSERT_EQ(glGetError(), static_cast(GL_NO_ERROR)) << "the capture span did not complete"; + + glDrawTransformFeedbackInstanced(GL_LINES, feedback, 1); + EXPECT_EQ(glGetError(), static_cast(GL_INVALID_OPERATION)) + << "glDrawTransformFeedbackInstanced must honour the geometry input primitive"; + DrainErrors(); + + glDrawTransformFeedbackStreamInstanced(GL_LINES, feedback, 0, 1); + EXPECT_EQ(glGetError(), static_cast(GL_INVALID_OPERATION)) + << "glDrawTransformFeedbackStreamInstanced must honour the geometry input primitive"; + DrainErrors(); + + // The compatible mode replays the captured span with no error at all, which is what + // makes the two assertions above about the MODE and not about the span. + glDrawTransformFeedbackInstanced(GL_POINTS, feedback, 1); + EXPECT_EQ(glGetError(), static_cast(GL_NO_ERROR)) + << "a compatible mode must still replay the captured span"; + DrainErrors(); + + glUseProgram(0); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, 0); + glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, 0); + glDeleteBuffers(1, &captureBuffer); + glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, 0); + glDeleteTransformFeedbacks(1, &feedback); + DrainErrors(); + } + + // The control: a lines-in geometry shader is a NON-zero input primitive, so it exercised + // the rule even before the fix. It must still accept the line modes and still reject the + // others - a fix that widened the rule instead of repairing its guard breaks this. + TEST_F(GeometryDrawModeScenario, LinesInGeometryProgramStillAcceptsLineModesOnly) { + if (!Ready()) GTEST_SKIP(); + + const GLuint program = BuildProgram(kLinesInGeometrySource); + ASSERT_NE(program, 0u) << "the lines-in geometry program did not build: " << BuildLog(); + + glUseProgram(program); + DrainErrors(); + + for (const GLenum mode : {static_cast(GL_LINES), static_cast(GL_LINE_STRIP), + static_cast(GL_LINE_LOOP)}) { + glDrawArrays(mode, 0, 2); + EXPECT_EQ(glGetError(), static_cast(GL_NO_ERROR)) + << "mode " << mode << " decomposes into lines and must be accepted"; + DrainErrors(); + } + + for (const GLenum mode : {static_cast(GL_POINTS), static_cast(GL_TRIANGLES)}) { + glDrawArrays(mode, 0, 3); + EXPECT_EQ(glGetError(), static_cast(GL_INVALID_OPERATION)) + << "mode " << mode << " does not decompose into lines"; + DrainErrors(); + } + } + + } // namespace +} // namespace MGITest