diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index ccc68687..5522ccba 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -517,7 +517,9 @@ namespace MobileGL::MG_State::GLState { }; } // namespace - void ProgramLinkTask::DeferLog(String line) { diagnostics.logLines.push_back(Move(line)); } + void ProgramLinkTask::DeferLog(String line, const Int level) { + diagnostics.logLines.push_back({level, Move(line)}); + } void ProgramLinkTask::SubmitAfter(const Vector>& deps) { // +1 for the guard this function releases itself. Without it, a dependency that @@ -855,6 +857,18 @@ namespace MobileGL::MG_State::GLState { spirvHandoff.reflection.uniformReflection = artifacts.uniformReflection; spirvHandoff.reflection.blockReflection = artifacts.blockReflection; spirvHandoff.reflection.tProgramBlockIndexToGl = artifacts.tProgramBlockIndexToGl; + // The capture set is NOT part of that slice (see the handoff's own comment), and the + // point-size demotion needs exactly one bit out of it: whether anything asked to + // capture gl_PointSize. Derived here, where ResolveTransformFeedbackVaryings has + // just filled artifacts.xfbVaryings and before the join moves them away, because a + // capture stage that only READS the built-in still has to declare the carrier the + // capture binds to - and phase B has no other way to learn that. + for (const ProgramObject::XfbVarying& varying : artifacts.xfbVaryings) { + if (varying.name == "gl_PointSize") { + spirvHandoff.captureRequestsPointSize = true; + break; + } + } // Phase B pairs this with its own SpirvArtifacts to insert the completed front end. // A COPY, because the GL-thread join moves `artifacts` out of this node before phase B // runs - and with the TProgram dropped, because a memo must never hold a glslang arena. diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h index 46bd54f6..245f2665 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.h @@ -117,6 +117,19 @@ namespace MobileGL::MG_State::GLState { // for phase B after the join has moved `artifacts` away. ProgramObject::LinkArtifacts reflection; + // Whether the RESOLVED transform-feedback capture set names gl_PointSize - the + // one fact about `artifacts.xfbVaryings` phase B needs, carried as a derived + // bool rather than by widening the slice above, which is deliberately the five + // (now eight) fields BuildGlobalUboRouting consumes and nothing else. + // + // It has to be here and cannot be re-derived: the point-size demotion forces the + // capture-capable stage to declare its carrier even when that stage never WRITES + // the built-in (ShaderCompiler::DemoteTessellationGeometryPointSizeForProgram's + // `captureRequestsPointSize`), and by phase B the only record of the request is + // this bit. No new L1 key material: the key already covers + // `requestedXfbVaryings`, of which this is a function. + Bool captureRequestsPointSize = false; + // L1 shader-translation memo key for this program's SPIR-V (see // MG_Util/ShaderTranspiler/TranslationCache.h). Built HERE, at the tail of phase // A, and not by phase B - two reasons, both structural: @@ -201,8 +214,10 @@ namespace MobileGL::MG_State::GLState { // Worker-side MGLOG replacement: appended to diagnostics.logLines and replayed by the // join, on the GL thread, where a serial implementation would have printed it. // Logging straight from a worker interleaves mid-line with the GL thread's output and - // lands out of order relative to the glLinkProgram that caused it. - void DeferLog(String line); + // lands out of order relative to the glLinkProgram that caused it. `level` is the + // severity the replay uses; DEBUG (the default) is compiled out of every shipped + // build, so a line that has to survive one names its own. + void DeferLog(String line, Int level = MOBILEGL_LOG_LEVEL_DEBUG); // Counts down to zero exactly once. Starts at deps + 1: the extra guard is released // by SubmitAfter itself, so a dependency that settles while the edges are still being diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.cpp index 74780c8c..36878336 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.cpp @@ -16,10 +16,29 @@ #include #include +#include #include namespace MobileGL::MG_State::GLState { - void ProgramSpirvTask::DeferLog(String line) { diagnostics.logLines.push_back(Move(line)); } + namespace { + // The MGLOG_*_ONCE latch, moved to the SOURCE of a deferred line. It cannot live at + // the replay: Async::ApplyDeferredDiagnostics is ONE site shared by every job in the + // tree, so a latch there would silence unrelated lines. And it has to exist: a shader + // pack hands the same refusal to program after program, and a per-program WARN on a + // path like that is exactly the repeated production logging the house rule forbids. + // First occurrence at WARN - the one a bug report needs - every later one back at + // DEBUG, which shipped builds compile out. + Int FirstTimeWarnLevel(std::atomic_flag& latch) { + return latch.test_and_set(std::memory_order_relaxed) ? MOBILEGL_LOG_LEVEL_DEBUG + : MOBILEGL_LOG_LEVEL_WARN; + } + std::atomic_flag g_pointSizeDeclineReported; + std::atomic_flag g_pointSizeOptimizerFailureReported; + } // namespace + + void ProgramSpirvTask::DeferLog(String line, const Int level) { + diagnostics.logLines.push_back({level, Move(line)}); + } void ProgramSpirvTask::SubmitAfter(const SharedPtr& phaseA) { MOBILEGL_ASSERT(phaseA != nullptr, "ProgramSpirvTask::SubmitAfter: the phase-A node is missing"); @@ -285,13 +304,12 @@ namespace MobileGL::MG_State::GLState { // size in those stages - pays one module parse per stage and no rewrite. artifacts.pointSizeDemoted = false; if (allOptimized && (demoteTessellationPointSize || demoteGeometryPointSize)) { - Bool captureRequestsPointSize = false; - for (const auto& varying : handoff.reflection.xfbVaryings) { - if (varying.name == "gl_PointSize") { - captureRequestsPointSize = true; - break; - } - } + // Read off the HANDOFF's own derived bit, not off `handoff.reflection`: that + // field is the routing slice phase A fills with eight named members, and + // xfbVaryings is not one of them - reading it there answered "no capture ever + // asks for gl_PointSize" on every production link, which left a read-only + // capture stage without the carrier its capture binds to. + const Bool captureRequestsPointSize = handoff.captureRequestsPointSize; ShaderCompiler::PointSizeDemotionOutcome outcome; if (!ShaderCompiler::DemoteTessellationGeometryPointSizeForProgram( artifacts.generatedSpirv, handoff.shaderTypes, demoteTessellationPointSize, @@ -301,7 +319,8 @@ namespace MobileGL::MG_State::GLState { // and the backends' existing refusals stay in charge - honest, just slower. DeferLog(std::format("ProgramObject {}: point-size demotion failed in the optimizer; the " "program keeps its built-in and the device's declines apply", - externalIndex)); + externalIndex), + FirstTimeWarnLevel(g_pointSizeOptimizerFailureReported)); } else if (outcome.demoted) { artifacts.pointSizeDemoted = true; DeferLog(std::format("ProgramObject {}: gl_PointSize demoted to an ordinary varying across " @@ -309,9 +328,15 @@ namespace MobileGL::MG_State::GLState { "gl_in reads; rasterized size falls back to 1.0)", externalIndex)); } else if (!outcome.declineDetail.empty()) { + // THE MOST VALUABLE LINE THIS FEATURE PRODUCES: which module shape the pass + // refused, and therefore why an affected device is still about to lose the + // program. Nothing else records it - `declineDetail` has no other runtime + // surface - so at the deferred channel's DEBUG default it was formatted and + // then dropped by every INFO build, i.e. every device and every CI artifact. DeferLog(std::format("ProgramObject {}: point-size demotion declined ({}); the program " "keeps its built-in and the device's declines apply", - externalIndex, outcome.declineDetail)); + externalIndex, outcome.declineDetail), + FirstTimeWarnLevel(g_pointSizeDeclineReported)); } } } diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.h b/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.h index 6d25f99b..d04c9907 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.h @@ -72,8 +72,9 @@ namespace MobileGL::MG_State::GLState { void BuildGlobalUboRouting(const ProgramLinkTask::SpirvHandoff& handoff, Uint externalIndex); // Worker-side MGLOG replacement, replayed by the join on the GL thread. Same reason as - // ProgramLinkTask::DeferLog. - void DeferLog(String line); + // ProgramLinkTask::DeferLog, and the same severity rule: DEBUG is compiled out of + // every shipped build, so a line that has to survive one names its own level. + void DeferLog(String line, Int level = MOBILEGL_LOG_LEVEL_DEBUG); SharedPtr m_phaseA; }; diff --git a/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.cpp index 3d0b99ab..6ed64911 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.cpp @@ -384,12 +384,15 @@ namespace MobileGL::MG_State::GLState { // the log, for every failing shader. The info log is what names the offending // line; the source is recoverable from the application. const SizeT firstLineEnd = artifacts.infoLog.find('\n'); - diagnostics.logLines.push_back(std::format( - "ShaderCompileTask: shader {} (stage {}) failed to compile; compileStatus = false. " - "Preprocessed source: {} bytes. First log line: {}", - externalIndex, static_cast(stage), shared.preprocessedSource.length(), - artifacts.infoLog.substr(0, firstLineEnd == String::npos ? artifacts.infoLog.length() - : firstLineEnd))); + diagnostics.logLines.push_back( + {MOBILEGL_LOG_LEVEL_DEBUG, + std::format( + "ShaderCompileTask: shader {} (stage {}) failed to compile; compileStatus = false. " + "Preprocessed source: {} bytes. First log line: {}", + externalIndex, static_cast(stage), shared.preprocessedSource.length(), + artifacts.infoLog.substr(0, firstLineEnd == String::npos + ? artifacts.infoLog.length() + : firstLineEnd))}); if (shouldPopulateCache) { fresh->outcome = ShaderPreprocessOutcome::ParseFailed; fresh->infoLog = artifacts.infoLog; diff --git a/MobileGL/MG_Test/Util/JobNodeTest.cpp b/MobileGL/MG_Test/Util/JobNodeTest.cpp index 79fbc002..b27be492 100644 --- a/MobileGL/MG_Test/Util/JobNodeTest.cpp +++ b/MobileGL/MG_Test/Util/JobNodeTest.cpp @@ -441,7 +441,7 @@ TEST(JobNodeException, AnExceptionEscapingABodyCancelsTheJobInsteadOfTerminating EXPECT_TRUE(job->IsCancelled()); EXPECT_FALSE(job->IsComplete()); ASSERT_EQ(job->diagnostics.logLines.size(), 1u); - EXPECT_NE(job->diagnostics.logLines[0].find("boom"), String::npos); + EXPECT_NE(job->diagnostics.logLines[0].text.find("boom"), String::npos); } TEST(JobNodeException, ANonStandardExceptionIsContainedToo) { diff --git a/MobileGL/MG_Util/Async/JobNode.cpp b/MobileGL/MG_Util/Async/JobNode.cpp index 2cdd1f30..9c466f82 100644 --- a/MobileGL/MG_Util/Async/JobNode.cpp +++ b/MobileGL/MG_Util/Async/JobNode.cpp @@ -95,11 +95,13 @@ namespace MobileGL::MG_Util::Async { // which means std::terminate for the whole process. Every job boundary contains // it and reports the job as Cancelled; the joining GL thread then sees a node // that produced no result, which is the same shape as an abandoned node. - diagnostics.logLines.push_back(std::format("Job body threw: {}", e.what())); + diagnostics.logLines.push_back( + {MOBILEGL_LOG_LEVEL_DEBUG, std::format("Job body threw: {}", e.what())}); TryTransition(JobState::Running, JobState::Cancelled); return; } catch (...) { - diagnostics.logLines.emplace_back("Job body threw a non-std exception"); + diagnostics.logLines.push_back( + {MOBILEGL_LOG_LEVEL_DEBUG, String("Job body threw a non-std exception")}); TryTransition(JobState::Running, JobState::Cancelled); return; } @@ -162,10 +164,27 @@ namespace MobileGL::MG_Util::Async { "being written"); if (!node.diagnostics.logLines.empty()) { - Vector lines; + Vector lines; lines.swap(node.diagnostics.logLines); - for (const String& line : lines) { - MGLOG_D("%s", line.c_str()); + for (const DeferredLogLine& line : lines) { + // Per-line severity, because a shipped build compiles MGLOG_D away entirely + // and a verdict that only this channel records would vanish with it. The + // levels are the compile-time constants, so a suppressed one costs nothing + // beyond the string the worker already built. + switch (line.level) { + case MOBILEGL_LOG_LEVEL_INFO: + MGLOG_I("%s", line.text.c_str()); + break; + case MOBILEGL_LOG_LEVEL_WARN: + MGLOG_W("%s", line.text.c_str()); + break; + case MOBILEGL_LOG_LEVEL_ERROR: + MGLOG_E("%s", line.text.c_str()); + break; + default: + MGLOG_D("%s", line.text.c_str()); + break; + } } } diff --git a/MobileGL/MG_Util/Async/JobNode.h b/MobileGL/MG_Util/Async/JobNode.h index 127b2886..82aa9816 100644 --- a/MobileGL/MG_Util/Async/JobNode.h +++ b/MobileGL/MG_Util/Async/JobNode.h @@ -9,6 +9,7 @@ #pragma once #include #include +#include #include #include @@ -35,9 +36,25 @@ namespace MobileGL::MG_Util::Async { UniquePtr info; }; + // One line of worker-side MGLOG text, with the severity the join replays it at. + // + // DEBUG is the default and stays the default: nearly every deferred line is per-program + // trace that a shipped build compiles out, which is the whole reason this channel could + // be a plain string vector for as long as it was. A line a SHIPPED build has to show - + // the reason a repair refused, which no other surface records - has to name its level + // here, or it is formatted on the worker and then thrown away at replay under the INFO + // level every device and CI build pins. Callers that sit on a repeated path latch at + // the SOURCE (a per-call-site atomic, exactly what MGLOG_*_ONCE does): the replay below + // is one shared site for every job in the tree, so a latch there would silence + // unrelated lines. + struct DeferredLogLine { + Int level = MOBILEGL_LOG_LEVEL_DEBUG; + String text; + }; + struct JobDiagnostics { - Vector errors; // replayed, in ascending `sequence`, by the join - Vector logLines; // worker-side MGLOG text, flushed in order by the join + Vector errors; // replayed, in ascending `sequence`, by the join + Vector logLines; // worker-side MGLOG text, flushed in order by the join }; // The scheduling primitive every asynchronous compile and link is built on. A node owns