diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 728454f9..3f5a863b 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -7539,18 +7539,44 @@ namespace MobileGL::MG_Backend::DirectGLES { // carried 294 INFO lines and zero ERROR lines while two generated shaders // were being rejected outright, and the lane could not say why it was // rendering an empty translucent layer. A shader the driver refuses is - // never noise, and one line per refused shader is bounded by program count. + // never noise. // - // The SOURCE goes with it, the way the synthesized pass-through control - // stage's failure already prints its own: the driver log names a line and a - // column in text that exists nowhere but here, and without it the only way - // to read "`gl_PointSize' undeclared" is to rebuild the whole library at - // DEBUG. Still one line per refused shader. + // A BOUNDED EXCERPT of the source goes with it. The driver log names a line + // and a column in text that exists nowhere but here, so without any source at + // all the only way to read "`gl_PointSize' undeclared" is to rebuild the whole + // library at DEBUG - but the full dump cannot go at E either. This is not + // "one line per refused shader": SyncToBackend's rebuild gate keys on + // per-draw state (the enabled-draw-buffer count among it), so a program used + // across passes with different draw-buffer counts re-transpiles, re-compiles + // and re-fails on every alternation, i.e. per frame. At E - live at the + // production INFO level - each of those records would push the whole + // post-SPIRV-Cross ESSL through the global log mutex with a forced flush onto + // /sdcard/MG/latest.log, the file users are asked to share. The excerpt keeps + // the record O(1); the full text is still there at D, printed against this + // same backend shader id by the "Setting shader source" line above, so + // nothing needs to be dumped twice. + constexpr SizeT kMaxLoggedSourceBytes = 2048; + String truncatedSource; + const char* sourceForLog = source.c_str(); + if (source.size() > kMaxLoggedSourceBytes) { + // Back up to a line boundary when there is one inside the window, so the + // excerpt ends on a whole statement rather than mid-token. Built only on + // this branch: a stage that fits keeps its own buffer and is not copied. + SizeT cut = kMaxLoggedSourceBytes; + if (const SizeT lastNewline = source.rfind('\n', cut); + lastNewline != String::npos && lastNewline > 0) { + cut = lastNewline + 1; + } + truncatedSource = source.substr(0, cut); + truncatedSource += "... [" + std::to_string(source.size() - cut) + + " more bytes; the whole stage is printed at the DEBUG level]\n"; + sourceForLog = truncatedSource.c_str(); + } MGLOG_E("Shader compilation failed. State program ID: %u, stage: %s, backend shader ID: " "%u, driver log: %s\nSource:\n%s", stateProgramObject->GetExternalIndex(), MG_Util::ConvertGLEnumToString(glShaderType).c_str(), backendShaderId, - log.data(), source.c_str()); + log.data(), sourceForLog); m_backendProgramUsable = false; // Nothing will ever attach this one, so nothing else can free it. g_GLESFuncs.glDeleteShader(backendShaderId); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp index ab77b34d..b001bbcb 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp @@ -3764,6 +3764,31 @@ namespace MobileGL::MG_Backend::DirectVulkan { entry.xfbCaptureDeclined = true; } + // Does this stage need a device feature the device did not give us? Asked ONLY when + // the feature is off, so a device that has it - the common case - pays nothing: the + // whole test is short-circuited before the module is parsed. + // + // gl_PointSize is an ordinary per-vertex output in desktop GL and any + // vertex-processing stage may write it, but Vulkan puts the built-in behind + // shaderTessellationAndGeometryPointSize in the tessellation and geometry stages + // (VUID-RuntimeSpirv-PointSize-06439). glslang emits TessellationPointSize / + // GeometryPointSize from the application's own access, so this program is legal GL + // that this device cannot run - the same shape the DirectGLES arm reports when a + // driver advertises neither EXT nor OES point-size extension, and it deserves the + // same named message rather than a pipeline the driver may fault on. + if (!m_tessellationAndGeometryPointSizeEnabled && + (stages[i] == ShaderStage::TessControl || stages[i] == ShaderStage::TessEval || + stages[i] == ShaderStage::Geometry) && + MG_Util::ShaderTranspiler::ShaderCompiler::ModuleDeclaresTessellationOrGeometryPointSize( + moduleSpv)) { + MGLOG_E_ONCE("ProgramFactory: program %u stage %d accesses gl_PointSize, but this device does not " + "support shaderTessellationAndGeometryPointSize; its draws are refused rather than " + "built into a pipeline the driver may fault on. Point size from a non-vertex stage " + "is not available on this device.", + program.GetExternalIndex(), static_cast(stages[i])); + entry.pointSizeCapabilityUnsupported = true; + } + VkShaderModuleCreateInfo smci{VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO}; smci.codeSize = moduleSpv.size() * sizeof(Uint); smci.pCode = moduleSpv.data(); @@ -4040,11 +4065,24 @@ namespace MobileGL::MG_Backend::DirectVulkan { // PassthroughTessControlTest.MatchesTheFrontendPerVertexBlock is the latch, and it now // links the program at both 430 and 460. // - // Only gl_Position is written. gl_PointSize is declared but left alone deliberately: - // writing it from a tessellation stage requires the shaderTessellationAndGeometryPointSize - // feature, which this renderer does not enable, so a program whose evaluation stage reads - // gl_in[].gl_PointSize gets an undefined point size instead of the vertex stage's - a gap - // this trades for not making every tessellated pipeline depend on an optional feature. + // Only gl_Position is written, and gl_PointSize is declared without being forwarded. That + // is a KNOWN GAP, not a design: GL 4.6 core 11.2.2 says the fixed-function pass-through + // hands the input patch to the evaluation stage unmodified, so an evaluation stage + // reading gl_in[].gl_PointSize should see the vertex stage's value and instead sees + // whatever this stage left in gl_out[] - which is nothing. A capture of it (the mirror in + // XfbCaptureDecoratePass) faithfully records that nothing. + // + // The reason this comment used to give - "the renderer does not enable + // shaderTessellationAndGeometryPointSize" - stopped being true when + // VulkanRenderer::CreateLogicalDeviceAndQueues started taking the feature wherever the + // device advertises it. Closing the gap is therefore possible now, but it is not free: + // the forwarding store has to be gated on that feature, because on a device without it + // the store is exactly the invalid usage the build-time refusal + // (VkProgramObject::pointSizeCapabilityUnsupported) exists to keep away from the driver - + // and this synthesized stage is not the application's, so refusing the program because + // MobileGL's own pass-through named a built-in would be the wrong trade. Nothing pins + // the shape either: every case in TessellationXfbCaptureScenario builds an explicit + // control stage, so a TES-without-TCS test has to come with the fix. const String perVertexBody = BuildPerVertexMemberDeclarations(perVertexMembers); source += "in gl_PerVertex {\n" + perVertexBody + "} gl_in[gl_MaxPatchVertices];\n"; source += "out gl_PerVertex {\n" + perVertexBody + "} gl_out[];\n"; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h index eb841c71..9ac24faa 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h @@ -216,6 +216,18 @@ namespace MobileGL::MG_Backend::DirectVulkan { // an Xfb-flagged cache entry - the flag and the layout are part of the program cache // key, so it was sticky for every later captured draw of the program, not a glitch. Bool xfbCaptureDeclined = false; + // The program has a tessellation or geometry module declaring TessellationPointSize / + // GeometryPointSize on a device whose shaderTessellationAndGeometryPointSize feature + // is off, so a pipeline built from it is invalid usage + // (VUID-RuntimeSpirv-PointSize-06439). Its draws are refused in SetupDraw rather than + // handed to the driver - the same contract PipelineFactory's half-tessellated refusal + // implements one level up, and the counterpart of the DirectGLES arm that reports a + // driver with neither point-size extension by name. + // + // Sticky by construction, which is what makes ONE log line honest: the flag lives on + // the cache entry, so every later draw of the same program variant reads the same + // answer instead of re-deciding it. + Bool pointSizeCapabilityUnsupported = false; Bool needsPassthroughTessControl = false; // ...and the pass-through this renderer can synthesize carries gl_Position and // nothing else, so it is only correct when the evaluation stage's inputs are @@ -440,12 +452,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { explicit ProgramFactory(VkDevice device, const VulkanRendererConfig& config, Uint32 maxBindings, Bool shaderDrawParametersEnabled, Bool unformattedFloatStorageImagesEnabled, + Bool tessellationAndGeometryPointSizeEnabled, Bool enableSpirvValidation, UpdateAfterBindLimits updateAfterBindLimits, SubgroupLoweringPolicy subgroupPolicy) : m_device(device), m_maxBindings(maxBindings), m_config(config), m_shaderDrawParametersEnabled(shaderDrawParametersEnabled), m_unformattedFloatStorageImagesEnabled(unformattedFloatStorageImagesEnabled), + m_tessellationAndGeometryPointSizeEnabled(tessellationAndGeometryPointSizeEnabled), m_enableSpirvValidation(enableSpirvValidation), m_updateAfterBindLimits(updateAfterBindLimits), m_subgroupPolicy(subgroupPolicy) { @@ -605,6 +619,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { // True only when the logical device enabled both // shaderStorageImageReadWithoutFormat and shaderStorageImageWriteWithoutFormat. Bool m_unformattedFloatStorageImagesEnabled = false; + // True when the logical device enabled shaderTessellationAndGeometryPointSize. When it is + // FALSE a program whose tessellation or geometry module declares TessellationPointSize / + // GeometryPointSize is refused at build time (see VkProgramObject:: + // pointSizeCapabilityUnsupported) instead of being handed to the driver as invalid usage. + Bool m_tessellationAndGeometryPointSizeEnabled = false; // Startup snapshot used only by internally synthesized shader modules, which do not // originate from a ProgramLinkTask. Bool m_enableSpirvValidation = false; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 042e3bc7..4b1755bb 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3159,6 +3159,7 @@ void main() { m_programFactory = MakeUnique(m_device, m_config, maxProgramBindings, m_shaderDrawParametersFeatureEnabled, m_unformattedFloatStorageImagesEnabled, + m_tessellationAndGeometryPointSizeFeatureEnabled, MG_Config::Features.EnableSpirvValidation, m_updateAfterBindLimits, subgroupPolicy); MOBILEGL_ASSERT(m_programFactory != nullptr, "ProgramFactory creation failed."); @@ -6111,6 +6112,12 @@ void main() { // no way to know the bound pipeline's last pre-rasterization module lost (or never got) // its Xfb execution mode. See VkProgramObject::xfbCaptureDeclined. m_currentDrawXfbCaptureDeclined = programObj.xfbCaptureDeclined; + // A refused program cannot reach here today - the full path refuses before it ever + // records a snapshot - but declining the fast path costs one compare and means the + // refusal does not depend on that ordering staying true. + if (programObj.pointSizeCapabilityUnsupported) { + return false; + } // The pipeline and the vertex-input pre-flight depend on the VAO only through // its resolved LAYOUT (layoutHash folds the attribute formats, bindings and the @@ -6518,6 +6525,12 @@ void main() { // no way to know the bound pipeline's last pre-rasterization module lost (or never got) // its Xfb execution mode. See VkProgramObject::xfbCaptureDeclined. m_currentDrawXfbCaptureDeclined = programObj.xfbCaptureDeclined; + // The build already said why, once, naming the program and the stage. Refusing here - + // before any pipeline is built from it - is what makes that message a decline rather + // than a note attached to invalid usage the driver still receives. + if (programObj.pointSizeCapabilityUnsupported) { + return false; + } // For the snapshot's memoised entry pointer: if anything below inserts into the // program cache (blit/aux program compiles), the epoch moves and the snapshot // stores no pointer for this draw - the fast path then re-looks-up once. diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index 73a36399..271320c2 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -586,8 +586,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { Bool m_primitiveTopologyListRestartFeatureEnabled = false; // shaderTessellationAndGeometryPointSize gates the PointSize built-in in a tessellation // or geometry stage, which desktop GL treats as an ordinary per-vertex output (writable, - // and capturable by name through transform feedback). Cached at device creation so the - // program build can say so when a program asks for something the device will not run. + // and capturable by name through transform feedback). Cached at device creation and + // handed to ProgramFactory, which refuses a program whose tessellation or geometry module + // declares the matching SPIR-V capability while this is false - SetupDraw then skips its + // draws (VkProgramObject::pointSizeCapabilityUnsupported) rather than building a pipeline + // that is invalid usage. Bool m_tessellationAndGeometryPointSizeFeatureEnabled = false; // VK_EXT_custom_border_color. Vulkan's four predefined VkBorderColor values cover only // transparent/opaque black and opaque white; GL_TEXTURE_BORDER_COLOR is an arbitrary vec4 (or diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp index 379adb91..4eb22f2c 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp @@ -785,6 +785,36 @@ namespace MobileGL { return false; } + Bool ShaderCompiler::ModuleDeclaresTessellationOrGeometryPointSize(const Vector& spirv) { + if (spirv.empty()) { + return false; + } + std::unique_ptr context = spvtools::BuildModule( + SPV_ENV_VULKAN_1_1, + MakeSpirvMessageConsumer("ModuleDeclaresTessellationOrGeometryPointSize"), spirv.data(), + spirv.size()); + if (!context) { + // Unparseable is not a verdict about point size. Say no, so the caller keeps + // building the program: the module is already broken for other reasons and + // the diagnostics that own that failure are better placed than this one. + return false; + } + // The CAPABILITY, not the BuiltIn decoration, because the capability is exactly + // what the feature gates: a module may declare gl_PerVertex with a PointSize + // member and never access it, and glslang then emits no capability + // (GlslangToSpv defers it to actual use) - such a module is legal without the + // feature and must not be declined. + for (const spvtools::opt::Instruction& capability : context->capabilities()) { + if (capability.NumInOperands() < 1) continue; + const auto declared = static_cast(capability.GetSingleWordInOperand(0)); + if (declared == spv::Capability::TessellationPointSize || + declared == spv::Capability::GeometryPointSize) { + return true; + } + } + return false; + } + Bool ShaderCompiler::ModuleDeclaresFloat64(const Vector& spirv) { if (spirv.empty()) { // Same reasoning as ModuleDeclaresBufferTextureSampler: a stage that produced diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h index 2cfe97e3..3eb21723 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h @@ -538,6 +538,15 @@ namespace MobileGL { // Asked of the FINAL bytes, so it answers for whatever the backend transform // chain actually produced rather than for what it was asked to produce. static Bool ModuleDeclaresTransformFeedback(const Vector& spirv); + // Does this module declare TessellationPointSize or GeometryPointSize - i.e. does + // it need VkPhysicalDeviceFeatures::shaderTessellationAndGeometryPointSize before + // a pipeline built from it is legal usage (VUID-RuntimeSpirv-PointSize-06439)? + // glslang emits either capability from any access to the PointSize built-in in a + // tessellation or geometry stage, which desktop GL treats as an ordinary + // per-vertex output, so a program that is perfectly legal in GL can need a Vulkan + // feature the device does not have. Callers only ask when the feature is OFF, so + // the module parse costs nothing on a device that has it. + static Bool ModuleDeclaresTessellationOrGeometryPointSize(const Vector& spirv); // True when the module still declares a 64-bit float type. After // SanitizeAndOptimizeBinary that can only mean DemoteFloat64Pass declined the