mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-07 19:58:32 +09:00
[Fix] (Review): decline a point-size program the device cannot run, bound the compile-failure source dump, and correct the pass-through rationale
This commit is contained in:
@@ -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);
|
||||
|
||||
@@ -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<Int>(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";
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -3159,6 +3159,7 @@ void main() {
|
||||
m_programFactory = MakeUnique<ProgramFactory>(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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -785,6 +785,36 @@ namespace MobileGL {
|
||||
return false;
|
||||
}
|
||||
|
||||
Bool ShaderCompiler::ModuleDeclaresTessellationOrGeometryPointSize(const Vector<Uint32>& spirv) {
|
||||
if (spirv.empty()) {
|
||||
return false;
|
||||
}
|
||||
std::unique_ptr<spvtools::opt::IRContext> 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<spv::Capability>(capability.GetSingleWordInOperand(0));
|
||||
if (declared == spv::Capability::TessellationPointSize ||
|
||||
declared == spv::Capability::GeometryPointSize) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
Bool ShaderCompiler::ModuleDeclaresFloat64(const Vector<Uint32>& spirv) {
|
||||
if (spirv.empty()) {
|
||||
// Same reasoning as ModuleDeclaresBufferTextureSampler: a stage that produced
|
||||
|
||||
@@ -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<Uint32>& 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<Uint32>& spirv);
|
||||
|
||||
// True when the module still declares a 64-bit float type. After
|
||||
// SanitizeAndOptimizeBinary that can only mean DemoteFloat64Pass declined the
|
||||
|
||||
Reference in New Issue
Block a user