[Fix] (Tessellation): bound the pass-through control-stage cache and stop baking "draw nothing" for levels GL clamps

This commit is contained in:
Swung0x48
2026-08-27 03:18:14 -04:00
parent 2635fe84b6
commit 31252cf0da
7 changed files with 104 additions and 52 deletions
+2 -16
View File
@@ -835,20 +835,6 @@ 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,
@@ -885,11 +871,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
source += " gl_out[gl_InvocationID].gl_Position = gl_in[gl_InvocationID].gl_Position;\n";
for (Uint i = 0; i < 4; ++i) {
source += " gl_TessLevelOuter[" + std::to_string(i) +
"] = " + TessLevelLiteral(defaultOuterLevel[i]) + ";\n";
"] = " + MG_Util::ShaderTranspiler::TessellationLevelLiteral(defaultOuterLevel[i]) + ";\n";
}
for (Uint i = 0; i < 2; ++i) {
source += " gl_TessLevelInner[" + std::to_string(i) +
"] = " + TessLevelLiteral(defaultInnerLevel[i]) + ";\n";
"] = " + MG_Util::ShaderTranspiler::TessellationLevelLiteral(defaultInnerLevel[i]) + ";\n";
}
source += "}\n";
return source;
@@ -3597,19 +3597,6 @@ namespace MobileGL::MG_Backend::DirectVulkan {
}
}
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) {
@@ -3683,11 +3670,11 @@ namespace MobileGL::MG_Backend::DirectVulkan {
source += " gl_out[gl_InvocationID].gl_Position = gl_in[gl_InvocationID].gl_Position;\n";
for (Uint32 i = 0; i < 4; ++i) {
source += " gl_TessLevelOuter[" + std::to_string(i) +
"] = " + TessLevelLiteral(defaultOuterLevel[i]) + ";\n";
"] = " + MG_Util::ShaderTranspiler::TessellationLevelLiteral(defaultOuterLevel[i]) + ";\n";
}
for (Uint32 i = 0; i < 2; ++i) {
source += " gl_TessLevelInner[" + std::to_string(i) +
"] = " + TessLevelLiteral(defaultInnerLevel[i]) + ";\n";
"] = " + MG_Util::ShaderTranspiler::TessellationLevelLiteral(defaultInnerLevel[i]) + ";\n";
}
source += "}\n";
return source;
@@ -3707,6 +3694,26 @@ namespace MobileGL::MG_Backend::DirectVulkan {
return cached->second;
}
// The key stopped being bounded when the levels joined it: patchVertices alone could only
// take 32 values, but six unclamped application floats can take any number, and an
// application that ramps a level per frame would retain one VkShaderModule per frame for
// the lifetime of the device. Flushed wholesale rather than aged: a module is not
// referenced by the pipelines built from it (Vulkan copies what it needs at
// vkCreateGraphicsPipelines), everything here runs on the GL thread, and an application
// that can overflow this cap is already recompiling every frame - so the flush costs it
// nothing it was not paying anyway.
if (m_passthroughTessControlStages.size() >= kMaxPassthroughTessControlStages) {
MGLOG_D("ProgramFactory: flushing %zu pass-through tessellation control stages; the application has "
"used more than %zu distinct (patch size, default level) combinations",
m_passthroughTessControlStages.size(), kMaxPassthroughTessControlStages);
for (auto& entry : m_passthroughTessControlStages) {
if (entry.second.module != VK_NULL_HANDLE) {
vkDestroyShaderModule(m_device, entry.second.module, nullptr);
}
}
m_passthroughTessControlStages.clear();
}
VkPipelineShaderStageCreateInfo stage{VK_STRUCTURE_TYPE_PIPELINE_SHADER_STAGE_CREATE_INFO};
stage.stage = VK_SHADER_STAGE_TESSELLATION_CONTROL_BIT;
stage.module = VK_NULL_HANDLE;
@@ -570,11 +570,16 @@ namespace MobileGL::MG_Backend::DirectVulkan {
IEvictionObserver* m_evictionObserver = nullptr;
// 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.
// 64-bit key by ComputePassthroughTessControlKey (the levels are float state, so the map
// cannot simply be keyed on the patch size any more). A failed build is cached as
// VK_NULL_HANDLE so a broken generator costs one compile, not one per draw.
//
// Hard-capped, because the key is application-controlled: glPatchParameterfv clamps
// nothing, so an application that recomputes a level per frame mints a new key per frame.
// Reaching the cap destroys every module and starts over (see the flush in
// GetOrCreatePassthroughTessControlStage); the cap is far above what any program that
// holds its levels still will ever need.
static constexpr SizeT kMaxPassthroughTessControlStages = 64;
UnorderedMap<Uint64, VkPipelineShaderStageCreateInfo> m_passthroughTessControlStages;
static inline XXH64_state_t* m_hashState = XXH64_createState();
};
@@ -1314,7 +1314,7 @@ 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.000000;")) << level << "\n" << out;
EXPECT_TRUE(Contains(out, String(level) + " = 1.0;")) << 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.
@@ -1327,12 +1327,12 @@ TEST(PassthroughTessControlEsslTest, DeclaresThePatchSizeAndWritesEveryTessLevel
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;
EXPECT_TRUE(Contains(out, "gl_TessLevelOuter[0] = 2.0;")) << out;
EXPECT_TRUE(Contains(out, "gl_TessLevelOuter[1] = 3.0;")) << out;
EXPECT_TRUE(Contains(out, "gl_TessLevelOuter[2] = 4.0;")) << out;
EXPECT_TRUE(Contains(out, "gl_TessLevelOuter[3] = 5.0;")) << out;
EXPECT_TRUE(Contains(out, "gl_TessLevelInner[0] = 6.5;")) << out;
EXPECT_TRUE(Contains(out, "gl_TessLevelInner[1] = 7.25;")) << out;
}
// Every level literal carries a decimal point even when the value is integral: ESSL reads
@@ -1344,22 +1344,36 @@ TEST(PassthroughTessControlEsslTest, SpellsIntegralLevelsAsFloatLiterals) {
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) {
// glPatchParameterfv accepts any float, NaN and infinity included, and GL 4.6 core 11.2.2
// discards a patch ONLY when a relevant outer level is <= 0 - everything else is clamped into
// [1, MAX_TESS_GEN_LEVEL]. So the three non-finite inputs do not share one answer: NaN is
// unspecified and 0.0 is the safe reading, -inf really does discard, and +inf must tessellate at
// the maximum. Baking 0.0 for +inf inverted "as finely as possible" into "draw nothing".
TEST(PassthroughTessControlEsslTest, NonFiniteLevelsFollowTheDiscardRule) {
const Float notANumber = std::numeric_limits<Float>::quiet_NaN();
const Float infinity = std::numeric_limits<Float>::infinity();
const String out = BuildPassthroughTessControlEssl(320, 4, "", "",
FloatVec4(notANumber, infinity, 1.0f, 1.0f),
FloatVec4(notANumber, -infinity, infinity, 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_FALSE(Contains(out, "gl_TessLevelOuter[2] = 0.0;"))
<< "a positive infinity clamps to GL_MAX_TESS_GEN_LEVEL, not to a discarded patch" << out;
EXPECT_TRUE(Contains(out, "gl_TessLevelInner[0] = 0.0;")) << out;
EXPECT_FALSE(Contains(out, "nan")) << out;
EXPECT_FALSE(Contains(out, "inf")) << out;
}
// A level below the old six-decimal format's resolution is still a POSITIVE level, which GL clamps
// to 1 and draws; rendering it as "0.000000" discarded the patch instead.
TEST(PassthroughTessControlEsslTest, TinyPositiveLevelsDoNotFlushToZero) {
const String out = BuildPassthroughTessControlEssl(320, 4, "", "",
FloatVec4(1e-7f, 1.0f, 1.0f, 1.0f),
FloatVec2(1.0f, 1.0f));
EXPECT_FALSE(Contains(out, "gl_TessLevelOuter[0] = 0.0;")) << out;
EXPECT_FALSE(Contains(out, "gl_TessLevelOuter[0] = 0.000000;")) << 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) {
@@ -179,10 +179,10 @@ 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;
EXPECT_NE(source.find("gl_TessLevelOuter[0] = 2.0;"), String::npos) << source;
EXPECT_NE(source.find("gl_TessLevelOuter[3] = 5.0;"), String::npos) << source;
EXPECT_NE(source.find("gl_TessLevelInner[0] = 6.5;"), String::npos) << source;
EXPECT_NE(source.find("gl_TessLevelInner[1] = 7.25;"), String::npos) << source;
const Uint64 defaultKey =
ProgramFactory::ComputePassthroughTessControlKey(4, kDefaultOuter, kDefaultInner);
@@ -12,6 +12,10 @@
#include "ShaderCompiler.h"
#include <format>
#include <cmath>
#include "SpirvPasses/EliminateFloatEqualsZeroPass.h"
#include "SpirvPasses/FlattenInterfaceStructPass.h"
#include "SpirvPasses/RenameSamplerFunctionParameterPass.h"
@@ -64,6 +68,30 @@
namespace MobileGL {
namespace MG_Util {
namespace ShaderTranspiler {
// Above every plausible GL_MAX_TESS_GEN_LEVEL (the GL core minimum is 64), so it lands on the
// same clamped result the device's own maximum would. +inf has to reach the tessellator as
// "as finely as possible", not as "discard".
static constexpr const char* kClampedHighTessLevelLiteral = "65536.0";
String TessellationLevelLiteral(Float value) {
// GL leaves a NaN level unspecified; 0.0 is the safe reading, and unlike "nan" it compiles.
if (std::isnan(value)) return "0.0";
// -inf is <= 0 and discards the patch, exactly like 0.0. +inf clamps to the maximum.
if (std::isinf(value)) return value > 0.0f ? kClampedHighTessLevelLiteral : "0.0";
// Shortest round-trip, not a fixed six decimals: "{:.6f}" renders every level below ~5e-7
// as "0.000000", which turns a positive level GL would clamp to 1 into a discarded patch.
String text = std::format("{}", value);
// ...but shortest round-trip spells an integral value as a bare digit sequence, which GLSL
// reads as an INT literal, so the decimal point has to be put back when nothing else marks
// the literal as floating point.
if (text.find('.') == String::npos && text.find('e') == String::npos &&
text.find('E') == String::npos) {
text += ".0";
}
return text;
}
// `env` is the compile-time backend snapshot; null means "resolve from the live
// backend", which is what the standalone/test entry points do. The pipeline always
// passes one, so a worker never reaches pActiveBackendObject through here.
@@ -18,6 +18,18 @@
namespace MobileGL {
namespace MG_Util {
namespace ShaderTranspiler {
// A GLSL float literal for a tessellation level, for the pass-through tessellation
// control stage both backends synthesize when a program has an evaluation stage and no
// control stage. Shared so the two generators cannot disagree about what a level means.
//
// GL 4.6 core 11.2.2 discards a patch only when a relevant OUTER level is <= 0; every
// other value is CLAMPED into [1, MAX_TESS_GEN_LEVEL]. So "draw nothing" is reserved
// for the values that really mean it, and everything else has to survive the trip
// through text: a shortest-round-trip spelling, because a fixed-decimal one flushes
// small positive levels to zero, and always with a '.' or an exponent, because a bare
// digit sequence is an INT literal and `gl_TessLevelOuter[0] = 1;` does not compile.
String TessellationLevelLiteral(Float value);
class ShaderCompiler {
public:
static Result<SharedPtr<glslang::TShader>> CompileShader(const ShaderAttrib& attrib);