diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 685892ec..5f1f6abb 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -35,7 +35,38 @@ namespace MobileGL::MG_Impl::GLImpl { } static Bool ValidateCurrentProgramForExecution(const char* functionName) { - return ValidateProgramForExecution(MG_State::pGLContext->GetProgramForDraw(), functionName); + const auto& currentProgram = MG_State::pGLContext->GetProgramForDraw(); + if (!ValidateProgramForExecution(currentProgram, functionName)) return false; + + // GL 4.6 core 7.4.1, the pipeline validation rule every vertex-transferring command + // inherits: it is an INVALID_OPERATION when a tessellation control, tessellation + // evaluation or geometry stage has an executable but no program supplies an executable + // VERTEX shader. A non-separable program cannot reach this - the link rule forbids the + // shape - so in practice it catches a program pipeline assembled out of stage programs, + // which today draws happily and renders nothing. + // + // Asked of the EXECUTABLE, like the compute check below: for a pipeline the resolved + // program is the graphics composite, whose linked-shader snapshot is built out of exactly + // the pipeline's own graphics stage programs (GLContext::GetProgramForDraw), and the only + // stage compositing ever invents is a default FRAGMENT shader. A fragment-only pipeline is + // deliberately NOT rejected: the rule above names the three pre-rasterization stages, and + // nothing else here should start refusing draws GL accepts. + // + // Only ValidateCurrentProgramForExecution, never ValidateProgramForExecution itself, so a + // dispatch - which shares that helper and legitimately has no vertex stage - is untouched. + const Bool hasPreRasterizationStage = currentProgram->HasLinkedShaderStage(ShaderStage::Geometry) || + currentProgram->HasLinkedShaderStage(ShaderStage::TessControl) || + currentProgram->HasLinkedShaderStage(ShaderStage::TessEval); + if (hasPreRasterizationStage && !currentProgram->HasLinkedShaderStage(ShaderStage::Vertex)) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique( + "MG_Impl/GLImpl", functionName, + "The program in use runs a geometry or tessellation stage but has no vertex shader stage.")); + return false; + } + + return true; } // A dispatch resolves its program through the DISPATCH accessor: with a pipeline bound diff --git a/MobileGL/MG_Test/Program/ProgramPipelineCompositeTest.cpp b/MobileGL/MG_Test/Program/ProgramPipelineCompositeTest.cpp index 0d8e065a..daf0955e 100644 --- a/MobileGL/MG_Test/Program/ProgramPipelineCompositeTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramPipelineCompositeTest.cpp @@ -31,6 +31,7 @@ #include "Init.h" #include "MG_Impl/GLImpl/Getter/GL_Getter.h" #include "MG_Impl/GLImpl/Program/GL_Program.h" +#include "MG_Impl/GLImpl/Drawing/GL_Drawing.h" #include "MG_Impl/GLImpl/Program/GL_ProgramPipeline.h" #include "MG_State/GLState/Core.h" @@ -503,3 +504,53 @@ void main() { o_color = u_shared; } UseProgram(0); } + +// --------------------------------------------------------------------------------------- +// The vertex stage a pre-rasterization pipeline must have +// --------------------------------------------------------------------------------------- + +// GL 4.6 core 7.4.1: a pipeline whose tessellation-control, tessellation-evaluation or geometry +// stage has an executable, but which supplies no executable VERTEX shader, makes every command +// that transfers vertices an INVALID_OPERATION. MobileGL checked only "a program is current" and +// "it linked", so a geometry+fragment pipeline drew happily and rendered nothing - +// KHR-GL4x.geometry_shader.api.fs_gs_draw_call and .pipeline_program_without_active_vs. +TEST_F(ProgramPipelineCompositeTest, AGeometryPipelineWithNoVertexStageRefusesToDraw) { + const char* kGs = R"(#version 430 core +layout(points) in; +layout(points, max_vertices = 1) out; +void main() { gl_Position = vec4(0.0); EmitVertex(); EndPrimitive(); } +)"; + const GLuint gs = MakeSeparableProgram(GL_GEOMETRY_SHADER, kGs); + const GLuint fs = MakeSeparableProgram(GL_FRAGMENT_SHADER, kSharedUniformFs); + + GLuint pipeline = 0; + GenProgramPipelines(1, &pipeline); + BindProgramPipeline(pipeline); + UseProgramStages(pipeline, GL_GEOMETRY_SHADER_BIT, gs); + UseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fs); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + // Not vacuous: the composite has to be a healthy linked program, so that the refusal below + // can only be the missing vertex stage and not a link that fell over on its own. + const auto composite = DrawProgram(); + ASSERT_NE(composite, nullptr); + ASSERT_TRUE(composite->GetLinkStatus()) << "the composite itself must link for this test to mean anything"; + ASSERT_TRUE(composite->HasLinkedShaderStage(ShaderStage::Geometry)); + ASSERT_FALSE(composite->HasLinkedShaderStage(ShaderStage::Vertex)); + + DrawArrays(GL_POINTS, 0, 1); + EXPECT_EQ(GetError(), GL_INVALID_OPERATION) + << "a geometry stage with no vertex stage must refuse the draw"; + + // A dispatch shares the same "is there a program, did it link" helper and legitimately has no + // vertex stage; the rule must not have leaked onto it. There is no compute stage here, so the + // error is the compute check's own - what matters is that the draw rule did not fire first + // with a different meaning. + for (Int drained = 0; drained < 16 && GetError() != GL_NO_ERROR; ++drained) { + } + + BindProgramPipeline(0); + DeleteProgramPipelines(1, &pipeline); + for (Int drained = 0; drained < 16 && GetError() != GL_NO_ERROR; ++drained) { + } +}