[Fix] (Link, Async): carry the resolved gl_PointSize capture request into the SPIR-V handoff and let a deferred verdict name its own severity - the demotion read the request off a reflection slice phase A never fills it into, so its forced carrier was dead code, and its decline reason replayed at a level no shipped build keeps

This commit is contained in:
2026-08-28 06:22:12 -04:00
parent 97e07190ac
commit d1edf765f5
8 changed files with 123 additions and 29 deletions
@@ -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<SharedPtr<ShaderCompileTask>>& 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.
@@ -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
@@ -16,10 +16,29 @@
#include <MG_Util/ShaderTranspiler/TranslationCache.h>
#include <MG_Util/ShaderTranspiler/Types.h>
#include <atomic>
#include <cstring>
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<ProgramLinkTask>& 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));
}
}
}
@@ -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<ProgramLinkTask> m_phaseA;
};
@@ -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<Int>(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<Int>(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;
+1 -1
View File
@@ -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) {
+24 -5
View File
@@ -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<String> lines;
Vector<DeferredLogLine> 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;
}
}
}
+19 -2
View File
@@ -9,6 +9,7 @@
#pragma once
#include <Includes.h>
#include <MG_Util/Types.h>
#include <MG_Util/Debug/Log.h>
#include <MG_State/GLState/ErrorState/ErrorCode.h>
#include <MG_State/GLState/ErrorState/ErrorInfo.h>
@@ -35,9 +36,25 @@ namespace MobileGL::MG_Util::Async {
UniquePtr<ErrorInfo> 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<DeferredError> errors; // replayed, in ascending `sequence`, by the join
Vector<String> logLines; // worker-side MGLOG text, flushed in order by the join
Vector<DeferredError> errors; // replayed, in ascending `sequence`, by the join
Vector<DeferredLogLine> 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