From d5286e69b6b8daab468b3cbf40f2413fcefc5bf6 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 27 Aug 2026 02:11:19 -0400 Subject: [PATCH] [Feat] (Tessellation): implement glPatchParameterfv and bake the default levels into both pass-through control stages --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 14 +- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 21 ++- MobileGL/MG_Backend/DirectGLES/Managers.h | 15 +++ MobileGL/MG_Backend/DirectGLES/Utils.cpp | 33 ++++- MobileGL/MG_Backend/DirectGLES/Utils.h | 14 +- .../DirectVulkan/Renderer/PipelineFactory.cpp | 2 + .../DirectVulkan/Renderer/PipelineFactory.h | 11 +- .../DirectVulkan/Renderer/ProgramFactory.cpp | 83 +++++++++--- .../DirectVulkan/Renderer/ProgramFactory.h | 43 ++++-- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 28 +++- .../MG_Impl/GLImpl/Drawing/GL_Drawing.cpp | 31 +++++ MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.h | 1 + .../MG_Impl/GLImpl/Exporting/Definitions.cpp | 2 +- MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp | 35 +++++ MobileGL/MG_State/GLState/Core.cpp | 16 +++ MobileGL/MG_State/GLState/Core.h | 4 + .../GLState/RenderState/RenderState.cpp | 25 ++++ .../GLState/RenderState/RenderState.h | 11 ++ .../Backend/DirectGLES/EsslShaderPassTest.cpp | 54 +++++++- .../Pipeline/PassthroughTessControlTest.cpp | 41 +++++- MobileGL/MG_Test/State/RenderStateTest.cpp | 126 ++++++++++++++++++ 21 files changed, 546 insertions(+), 64 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index a53752ff..1a2b7003 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -2461,9 +2461,19 @@ namespace MobileGL::MG_Backend::DirectGLES { // `layout(vertices = N) out` - so a glPatchParameteri between two draws makes the // built program wrong. -1 is "this program needed no such stage", which compares // equal to itself and costs every other program one integer test. + // + // GL_PATCH_DEFAULT_{OUTER,INNER}_LEVEL are baked into the same stage for the same + // reason (ES has neither the state nor an entry point), so glPatchParameterfv + // makes it stale too. Both level comparisons sit INSIDE the >= 0 guard: a program + // with a control stage of its own - which is nearly all of them - still pays only + // the one integer test. (twin->GetPassthroughTessControlPatchVertices() >= 0 && - twin->GetPassthroughTessControlPatchVertices() != - static_cast(MG_State::pGLContext->GetPatchVertices()))) { + (twin->GetPassthroughTessControlPatchVertices() != + static_cast(MG_State::pGLContext->GetPatchVertices()) || + twin->GetPassthroughTessControlOuterLevel() != + MG_State::pGLContext->GetPatchDefaultOuterLevel() || + twin->GetPassthroughTessControlInnerLevel() != + MG_State::pGLContext->GetPatchDefaultInnerLevel()))) { twin->SyncToBackend(currentProgram); } g_currentDrawFrontendProgram = currentProgram.get(); diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index dec11a94..7f38ff62 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -6730,6 +6730,16 @@ namespace MobileGL::MG_Backend::DirectGLES { ? MG_State::pGLContext->GetPatchVertices() : 3u; m_passthroughTessControlPatchVertices = static_cast(patchVertices); + // PATCH_DEFAULT_{OUTER,INNER}_LEVEL are the same kind of dynamic state and are baked + // into the same stage (ES has no such state and no entry point to forward them to), so + // they are recorded and compared alongside the patch size - the two move together, as + // BuildPassthroughTessControlEssl's contract says. + m_passthroughTessControlOuterLevel = MG_State::pGLContext != nullptr + ? MG_State::pGLContext->GetPatchDefaultOuterLevel() + : FloatVec4(1.0f, 1.0f, 1.0f, 1.0f); + m_passthroughTessControlInnerLevel = MG_State::pGLContext != nullptr + ? MG_State::pGLContext->GetPatchDefaultInnerLevel() + : FloatVec2(1.0f, 1.0f); if (tessEvalShaderIndex < 0 || static_cast(tessEvalShaderIndex) >= shaderSpirvs.size()) { @@ -6770,8 +6780,10 @@ namespace MobileGL::MG_Backend::DirectGLES { const String outMembers = ExtractPerVertexBlockMembers(tessEvalStageEssl, /*input=*/true).value_or(String()); - const String source = - BuildPassthroughTessControlEssl(ResolveBackendEsslVersion(), patchVertices, inMembers, outMembers); + const String source = BuildPassthroughTessControlEssl(ResolveBackendEsslVersion(), patchVertices, + inMembers, outMembers, + m_passthroughTessControlOuterLevel, + m_passthroughTessControlInnerLevel); const GLuint backendShaderId = g_GLESFuncs.glCreateShader(GL_TESS_CONTROL_SHADER); if (backendShaderId == 0) { @@ -6861,8 +6873,11 @@ namespace MobileGL::MG_Backend::DirectGLES { m_atomicCounterEsslBindingTop = AtomicCounterEsslBindingTop(); // Re-established by AttachPassthroughTessControlStage below when this program needs // one; cleared first so a program that stops needing one (a relink that now attaches - // a real control stage) does not keep comparing against a stale patch size. + // a real control stage) does not keep comparing against a stale patch size. The + // default levels are re-established from the same call and gated on the same -1. m_passthroughTessControlPatchVertices = -1; + m_passthroughTessControlOuterLevel = FloatVec4(1.0f, 1.0f, 1.0f, 1.0f); + m_passthroughTessControlInnerLevel = FloatVec2(1.0f, 1.0f); // The same shape again for image FORMATS: what a format-less image declaration // compiles to depends on live glBindImageTexture state, so the pairs it was built // against are recorded here and compared per draw (ImageUnitFormatsStillMatch). diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 79af2709..f100ebdb 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -1490,6 +1490,17 @@ namespace MobileGL::MG_Backend::DirectGLES { Int GetPassthroughTessControlPatchVertices() const { return m_passthroughTessControlPatchVertices; } + // GL_PATCH_DEFAULT_{OUTER,INNER}_LEVEL the same synthesized stage was built with, for + // the same reason: ES has neither the state nor an entry point to forward it to, so + // glPatchParameterfv's values are compiled in as literals and a program built with one + // set is stale for another. Meaningless (and never read) when the patch-vertices field + // above is -1, which is the gate the draw path tests first. + const FloatVec4& GetPassthroughTessControlOuterLevel() const { + return m_passthroughTessControlOuterLevel; + } + const FloatVec2& GetPassthroughTessControlInnerLevel() const { + return m_passthroughTessControlInnerLevel; + } Bool HasGlobalUboBlock() const { return m_globalUboBackendBlockIndex >= 0; } const Vector& GetUniformBlockBackendIndices() const { return m_uniformBlockBackendIndices; } @@ -1590,6 +1601,10 @@ namespace MobileGL::MG_Backend::DirectGLES { // all); otherwise the GL_PATCH_VERTICES the synthesized pass-through stage was built // with. See GetPassthroughTessControlPatchVertices. Int m_passthroughTessControlPatchVertices = -1; + // The default tessellation levels baked into that same stage. Only meaningful while + // the field above is not -1. + FloatVec4 m_passthroughTessControlOuterLevel = FloatVec4(1.0f, 1.0f, 1.0f, 1.0f); + FloatVec2 m_passthroughTessControlInnerLevel = FloatVec2(1.0f, 1.0f); Bool m_isInitialized = false; Bool m_backendProgramUsable = false; // Set by SyncToBackend every time it relinks the driver program, cleared by the diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp index 57669337..2ca43e94 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Utils.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include namespace MobileGL::MG_Backend::DirectGLES { @@ -834,9 +835,25 @@ namespace MobileGL::MG_Backend::DirectGLES { return std::nullopt; } + namespace { + // A GLSL float literal for a tessellation level. Always spelled with a decimal point, + // because an integral value written without one is an INT literal and + // `gl_TessLevelOuter[0] = 1;` does not compile. A non-finite value is baked as 0.0: + // glPatchParameterfv accepts any float, and a level that is not a positive number + // discards the patch - which is what a NaN level does in GL too - whereas emitting + // "nan" would make the synthesized stage fail to compile and take the whole program + // down with it. + String TessLevelLiteral(Float value) { + if (!std::isfinite(value)) return "0.0"; + return std::format("{:.6f}", value); + } + } // namespace + String BuildPassthroughTessControlEssl(const Uint esslVersion, const Uint patchVertices, const String& inPerVertexMembers, - const String& outPerVertexMembers) { + const String& outPerVertexMembers, + const FloatVec4& defaultOuterLevel, + const FloatVec2& defaultInnerLevel) { #ifdef TRACY_ENABLE ZoneScopedC(TRACY_ZONECOLOR_BACKEND); #endif @@ -866,12 +883,14 @@ namespace MobileGL::MG_Backend::DirectGLES { // was declined before this was ever called (ModuleReadsLocatedInput), and gl_PointSize // from a tessellation stage is a separate capability on both targets. source += " gl_out[gl_InvocationID].gl_Position = gl_in[gl_InvocationID].gl_Position;\n"; - source += " gl_TessLevelOuter[0] = 1.0;\n"; - source += " gl_TessLevelOuter[1] = 1.0;\n"; - source += " gl_TessLevelOuter[2] = 1.0;\n"; - source += " gl_TessLevelOuter[3] = 1.0;\n"; - source += " gl_TessLevelInner[0] = 1.0;\n"; - source += " gl_TessLevelInner[1] = 1.0;\n"; + for (Uint i = 0; i < 4; ++i) { + source += " gl_TessLevelOuter[" + std::to_string(i) + + "] = " + TessLevelLiteral(defaultOuterLevel[i]) + ";\n"; + } + for (Uint i = 0; i < 2; ++i) { + source += " gl_TessLevelInner[" + std::to_string(i) + + "] = " + TessLevelLiteral(defaultInnerLevel[i]) + ";\n"; + } source += "}\n"; return source; } diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.h b/MobileGL/MG_Backend/DirectGLES/Utils.h index dadeea0a..c7239006 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.h +++ b/MobileGL/MG_Backend/DirectGLES/Utils.h @@ -368,11 +368,11 @@ namespace MobileGL::MG_Backend::DirectGLES { // // All four outer levels and both inner levels are written unconditionally: writing a // level the evaluation stage's domain does not use is legal and ignored, and it saves - // this from having to know the domain. They are literal 1.0 because that is the GL - // default and glPatchParameterfv - their only setter - is a stub in this frontend - // (MG_Impl/GLImpl/Exporting/Definitions.cpp). Implementing that entry point means making - // the levels a parameter here AND part of what makes a built program stale, exactly as - // PATCH_VERTICES already is; the two must move together, so they are named together. + // this from having to know the domain. They are the GL_PATCH_DEFAULT_OUTER_LEVEL / + // GL_PATCH_DEFAULT_INNER_LEVEL state, baked in as literals - ES has no such state and no + // glPatchParameterfv to forward to, so compiling them in is the only way to honour them. + // That makes them part of what a built program is stale against, exactly as PATCH_VERTICES + // is: see the staleness clause in DirectGLES.cpp's SyncCurrentProgram, which compares both. // // The same stage, for the same reason, that DirectVulkan synthesizes in // ProgramFactory::BuildPassthroughTessControlSource - Vulkan likewise requires both @@ -382,7 +382,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // VkShaderModule against a driver shader object. String BuildPassthroughTessControlEssl(Uint esslVersion, Uint patchVertices, const String& inPerVertexMembers, - const String& outPerVertexMembers); + const String& outPerVertexMembers, + const FloatVec4& defaultOuterLevel, + const FloatVec2& defaultInnerLevel); // Prefix of the writeonly half a read+write image uniform is split into (see // SplitReadWriteImageUniforms); the suffix is the image's own (already access-tagged) name. constexpr const char* IMAGE_WRITE_ALIAS_PREFIX = "mg_imageWrite_"; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp index 44a46ec6..c4b1d230 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp @@ -206,6 +206,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { XXHASH_VERIFY( XXH64_update(m_hashState, &payload.primitiveRestartEnable, sizeof(payload.primitiveRestartEnable))); XXHASH_VERIFY(XXH64_update(m_hashState, &payload.patchControlPoints, sizeof(payload.patchControlPoints))); + XXHASH_VERIFY(XXH64_update(m_hashState, &payload.passthroughTessControlKey, + sizeof(payload.passthroughTessControlKey))); XXHASH_VERIFY(XXH64_update(m_hashState, &payload.viewportCount, sizeof(payload.viewportCount))); XXHASH_VERIFY(XXH64_update(m_hashState, &payload.polygonMode, sizeof(payload.polygonMode))); XXHASH_VERIFY(XXH64_update(m_hashState, &payload.cullMode, sizeof(payload.cullMode))); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h index cf846a3d..a0f7f0c5 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h @@ -42,6 +42,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { Bool primitiveRestartEnable = false; // GL_PATCH_VERTICES; only read for a PATCH_LIST topology. Uint32 patchControlPoints = 3; + // ProgramFactory::ComputePassthroughTessControlKey of the synthesized pass-through + // tessellation control stage below, or 0 when this pipeline has none. Hashed, because + // the levels glPatchParameterfv set are compiled INTO that module and are not a + // function of the program or of patchControlPoints - see the note on + // passthroughTessControlStage. + Uint64 passthroughTessControlKey = 0; // How many of ARB_viewport_array's viewports this pipeline rasterizes into. 1 for // every program that never assigns gl_ViewportIndex, which is all of them outside the // conformance suite - the wide shape costs a longer vkCmdSetViewport/Scissor per state @@ -87,8 +93,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { // renderer could not build one, and CreatePipeline refuses the pipeline - the same // refusal it applies when `stages` itself is half-tessellated. // - // NOT hashed: it is a pure function of the program and of patchControlPoints, both - // of which ComputeHash already mixes in. + // NOT hashed directly: it is a pure function of the program, of patchControlPoints and + // of the default tessellation levels - the first two of which ComputeHash already + // mixes in, and the third of which arrives through passthroughTessControlKey above. VkPipelineShaderStageCreateInfo passthroughTessControlStage{}; const VkPipelineVertexInputStateCreateInfo* vertexInputState = nullptr; // Diagnostic only; may be null. Read solely from the pipeline-creation failure path. diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp index dfdcebcb..64c2bd59 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp @@ -13,7 +13,10 @@ #include "MG_Util/ShaderTranspiler/SpvcSession.h" #include "MG_Util/ShaderTranspiler/Types.h" #include +#include +#include #include +#include #include #include #include @@ -3594,18 +3597,48 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } - String ProgramFactory::BuildPassthroughTessControlSource(Uint32 patchVertices) { + namespace { + // A GLSL float literal for a tessellation level. Always spelled with a decimal point, + // because an integral value written without one is an INT literal and + // `gl_TessLevelOuter[0] = 1;` does not compile. A non-finite value is baked as 0.0: + // glPatchParameterfv accepts any float, and a level that is not a positive number + // discards the patch - which is what a NaN level does in GL too - whereas emitting "nan" + // would make the synthesized stage fail to compile and take the whole program down. + String TessLevelLiteral(Float value) { + if (!std::isfinite(value)) return "0.0"; + return std::format("{:.6f}", value); + } + } // namespace + + Uint64 ProgramFactory::ComputePassthroughTessControlKey(Uint32 patchVertices, + const FloatVec4& defaultOuterLevel, + const FloatVec2& defaultInnerLevel) { + // A plain 28-byte blob of exactly what the generator reads, hashed once. Deliberately over + // the RAW BITS rather than the values: two levels that compare unequal must key apart, and + // a NaN level - which glPatchParameterfv accepts - compares unequal to itself. + struct Blob { + Uint32 patchVertices; + Uint32 outerBits[4]; + Uint32 innerBits[2]; + } blob{}; + blob.patchVertices = patchVertices; + for (Uint32 i = 0; i < 4; ++i) blob.outerBits[i] = std::bit_cast(defaultOuterLevel[i]); + for (Uint32 i = 0; i < 2; ++i) blob.innerBits[i] = std::bit_cast(defaultInnerLevel[i]); + return XXH64(&blob, sizeof(blob), 0); + } + + String ProgramFactory::BuildPassthroughTessControlSource(Uint32 patchVertices, + const FloatVec4& defaultOuterLevel, + const FloatVec2& defaultInnerLevel) { // The stage GL 4.6 core 11.2.2 describes when a program has an evaluation shader and no // control shader: "the input patch is passed through unmodified", the output patch has // as many vertices as the input one (PATCH_VERTICES), and the levels come from the // PATCH_DEFAULT_OUTER_LEVEL / PATCH_DEFAULT_INNER_LEVEL state. // - // Those two levels default to 1.0 and are baked here as literals because - // glPatchParameterfv - their only setter - is not implemented in this frontend (it is a - // stub in MG_Impl/GLImpl/Exporting/Definitions.cpp). Implementing that entry point means - // making the levels a parameter of this source AND of the cache key in - // GetOrCreatePassthroughTessControlStage; the two must move together, so they are named - // together here. + // Those two levels are baked in as literals - Vulkan has no equivalent dynamic state, so + // compiling them in is the only way to honour glPatchParameterfv. That makes them part of + // this module's identity: GetOrCreatePassthroughTessControlStage keys its cache on them, + // and PipelineFactory hashes them into the pipeline key. The three must move together. // // gl_out carries gl_Position and nothing else on purpose. The evaluation stage that // reads it was linked against the VERTEX stage directly, so its input gl_PerVertex holds @@ -3648,20 +3681,28 @@ namespace MobileGL::MG_Backend::DirectVulkan { "} gl_out[];\n"; source += "void main() {\n"; source += " gl_out[gl_InvocationID].gl_Position = gl_in[gl_InvocationID].gl_Position;\n"; - source += " gl_TessLevelOuter[0] = 1.0;\n"; - source += " gl_TessLevelOuter[1] = 1.0;\n"; - source += " gl_TessLevelOuter[2] = 1.0;\n"; - source += " gl_TessLevelOuter[3] = 1.0;\n"; - source += " gl_TessLevelInner[0] = 1.0;\n"; - source += " gl_TessLevelInner[1] = 1.0;\n"; + for (Uint32 i = 0; i < 4; ++i) { + source += " gl_TessLevelOuter[" + std::to_string(i) + + "] = " + TessLevelLiteral(defaultOuterLevel[i]) + ";\n"; + } + for (Uint32 i = 0; i < 2; ++i) { + source += " gl_TessLevelInner[" + std::to_string(i) + + "] = " + TessLevelLiteral(defaultInnerLevel[i]) + ";\n"; + } source += "}\n"; return source; } - VkPipelineShaderStageCreateInfo ProgramFactory::GetOrCreatePassthroughTessControlStage(Uint32 patchVertices) { + VkPipelineShaderStageCreateInfo ProgramFactory::GetOrCreatePassthroughTessControlStage( + Uint32 patchVertices, const FloatVec4& defaultOuterLevel, const FloatVec2& defaultInnerLevel) { + // Everything compiled into the stage, folded into one key. The patch size alone stopped + // being enough once glPatchParameterfv could change the levels: two modules that differ + // only in a baked-in level are different modules, and pipelines built from either may be + // alive at the same time. + const Uint64 key = ComputePassthroughTessControlKey(patchVertices, defaultOuterLevel, defaultInnerLevel); // A cached VK_NULL_HANDLE is a remembered failure, not a miss: returning it keeps a // generator that cannot compile from re-running glslang on every draw. - const auto cached = m_passthroughTessControlStages.find(patchVertices); + const auto cached = m_passthroughTessControlStages.find(key); if (cached != m_passthroughTessControlStages.end()) { return cached->second; } @@ -3672,7 +3713,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { stage.pName = "main"; using namespace MG_Util::ShaderTranspiler; - const String source = BuildPassthroughTessControlSource(patchVertices); + const String source = BuildPassthroughTessControlSource(patchVertices, defaultOuterLevel, defaultInnerLevel); // Same compile configuration as every other stage of every other program: this runs on // the GL thread (the draw path), so the live compile env is the right one, and flags=0 // is the Vulkan-targeting form (CompileForOpenGL is what the GLES backend adds). @@ -3686,7 +3727,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { MGLOG_E("ProgramFactory: could not compile the pass-through tessellation control stage for " "patchVertices=%u; a program with an evaluation stage and no control stage cannot draw. %s", patchVertices, compiled.error().log.c_str()); - m_passthroughTessControlStages.emplace(patchVertices, stage); + m_passthroughTessControlStages.emplace(key, stage); return stage; } @@ -3696,7 +3737,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { if (!linked) { MGLOG_E("ProgramFactory: could not link the pass-through tessellation control stage for " "patchVertices=%u. %s", patchVertices, linked.error().log.c_str()); - m_passthroughTessControlStages.emplace(patchVertices, stage); + m_passthroughTessControlStages.emplace(key, stage); return stage; } @@ -3705,7 +3746,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { if (!binary || binary.value().empty() || binary.value().front().empty()) { MGLOG_E("ProgramFactory: could not generate SPIR-V for the pass-through tessellation control stage " "for patchVertices=%u", patchVertices); - m_passthroughTessControlStages.emplace(patchVertices, stage); + m_passthroughTessControlStages.emplace(key, stage); return stage; } @@ -3727,14 +3768,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { if (result != VK_SUCCESS) { MGLOG_E("ProgramFactory: vkCreateShaderModule failed (%d) for the pass-through tessellation control " "stage for patchVertices=%u", static_cast(result), patchVertices); - m_passthroughTessControlStages.emplace(patchVertices, stage); + m_passthroughTessControlStages.emplace(key, stage); return stage; } stage.module = module; MGLOG_D("ProgramFactory: built the pass-through tessellation control stage for patchVertices=%u " "(GL 4.6 11.2.2; Vulkan has no fixed-function equivalent)", patchVertices); - m_passthroughTessControlStages.emplace(patchVertices, stage); + m_passthroughTessControlStages.emplace(key, stage); return stage; } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h index 79700e38..b8a594d9 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h @@ -485,18 +485,30 @@ namespace MobileGL::MG_Backend::DirectVulkan { // the caller then has no control stage to inject, and CreatePipeline refuses the // pipeline rather than handing the driver a half-tessellated one. // - // Keyed on the patch size because GL takes the output patch size from PATCH_VERTICES, - // which is draw state, not link state - the CTS case that motivated this links at the - // default 3 and draws at 4. The pipeline cache already re-keys on patchControlPoints, - // so the module a pipeline was built with is part of that pipeline's identity. - // Compiling is bounded by the number of distinct patch sizes a program draws with - // (MAX_PATCH_VERTICES = 32 in the worst case, one or two in practice) and only ever - // happens for the rare program that has no control stage at all. - VkPipelineShaderStageCreateInfo GetOrCreatePassthroughTessControlStage(Uint32 patchVertices); + // Keyed on the patch size AND the six default tessellation levels, because GL takes the + // output patch size from PATCH_VERTICES and the levels from PATCH_DEFAULT_OUTER_LEVEL / + // PATCH_DEFAULT_INNER_LEVEL, all of which are draw state rather than link state - the CTS + // case that motivated this links at the default 3 and draws at 4. The pipeline cache + // re-keys on the same three inputs, so the module a pipeline was built with is part of + // that pipeline's identity. Compiling is bounded by the number of distinct + // (size, levels) combinations a program draws with - one or two in practice - and only + // ever happens for the rare program that has no control stage at all. + VkPipelineShaderStageCreateInfo GetOrCreatePassthroughTessControlStage(Uint32 patchVertices, + const FloatVec4& defaultOuterLevel, + const FloatVec2& defaultInnerLevel); // Source of the module above. Exposed for tests: the generated GLSL is the whole // contract with the evaluation stage, so it is worth pinning independently of a device. - static String BuildPassthroughTessControlSource(Uint32 patchVertices); + static String BuildPassthroughTessControlSource(Uint32 patchVertices, const FloatVec4& defaultOuterLevel, + const FloatVec2& defaultInnerLevel); + + // The identity of one such module: everything the generator bakes in, folded into a + // 64-bit key over the raw bits (so -0.0 and +0.0 key apart, which is harmless, and NaN + // keys to itself, which is what matters). Shared with PipelineFactory, which mixes the + // same value into the pipeline hash so a pipeline can never be handed a module built for + // different levels. + static Uint64 ComputePassthroughTessControlKey(Uint32 patchVertices, const FloatVec4& defaultOuterLevel, + const FloatVec2& defaultInnerLevel); private: struct ProgramLookupCache { @@ -556,11 +568,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { // See GetCacheStructureEpoch(). Starts at 1 so a zero-initialized memo can never match. Uint64 m_cacheStructureEpoch = 1; IEvictionObserver* m_evictionObserver = nullptr; - // Pass-through tessellation control stages by input patch size. Never evicted: at most - // MAX_PATCH_VERTICES entries exist for the lifetime of the device, and every pipeline - // ever built from one keeps referencing its module. A failed build is cached as - // VK_NULL_HANDLE so a broken generator costs one compile, not one per draw. - UnorderedMap m_passthroughTessControlStages; + // Pass-through tessellation control stages by the identity of what was compiled into + // them - the input patch size and the six default tessellation levels, folded into one + // 64-bit key by PassthroughTessControlKey below (the levels are float state, so the map + // cannot simply be keyed on the patch size any more). Never evicted: a handful of entries + // exist for the lifetime of the device, and every pipeline ever built from one keeps + // referencing its module. A failed build is cached as VK_NULL_HANDLE so a broken generator + // costs one compile, not one per draw. + UnorderedMap m_passthroughTessControlStages; static inline XXH64_state_t* m_hashState = XXH64_createState(); }; } // namespace MobileGL::MG_Backend::DirectVulkan diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index a0dd8e4f..ba6cfd4f 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -30,6 +30,7 @@ #include "MG_Util/Texture/PixelStoreProcessor.h" #include #include +#include #include #include #include @@ -4735,6 +4736,20 @@ void main() { capabilityBits |= p.DepthMask ? 1ull << 8 : 0; Uint64 hash = CombinePipelineStateWord(0x243F6A8885A308D3ull, capabilityBits); hash = CombinePipelineStateWord(hash, static_cast(p.PatchVertices)); + // The default tessellation levels belong here for the same reason PatchVertices does: + // when a program has an evaluation stage and no control stage, both are compiled into the + // synthesized pass-through control stage, so two draws that differ only in a level need + // different pipelines. Hashed over the RAW BITS so a NaN level - which glPatchParameterfv + // accepts - keys to itself. Six extra words on a path that only recomputes when the + // pipeline-state version moved. + for (Uint32 i = 0; i < 4; ++i) { + hash = CombinePipelineStateWord(hash, + static_cast(std::bit_cast(p.PatchDefaultOuterLevel[i]))); + } + for (Uint32 i = 0; i < 2; ++i) { + hash = CombinePipelineStateWord(hash, + static_cast(std::bit_cast(p.PatchDefaultInnerLevel[i]))); + } hash = CombinePipelineStateWord(hash, static_cast(p.PolygonModeFront)); hash = CombinePipelineStateWord(hash, static_cast(p.CullFaceModeSetting)); hash = CombinePipelineStateWord(hash, static_cast(p.DepthFunc)); @@ -5116,10 +5131,19 @@ void main() { // program with a tessellation stage may only be drawn with GL_PATCHES), so nothing legal // loses its pass-through here; what it does lose is the pipeline, because the refusal // below then sees an evaluation stage with no control stage and declines. + // + // The default tessellation levels (glPatchParameterfv) are draw state for the same reason + // and are compiled into the same module, so they are read here too and their key is mixed + // into the pipeline hash - without that a pipeline memoised at one set of levels would be + // handed back after the application changed them. if (programObj.needsPassthroughTessControl && programObj.passthroughTessControlEmulatable && vkTopology == VK_PRIMITIVE_TOPOLOGY_PATCH_LIST) { - payload.passthroughTessControlStage = - m_programFactory->GetOrCreatePassthroughTessControlStage(payload.patchControlPoints); + const FloatVec4& defaultOuterLevel = MG_State::pGLContext->GetPatchDefaultOuterLevel(); + const FloatVec2& defaultInnerLevel = MG_State::pGLContext->GetPatchDefaultInnerLevel(); + payload.passthroughTessControlKey = ProgramFactory::ComputePassthroughTessControlKey( + payload.patchControlPoints, defaultOuterLevel, defaultInnerLevel); + payload.passthroughTessControlStage = m_programFactory->GetOrCreatePassthroughTessControlStage( + payload.patchControlPoints, defaultOuterLevel, defaultInnerLevel); } if (!payload.stencilTestEnable) { payload.frontStencilFailOp = VK_STENCIL_OP_KEEP; diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 10b6103e..685892ec 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -713,6 +713,37 @@ namespace MobileGL::MG_Impl::GLImpl { } } + // GL 4.6 core 11.2.2. The default tessellation levels a program with an evaluation stage and + // NO control stage tessellates at; both backends have to synthesize that control stage + // themselves (ES 3.2 and Vulkan both require one), and they compile these numbers into it, so + // there is no backend entry point to forward to - ES has none at all. INVALID_ENUM on a bad + // pname is the only error the spec lists: any float values are accepted, negatives and NaN + // included, and it is the tessellator that clamps them. + // + // This used to be a stub, which is why the two synthesizers hardcoded 1.0. + void PatchParameterfv(GLenum pname, const GLfloat* values) { + if (pname != GL_PATCH_DEFAULT_OUTER_LEVEL && pname != GL_PATCH_DEFAULT_INNER_LEVEL) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique( + "MG_Impl/GLImpl", __func__, + "pname must be GL_PATCH_DEFAULT_OUTER_LEVEL or GL_PATCH_DEFAULT_INNER_LEVEL.")); + return; + } + if (!values) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", __func__, "values pointer cannot be null")); + return; + } + if (pname == GL_PATCH_DEFAULT_OUTER_LEVEL) { + MG_State::pGLContext->SetPatchDefaultOuterLevel( + FloatVec4(values[0], values[1], values[2], values[3])); + } else { + MG_State::pGLContext->SetPatchDefaultInnerLevel(FloatVec2(values[0], values[1])); + } + } + namespace { // GL 4.6 core 7.11.2 (and ARB_shader_image_load_store, which introduced the call): the // barrier bitfield is INVALID_VALUE unless every bit is one of the defined ones, with diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.h b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.h index e9228610..46be0bed 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.h +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.h @@ -32,6 +32,7 @@ namespace MobileGL::MG_Impl::GLImpl { void DispatchCompute(GLuint numGroupsX, GLuint numGroupsY, GLuint numGroupsZ); void DispatchComputeIndirect(GLintptr indirect); void PatchParameteri(GLenum pname, GLint value); + void PatchParameterfv(GLenum pname, const GLfloat* values); void MemoryBarrier(GLbitfield barriers); void MemoryBarrierByRegion(GLbitfield barriers); void MultiDrawElementsIndirect(GLenum mode, GLenum type, const void* indirect, GLsizei drawcount, GLsizei stride); diff --git a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp index e9c3cd48..94bc5aab 100644 --- a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp +++ b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp @@ -923,7 +923,7 @@ DECLARE_GL_FUNCTION_STUB_HEAD(void, GetActiveSubroutineName, GLuint program, GLe DECLARE_GL_FUNCTION_STUB_HEAD(void, UniformSubroutinesuiv, GLenum shadertype, GLsizei count, const GLuint* indices) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, UniformSubroutinesuiv, shadertype, count, indices) DECLARE_GL_FUNCTION_STUB_HEAD(void, GetUniformSubroutineuiv, GLenum shadertype, GLint location, GLuint* params) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, GetUniformSubroutineuiv, shadertype, location, params) DECLARE_GL_FUNCTION_STUB_HEAD(void, GetProgramStageiv, GLuint program, GLenum shadertype, GLenum pname, GLint* values) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, GetProgramStageiv, program, shadertype, pname, values) -DECLARE_GL_FUNCTION_STUB_HEAD(void, PatchParameterfv, GLenum pname, const GLfloat* values) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, PatchParameterfv, pname, values) +DECLARE_GL_FUNCTION_HEAD(void, PatchParameterfv, GLenum pname, const GLfloat* values) DECLARE_GL_FUNCTION_END_NO_RETURN(void, PatchParameterfv, pname, values) DECLARE_GL_FUNCTION_HEAD(void, DrawTransformFeedback, GLenum mode, GLuint id) DECLARE_GL_FUNCTION_END_NO_RETURN(void, DrawTransformFeedback, mode, id) DECLARE_GL_FUNCTION_HEAD(void, DrawTransformFeedbackStream, GLenum mode, GLuint id, GLuint stream) DECLARE_GL_FUNCTION_END_NO_RETURN(void, DrawTransformFeedbackStream, mode, id, stream) DECLARE_GL_FUNCTION_HEAD(void, BeginQueryIndexed, GLenum target, GLuint index, GLuint id) DECLARE_GL_FUNCTION_END_NO_RETURN(void, BeginQueryIndexed, target, index, id) diff --git a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp index a7f0c1b3..e2549c0e 100644 --- a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp +++ b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp @@ -695,8 +695,12 @@ namespace MobileGL::MG_Impl::GLImpl { switch (pname) { case GL_COLOR_WRITEMASK: case GL_SCISSOR_BOX: + case GL_PATCH_DEFAULT_OUTER_LEVEL: CopyIntsToBooleans(ints, 4, params); return; + case GL_PATCH_DEFAULT_INNER_LEVEL: + CopyIntsToBooleans(ints, 2, params); + return; default: *params = ints[0] ? GL_TRUE : GL_FALSE; return; @@ -735,6 +739,22 @@ namespace MobileGL::MG_Impl::GLImpl { params[1] = depthRange.y(); return; } + // glPatchParameterfv's two states. Float-native, so they are answered here rather than + // through the integer fallback below - which rounds, and would report 0 for a level of 0.5. + case GL_PATCH_DEFAULT_OUTER_LEVEL: { + const FloatVec4& outer = MG_State::pGLContext->GetPatchDefaultOuterLevel(); + params[0] = outer.x(); + params[1] = outer.y(); + params[2] = outer.z(); + params[3] = outer.w(); + return; + } + case GL_PATCH_DEFAULT_INNER_LEVEL: { + const FloatVec2& inner = MG_State::pGLContext->GetPatchDefaultInnerLevel(); + params[0] = inner.x(); + params[1] = inner.y(); + return; + } case GL_VIEWPORT_BOUNDS_RANGE: { const auto& dynamicParameters = MG_Backend::pActiveBackendObject->GetDynamicParameters(); params[0] = dynamicParameters.ViewportBoundsRangeMin; @@ -1268,6 +1288,7 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_POINT_SIZE_RANGE: case GL_SMOOTH_LINE_WIDTH_RANGE: case GL_MAX_VIEWPORT_DIMS: + case GL_PATCH_DEFAULT_INNER_LEVEL: count = 2; break; case GL_BLEND_COLOR: @@ -1275,6 +1296,7 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_VIEWPORT: case GL_SCISSOR_BOX: case GL_COLOR_WRITEMASK: + case GL_PATCH_DEFAULT_OUTER_LEVEL: count = 4; break; default: @@ -2287,6 +2309,19 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_PATCH_VERTICES: *params = static_cast(MG_State::pGLContext->GetPatchVertices()); break; + // Float state, so glGetIntegerv rounds it (GL 4.6 core 2.2.2) - the exact values come back + // through glGetFloatv. Answered here so glGetBooleanv, which delegates to this getter for + // everything its own switch does not handle, does not report INVALID_ENUM for them. + case GL_PATCH_DEFAULT_OUTER_LEVEL: { + const FloatVec4& outer = MG_State::pGLContext->GetPatchDefaultOuterLevel(); + for (Uint i = 0; i < 4; ++i) params[i] = static_cast(std::lround(outer[i])); + break; + } + case GL_PATCH_DEFAULT_INNER_LEVEL: { + const FloatVec2& inner = MG_State::pGLContext->GetPatchDefaultInnerLevel(); + for (Uint i = 0; i < 2; ++i) params[i] = static_cast(std::lround(inner[i])); + break; + } case GL_MAX_PATCH_VERTICES: *params = dynamicParameters.MaxPatchVertices; break; diff --git a/MobileGL/MG_State/GLState/Core.cpp b/MobileGL/MG_State/GLState/Core.cpp index a13e351d..44ff3579 100644 --- a/MobileGL/MG_State/GLState/Core.cpp +++ b/MobileGL/MG_State/GLState/Core.cpp @@ -812,6 +812,22 @@ namespace MobileGL::MG_State { m_renderState.SetPatchVertices(vertices); } + void GLContext::SetPatchDefaultOuterLevel(const FloatVec4& levels) { + m_renderState.SetPatchDefaultOuterLevel(levels); + } + + const FloatVec4& GLContext::GetPatchDefaultOuterLevel() const { + return m_renderState.GetPatchDefaultOuterLevel(); + } + + void GLContext::SetPatchDefaultInnerLevel(const FloatVec2& levels) { + m_renderState.SetPatchDefaultInnerLevel(levels); + } + + const FloatVec2& GLContext::GetPatchDefaultInnerLevel() const { + return m_renderState.GetPatchDefaultInnerLevel(); + } + Uint GLContext::GetPatchVertices() const { return m_renderState.GetPatchVertices(); } diff --git a/MobileGL/MG_State/GLState/Core.h b/MobileGL/MG_State/GLState/Core.h index 2e6cfada..4b32f5ab 100644 --- a/MobileGL/MG_State/GLState/Core.h +++ b/MobileGL/MG_State/GLState/Core.h @@ -213,6 +213,10 @@ namespace MobileGL { Float GetPointSize() const; void SetPatchVertices(Uint vertices); Uint GetPatchVertices() const; + void SetPatchDefaultOuterLevel(const FloatVec4& levels); + const FloatVec4& GetPatchDefaultOuterLevel() const; + void SetPatchDefaultInnerLevel(const FloatVec2& levels); + const FloatVec2& GetPatchDefaultInnerLevel() const; void SetPolygonOffset(Float factor, Float units); Float GetPolygonOffsetFactor() const; Float GetPolygonOffsetUnits() const; diff --git a/MobileGL/MG_State/GLState/RenderState/RenderState.cpp b/MobileGL/MG_State/GLState/RenderState/RenderState.cpp index 8a394c37..135aa1e1 100644 --- a/MobileGL/MG_State/GLState/RenderState/RenderState.cpp +++ b/MobileGL/MG_State/GLState/RenderState/RenderState.cpp @@ -217,6 +217,31 @@ namespace MobileGL { return m_parameters.PatchVertices; } + // BumpVersions(), not just ++m_version, for the same reason SetPatchVertices does it: + // these levels are compiled INTO the synthesized pass-through tessellation control + // stage on both backends, so changing one makes an already-built program stale. + void RenderState::SetPatchDefaultOuterLevel(const FloatVec4& levels) { + if (m_parameters.PatchDefaultOuterLevel == levels) return; + + m_parameters.PatchDefaultOuterLevel = levels; + BumpVersions(); + } + + const FloatVec4& RenderState::GetPatchDefaultOuterLevel() const { + return m_parameters.PatchDefaultOuterLevel; + } + + void RenderState::SetPatchDefaultInnerLevel(const FloatVec2& levels) { + if (m_parameters.PatchDefaultInnerLevel == levels) return; + + m_parameters.PatchDefaultInnerLevel = levels; + BumpVersions(); + } + + const FloatVec2& RenderState::GetPatchDefaultInnerLevel() const { + return m_parameters.PatchDefaultInnerLevel; + } + void RenderState::SetPolygonOffset(Float factor, Float units) { if (m_parameters.PolygonOffsetFactor == factor && m_parameters.PolygonOffsetUnits == units) return; diff --git a/MobileGL/MG_State/GLState/RenderState/RenderState.h b/MobileGL/MG_State/GLState/RenderState/RenderState.h index 05e90b24..589d0644 100644 --- a/MobileGL/MG_State/GLState/RenderState/RenderState.h +++ b/MobileGL/MG_State/GLState/RenderState/RenderState.h @@ -240,6 +240,13 @@ namespace MobileGL { Float PointSize = 1.0f; // GL_PATCH_VERTICES: how many vertices one tessellation patch consumes. Uint PatchVertices = 3; + // GL_PATCH_DEFAULT_OUTER_LEVEL / GL_PATCH_DEFAULT_INNER_LEVEL (glPatchParameterfv). The + // tessellation levels used when a program has an evaluation stage and NO control stage - + // GL's fixed-function pass-through (4.6 core 11.2.2). Both backends have to synthesize + // that stage, and they bake these numbers into it, so a change here makes an already-built + // one stale exactly as PATCH_VERTICES does. Default 1.0, per table 23.44. + FloatVec4 PatchDefaultOuterLevel = FloatVec4(1.0f, 1.0f, 1.0f, 1.0f); + FloatVec2 PatchDefaultInnerLevel = FloatVec2(1.0f, 1.0f); Float PolygonOffsetFactor = 0.0f; Float PolygonOffsetUnits = 0.0f; @@ -374,6 +381,10 @@ namespace MobileGL { Float GetPointSize() const; void SetPatchVertices(Uint vertices); Uint GetPatchVertices() const; + void SetPatchDefaultOuterLevel(const FloatVec4& levels); + const FloatVec4& GetPatchDefaultOuterLevel() const; + void SetPatchDefaultInnerLevel(const FloatVec2& levels); + const FloatVec2& GetPatchDefaultInnerLevel() const; void SetPolygonOffset(Float factor, Float units); Float GetPolygonOffsetFactor() const; Float GetPolygonOffsetUnits() const; diff --git a/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp b/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp index e45088ba..cd1292fe 100644 --- a/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp +++ b/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp @@ -15,6 +15,8 @@ #include +#include + using namespace MobileGL; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::BakeImageFormatQualifiers; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::BuildPassthroughTessControlEssl; @@ -1296,8 +1298,13 @@ void main() { gl_ViewportIndex = 1; imageStore(uni_image, ivec2(0), uvec4(1u)); // rather than pick a shape, because a redeclaration that disagrees with the stage it feeds is an // ES link error against a program that has nothing else wrong with it. +namespace { + const FloatVec4 kDefaultOuter(1.0f, 1.0f, 1.0f, 1.0f); + const FloatVec2 kDefaultInner(1.0f, 1.0f); +} // namespace + TEST(PassthroughTessControlEsslTest, DeclaresThePatchSizeAndWritesEveryTessLevel) { - const String out = BuildPassthroughTessControlEssl(320, 4, "", ""); + const String out = BuildPassthroughTessControlEssl(320, 4, "", "", kDefaultOuter, kDefaultInner); EXPECT_EQ(out.find("#version 320 es"), 0u) << out; EXPECT_TRUE(Contains(out, "layout(vertices = 4) out;")) << out; EXPECT_TRUE(Contains(out, @@ -1307,17 +1314,56 @@ TEST(PassthroughTessControlEsslTest, DeclaresThePatchSizeAndWritesEveryTessLevel // legal and ignored, and it saves the generator from having to know the domain. for (const char* level : {"gl_TessLevelOuter[0]", "gl_TessLevelOuter[1]", "gl_TessLevelOuter[2]", "gl_TessLevelOuter[3]", "gl_TessLevelInner[0]", "gl_TessLevelInner[1]"}) { - EXPECT_TRUE(Contains(out, String(level) + " = 1.0;")) << level << "\n" << out; + EXPECT_TRUE(Contains(out, String(level) + " = 1.000000;")) << level << "\n" << out; } // Nothing redeclared when the neighbours redeclared nothing - the driver's own built-in // gl_in/gl_out is then what both sides agree on, and redeclaring is what would break it. EXPECT_FALSE(Contains(out, "gl_PerVertex")) << out; } +// glPatchParameterfv's state is compiled INTO this stage: ES has no PATCH_DEFAULT_*_LEVEL and no +// entry point to forward it to, so a generator that ignored these arguments would tessellate every +// control-stage-less program at level 1 whatever the application asked for. +TEST(PassthroughTessControlEsslTest, BakesTheDefaultTessLevelsIn) { + const String out = BuildPassthroughTessControlEssl(320, 4, "", "", FloatVec4(2.0f, 3.0f, 4.0f, 5.0f), + FloatVec2(6.5f, 7.25f)); + EXPECT_TRUE(Contains(out, "gl_TessLevelOuter[0] = 2.000000;")) << out; + EXPECT_TRUE(Contains(out, "gl_TessLevelOuter[1] = 3.000000;")) << out; + EXPECT_TRUE(Contains(out, "gl_TessLevelOuter[2] = 4.000000;")) << out; + EXPECT_TRUE(Contains(out, "gl_TessLevelOuter[3] = 5.000000;")) << out; + EXPECT_TRUE(Contains(out, "gl_TessLevelInner[0] = 6.500000;")) << out; + EXPECT_TRUE(Contains(out, "gl_TessLevelInner[1] = 7.250000;")) << out; +} + +// Every level literal carries a decimal point even when the value is integral: ESSL reads +// `gl_TessLevelOuter[0] = 1;` as an int assigned to a float and refuses to compile the stage, +// which would take the whole program down with it. +TEST(PassthroughTessControlEsslTest, SpellsIntegralLevelsAsFloatLiterals) { + const String out = BuildPassthroughTessControlEssl(320, 4, "", "", FloatVec4(2.0f, 2.0f, 2.0f, 2.0f), + FloatVec2(2.0f, 2.0f)); + EXPECT_FALSE(Contains(out, "= 2;")) << out; +} + +// glPatchParameterfv accepts any float, NaN and infinity included. Emitting "nan" would make the +// synthesized stage fail to compile; 0.0 is the honest stand-in, because a level that is not a +// positive number discards the patch in GL too. +TEST(PassthroughTessControlEsslTest, NonFiniteLevelsBecomeZero) { + const Float notANumber = std::numeric_limits::quiet_NaN(); + const Float infinity = std::numeric_limits::infinity(); + const String out = BuildPassthroughTessControlEssl(320, 4, "", "", + FloatVec4(notANumber, infinity, 1.0f, 1.0f), + FloatVec2(notANumber, 1.0f)); + EXPECT_TRUE(Contains(out, "gl_TessLevelOuter[0] = 0.0;")) << out; + EXPECT_TRUE(Contains(out, "gl_TessLevelOuter[1] = 0.0;")) << out; + EXPECT_TRUE(Contains(out, "gl_TessLevelInner[0] = 0.0;")) << out; + EXPECT_FALSE(Contains(out, "nan")) << out; + EXPECT_FALSE(Contains(out, "inf")) << out; +} + // ES 3.1 reaches tessellation only through the extension; the caller has already established // that the driver runs the evaluation stage at all, so the only question is the spelling. TEST(PassthroughTessControlEsslTest, RequestsTheExtensionBelowEs32) { - const String out = BuildPassthroughTessControlEssl(310, 3, "", ""); + const String out = BuildPassthroughTessControlEssl(310, 3, "", "", kDefaultOuter, kDefaultInner); EXPECT_EQ(out.find("#version 310 es"), 0u) << out; EXPECT_TRUE(Contains(out, "#extension GL_EXT_tessellation_shader : require")) << out; } @@ -1325,7 +1371,7 @@ TEST(PassthroughTessControlEsslTest, RequestsTheExtensionBelowEs32) { TEST(PassthroughTessControlEsslTest, MirrorsTheNeighboursPerVertexBlocks) { const String inMembers = " highp vec4 gl_Position; highp float gl_PointSize; "; const String outMembers = " highp vec4 gl_Position; "; - const String out = BuildPassthroughTessControlEssl(320, 4, inMembers, outMembers); + const String out = BuildPassthroughTessControlEssl(320, 4, inMembers, outMembers, kDefaultOuter, kDefaultInner); EXPECT_TRUE(Contains(out, "in gl_PerVertex {" + inMembers + "} gl_in[gl_MaxPatchVertices];")) << out; EXPECT_TRUE(Contains(out, "out gl_PerVertex {" + outMembers + "} gl_out[];")) << out; } diff --git a/MobileGL/MG_Test/Pipeline/PassthroughTessControlTest.cpp b/MobileGL/MG_Test/Pipeline/PassthroughTessControlTest.cpp index ac3b023d..5ba66d4d 100644 --- a/MobileGL/MG_Test/Pipeline/PassthroughTessControlTest.cpp +++ b/MobileGL/MG_Test/Pipeline/PassthroughTessControlTest.cpp @@ -101,9 +101,13 @@ namespace { return builtIns; } + const FloatVec4 kDefaultOuter(1.0f, 1.0f, 1.0f, 1.0f); + const FloatVec2 kDefaultInner(1.0f, 1.0f); + Vector CompileGeneratedSource(Uint32 patchVertices) { using namespace MG_Util::ShaderTranspiler; - const String source = ProgramFactory::BuildPassthroughTessControlSource(patchVertices); + const String source = + ProgramFactory::BuildPassthroughTessControlSource(patchVertices, kDefaultOuter, kDefaultInner); ShaderAttrib shaderAttrib{.shaderType = GL_TESS_CONTROL_SHADER, .sourceStr = source}; auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); @@ -162,11 +166,44 @@ TEST_F(PassthroughTessControlTest, ForwardsPositionAndWritesBothLevelArrays) { // user-defined varying, ReflectPassthroughTessControlNeed's "built-ins only" refusal stops being // the right gate and both have to move together. TEST_F(PassthroughTessControlTest, InterfaceIsBuiltInsOnly) { - const String source = ProgramFactory::BuildPassthroughTessControlSource(4); + const String source = ProgramFactory::BuildPassthroughTessControlSource(4, kDefaultOuter, kDefaultInner); EXPECT_EQ(source.find("layout(location"), String::npos) << source; EXPECT_NE(source.find("layout(vertices = 4) out;"), String::npos) << source; } +// glPatchParameterfv's levels are compiled into this stage - Vulkan has no dynamic state for them - +// so two different level sets must produce two different sources AND two different cache keys. +// Without the second half a pipeline memoised at one set of levels would be handed back after the +// application changed them, and the tessellation would silently stay at the old levels. +TEST_F(PassthroughTessControlTest, BakesTheDefaultTessLevelsInAndKeysOnThem) { + const FloatVec4 outer(2.0f, 3.0f, 4.0f, 5.0f); + const FloatVec2 inner(6.5f, 7.25f); + const String source = ProgramFactory::BuildPassthroughTessControlSource(4, outer, inner); + EXPECT_NE(source.find("gl_TessLevelOuter[0] = 2.000000;"), String::npos) << source; + EXPECT_NE(source.find("gl_TessLevelOuter[3] = 5.000000;"), String::npos) << source; + EXPECT_NE(source.find("gl_TessLevelInner[0] = 6.500000;"), String::npos) << source; + EXPECT_NE(source.find("gl_TessLevelInner[1] = 7.250000;"), String::npos) << source; + + const Uint64 defaultKey = + ProgramFactory::ComputePassthroughTessControlKey(4, kDefaultOuter, kDefaultInner); + EXPECT_NE(ProgramFactory::ComputePassthroughTessControlKey(4, outer, inner), defaultKey); + EXPECT_NE(ProgramFactory::ComputePassthroughTessControlKey(4, kDefaultOuter, inner), defaultKey); + EXPECT_NE(ProgramFactory::ComputePassthroughTessControlKey(3, kDefaultOuter, kDefaultInner), defaultKey); + EXPECT_EQ(ProgramFactory::ComputePassthroughTessControlKey(4, kDefaultOuter, kDefaultInner), defaultKey); +} + +// The generated stage still has to COMPILE with non-default levels: an integral level spelled +// without a decimal point is an int literal, and `gl_TessLevelOuter[0] = 2;` does not compile. +TEST_F(PassthroughTessControlTest, CompilesWithNonDefaultLevels) { + using namespace MG_Util::ShaderTranspiler; + const String source = + ProgramFactory::BuildPassthroughTessControlSource(4, FloatVec4(2.0f, 2.0f, 2.0f, 2.0f), + FloatVec2(2.0f, 2.0f)); + ShaderAttrib shaderAttrib{.shaderType = GL_TESS_CONTROL_SHADER, .sourceStr = source}; + auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); + EXPECT_TRUE(shaderResult) << (shaderResult ? String{} : shaderResult.error().log) << source; +} + // THE load-bearing test. Vulkan matches built-in interface blocks by their whole shape, and this // stage is compiled ON ITS OWN - it never goes through the glslang link that gives a real program // its gl_PerVertex. So the shape it declares has to equal the shape a linked vertex+evaluation diff --git a/MobileGL/MG_Test/State/RenderStateTest.cpp b/MobileGL/MG_Test/State/RenderStateTest.cpp index b4a2a7f7..5403a008 100644 --- a/MobileGL/MG_Test/State/RenderStateTest.cpp +++ b/MobileGL/MG_Test/State/RenderStateTest.cpp @@ -26,6 +26,7 @@ #include "Includes.h" #include "Init.h" +#include #include #include #include @@ -632,3 +633,128 @@ TEST_F(RenderStateTest, TheFirstScissorWriteBumpsTheVersionEvenWhenTheValueDoesN indexed.SetScissorBoxIndexed(3, IntVec4(0, 0, 0, 0)); EXPECT_EQ(indexed.GetVersion(), indexedSettled); } + +// --- glPatchParameterfv (GL 4.6 core 11.2.2) --------------------------------------------------- +// +// GL_PATCH_DEFAULT_OUTER_LEVEL / GL_PATCH_DEFAULT_INNER_LEVEL are the tessellation levels a +// program with an evaluation stage and NO control stage runs at. glPatchParameterfv was a stub +// that stored nothing and raised nothing, so the state could never move off its 1.0 default and +// both backends hardcoded 1.0 into the pass-through control stage they synthesize. The getters +// were absent too, which is what KHR-GL4x.tessellation_shader.single. +// default_values_of_context_wide_properties dies on. + +TEST_F(RenderStateTest, PatchDefaultLevelsStartAtTheGLDefault) { + GLfloat outer[4] = {-1.0f, -1.0f, -1.0f, -1.0f}; + MG_Impl::GLImpl::GetFloatv(GL_PATCH_DEFAULT_OUTER_LEVEL, outer); + ExpectSingleGlError(GL_NO_ERROR); + for (const GLfloat level : outer) EXPECT_FLOAT_EQ(level, 1.0f); + + GLfloat inner[2] = {-1.0f, -1.0f}; + MG_Impl::GLImpl::GetFloatv(GL_PATCH_DEFAULT_INNER_LEVEL, inner); + ExpectSingleGlError(GL_NO_ERROR); + for (const GLfloat level : inner) EXPECT_FLOAT_EQ(level, 1.0f); +} + +TEST_F(RenderStateTest, PatchDefaultLevelsRoundTripThroughEveryGetter) { + const GLfloat outerIn[4] = {2.0f, 3.5f, 4.0f, 5.25f}; + MG_Impl::GLImpl::PatchParameterfv(GL_PATCH_DEFAULT_OUTER_LEVEL, outerIn); + ExpectSingleGlError(GL_NO_ERROR); + const GLfloat innerIn[2] = {6.5f, 7.0f}; + MG_Impl::GLImpl::PatchParameterfv(GL_PATCH_DEFAULT_INNER_LEVEL, innerIn); + ExpectSingleGlError(GL_NO_ERROR); + + GLfloat outer[4] = {}; + MG_Impl::GLImpl::GetFloatv(GL_PATCH_DEFAULT_OUTER_LEVEL, outer); + EXPECT_FLOAT_EQ(outer[0], 2.0f); + EXPECT_FLOAT_EQ(outer[1], 3.5f); + EXPECT_FLOAT_EQ(outer[2], 4.0f); + EXPECT_FLOAT_EQ(outer[3], 5.25f); + GLfloat inner[2] = {}; + MG_Impl::GLImpl::GetFloatv(GL_PATCH_DEFAULT_INNER_LEVEL, inner); + EXPECT_FLOAT_EQ(inner[0], 6.5f); + EXPECT_FLOAT_EQ(inner[1], 7.0f); + ExpectSingleGlError(GL_NO_ERROR); + + // Float state read through the integer and boolean getters: glGetIntegerv rounds (GL 4.6 core + // 2.2.2) and glGetBooleanv delegates to it, so both must ANSWER rather than report + // INVALID_ENUM - which is exactly what the conformance suite asks them first. + GLint outerInts[4] = {}; + MG_Impl::GLImpl::GetIntegerv(GL_PATCH_DEFAULT_OUTER_LEVEL, outerInts); + EXPECT_EQ(outerInts[0], 2); + EXPECT_EQ(outerInts[1], 4) << "3.5 rounds away from zero"; + EXPECT_EQ(outerInts[3], 5); + ExpectSingleGlError(GL_NO_ERROR); + + GLboolean outerBools[4] = {}; + MG_Impl::GLImpl::GetBooleanv(GL_PATCH_DEFAULT_OUTER_LEVEL, outerBools); + EXPECT_EQ(outerBools[0], GL_TRUE); + ExpectSingleGlError(GL_NO_ERROR); + + GLdouble outerDoubles[4] = {}; + MG_Impl::GLImpl::GetDoublev(GL_PATCH_DEFAULT_OUTER_LEVEL, outerDoubles); + EXPECT_DOUBLE_EQ(outerDoubles[3], 5.25) << "glGetDoublev must widen all four, not just the first"; + ExpectSingleGlError(GL_NO_ERROR); + + // Put the context back where the rest of the binary expects it. + const GLfloat defaults4[4] = {1.0f, 1.0f, 1.0f, 1.0f}; + const GLfloat defaults2[2] = {1.0f, 1.0f}; + MG_Impl::GLImpl::PatchParameterfv(GL_PATCH_DEFAULT_OUTER_LEVEL, defaults4); + MG_Impl::GLImpl::PatchParameterfv(GL_PATCH_DEFAULT_INNER_LEVEL, defaults2); + DrainPendingGlErrors(); +} + +TEST_F(RenderStateTest, PatchParameterfvRejectsEveryOtherPname) { + const GLfloat levels[4] = {1.0f, 1.0f, 1.0f, 1.0f}; + MG_Impl::GLImpl::PatchParameterfv(GL_PATCH_VERTICES, levels); + ExpectSingleGlError(GL_INVALID_ENUM); + MG_Impl::GLImpl::PatchParameterfv(GL_MAX_PATCH_VERTICES, levels); + ExpectSingleGlError(GL_INVALID_ENUM); + // The integer setter keeps its own, disjoint, accepted pname. + MG_Impl::GLImpl::PatchParameteri(GL_PATCH_DEFAULT_OUTER_LEVEL, 4); + ExpectSingleGlError(GL_INVALID_ENUM); +} + +TEST_F(RenderStateTest, PatchDefaultLevelsAreTreatedAsPipelineState) { + // Load-bearing: both backends compile these numbers into the pass-through tessellation control + // stage they synthesize, so a change has to invalidate an already-built program the same way a + // glPatchParameteri does. Bumping only the all-state version would leave DirectVulkan's + // pipeline memo - which keys on the PIPELINE-state version - handing back a pipeline built + // with the old levels. + MG_State::GLState::RenderState state; + const Uint initialPipelineVersion = state.GetPipelineStateVersion(); + state.SetPatchDefaultOuterLevel(FloatVec4(2.0f, 2.0f, 2.0f, 2.0f)); + EXPECT_GT(state.GetPipelineStateVersion(), initialPipelineVersion); + + const Uint settled = state.GetPipelineStateVersion(); + state.SetPatchDefaultOuterLevel(FloatVec4(2.0f, 2.0f, 2.0f, 2.0f)); + EXPECT_EQ(state.GetPipelineStateVersion(), settled) << "a redundant write is free"; + + state.SetPatchDefaultInnerLevel(FloatVec2(3.0f, 3.0f)); + EXPECT_GT(state.GetPipelineStateVersion(), settled); +} + +// --- desktop GL_PRIMITIVE_RESTART state -------------------------------------------------------- +// +// The cap and its index are what a desktop application enables instead of ES's +// GL_PRIMITIVE_RESTART_FIXED_INDEX. Both halves have to be answerable, because the backends read +// them on every indexed draw to decide whether the index data needs rewriting. +TEST_F(RenderStateTest, PrimitiveRestartCapAndIndexAreBothQueryable) { + EXPECT_EQ(MG_Impl::GLImpl::IsEnabled(GL_PRIMITIVE_RESTART), GL_FALSE); + ExpectSingleGlError(GL_NO_ERROR); + + MG_Impl::GLImpl::Enable(GL_PRIMITIVE_RESTART); + MG_Impl::GLImpl::PrimitiveRestartIndex(1026u); + ExpectSingleGlError(GL_NO_ERROR); + + EXPECT_EQ(MG_Impl::GLImpl::IsEnabled(GL_PRIMITIVE_RESTART), GL_TRUE); + GLint index = 0; + MG_Impl::GLImpl::GetIntegerv(GL_PRIMITIVE_RESTART_INDEX, &index); + EXPECT_EQ(index, 1026); + // The fixed-index cap is a separate piece of state and must not have moved. + EXPECT_EQ(MG_Impl::GLImpl::IsEnabled(GL_PRIMITIVE_RESTART_FIXED_INDEX), GL_FALSE); + ExpectSingleGlError(GL_NO_ERROR); + + MG_Impl::GLImpl::Disable(GL_PRIMITIVE_RESTART); + MG_Impl::GLImpl::PrimitiveRestartIndex(0u); + DrainPendingGlErrors(); +}