Skip to content

Commit f7ad672

Browse files
bkaradzic-microsoftbkaradzicCopilot
authored
ShaderCompiler: fix 3-argument texture() rejected by glslang (#1755)
## Summary The sampler vertical-flip applied on the **D3D / Metal / Vulkan** backends was injected as a **2-argument** function-like macro: ```glsl #define texture(x,y) texture(x, flip(y)) #define textureLod(x,y,z) textureLod(x, flip(y), z) ``` This cannot match the **3-argument** bias form `texture(sampler, uv, bias)` emitted by some Babylon.js shaders (e.g. GreasedLine: `texture(grl_colors, uv, 0.)`), and glslang's preprocessor has **no variadic-macro support** (`Too many args in macro`), so those shaders failed to compile — surfacing as a `BJS - Error compiling effect` retry loop that hangs the test. ## Fix Replace the `texture()`/`textureLod()` macros with a `FlipSamplerCoordinates` **AST traverser** that rewrites a 2-component sample coordinate to `vec2(u, 1.0 - v)` for any call arity and sampler type. It runs only on the backends that already apply `ProcessSamplerFlip` (D3D, Metal, Vulkan); the OpenGL backend shares bgfx's V-orientation and is unaffected. `texelFetch` keeps its macro (integer texel coordinates), and 3D/cube/array (vec3+) coordinates are left untouched, matching the previous `flip(vec2)`/`flip(vec3)` behavior. Fixes [ThePirateCove#1656](BabylonJS/ThePirateCove#1656). ## Verification (D3D11 Debug) - GreasedLine idx 149 / 150 now **compile and render** — no hang, no `Error compiling effect`. - **No regression** across 20 diverse texture / postprocess / shadow / refraction / glow / cubemap tests (idx 5, 8, 25, 35, 37, 155, 373, 396, 410, 487, 4, 23, 32, 36, 49, 131, 135, 254, 343, 394). - OpenGL backend build-checked (`GRAPHICS_API=OpenGLWindowsDevOnly`); the unchanged sampler path compiles clean. ## Note This PR originally also bumped the bgfx transient vertex-buffer limits for [ThePirateCove#1570](BabylonJS/ThePirateCove#1570). On investigation that turned out to be a misdiagnosis (the overflow is a nanovg Canvas burst, not Gaussian Splatting / vertex pulling), so that change was dropped — details posted on #1570. Co-authored-by: Branimir Karadzic <branimirkaradzic@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 063244d commit f7ad672

7 files changed

Lines changed: 105 additions & 11 deletions

Plugins/ShaderCompiler/Source/ShaderCompilerCommon.cpp

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,14 @@ namespace Babylon::ShaderCompilerCommon
3535
throw std::runtime_error{"ProcessSamplerFlip: Could not find shader name define."};
3636
}
3737

38+
// The vertical (V) flip applied to texture()/textureLod() sample coordinates is performed by
39+
// the FlipSamplerCoordinates AST traverser, not by a preprocessor macro. A 2-argument
40+
// function-like macro (`#define texture(x,y) texture(x, flip(y))`) cannot match the
41+
// 3-argument bias form `texture(sampler, uv, bias)` emitted by some Babylon.js shaders (e.g.
42+
// GreasedLine), and glslang's preprocessor has no variadic-macro support, so those shaders
43+
// failed to compile. texelFetch keeps its macro because it takes integer texel coordinates,
44+
// which the float-coordinate AST flip does not handle.
3845
static const auto textureSamplerFunctions = R"(
39-
highp vec2 flip(highp vec2 uv)
40-
{
41-
return vec2(uv.x, 1. - uv.y);
42-
}
43-
highp vec3 flip(highp vec3 uv)
44-
{
45-
return uv;
46-
}
47-
48-
#define texture(x,y) texture(x, flip(y))
49-
#define textureLod(x,y,z) textureLod(x, flip(y), z)
5046
#define texelFetch(tex, uv, lod) texelFetch((tex), ivec2((uv).x, textureSize((tex), (lod)).y - 1 - (uv).y), (lod))
5147
#define SHADER_NAME)";
5248

Plugins/ShaderCompiler/Source/ShaderCompilerDXBC.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ namespace Babylon::Plugins
102102
}
103103

104104
ShaderCompilerTraversers::IdGenerator ids{};
105+
// Flip 2D texture sample coordinates (replaces the former ProcessSamplerFlip texture() macro).
106+
ShaderCompilerTraversers::FlipSamplerCoordinates(program);
105107
auto cutScope = ShaderCompilerTraversers::ChangeUniformTypes(program, ids);
106108
auto utstScope = ShaderCompilerTraversers::MoveNonSamplerUniformsIntoStruct(program, ids);
107109
std::map<std::string, std::string> vertexAttributeRenaming = {};

Plugins/ShaderCompiler/Source/ShaderCompilerDXIL.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,8 @@ namespace Babylon::Plugins
178178
}
179179

180180
ShaderCompilerTraversers::IdGenerator ids{};
181+
// Flip 2D texture sample coordinates (replaces the former ProcessSamplerFlip texture() macro).
182+
ShaderCompilerTraversers::FlipSamplerCoordinates(program);
181183
auto cutScope = ShaderCompilerTraversers::ChangeUniformTypes(program, ids);
182184
auto utstScope = ShaderCompilerTraversers::MoveNonSamplerUniformsIntoStruct(program, ids);
183185
std::map<std::string, std::string> vertexAttributeRenaming = {};

Plugins/ShaderCompiler/Source/ShaderCompilerMetal.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ namespace Babylon::Plugins
103103
}
104104

105105
ShaderCompilerTraversers::IdGenerator ids{};
106+
// Flip 2D texture sample coordinates (replaces the former ProcessSamplerFlip texture() macro).
107+
ShaderCompilerTraversers::FlipSamplerCoordinates(program);
106108
auto cutScope = ShaderCompilerTraversers::ChangeUniformTypes(program, ids);
107109
auto utstScope = ShaderCompilerTraversers::MoveNonSamplerUniformsIntoStruct(program, ids);
108110
std::map<std::string, std::string> vertexAttributeRenaming = {};

Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.cpp

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1673,6 +1673,76 @@ namespace Babylon::ShaderCompilerTraversers
16731673

16741674
TIntermediate* m_intermediate{};
16751675
};
1676+
1677+
class FlipSamplerCoordinatesTraverser : public TIntermTraverser
1678+
{
1679+
public:
1680+
static void Traverse(TProgram& program)
1681+
{
1682+
for (auto stage : {EShLangVertex, EShLangFragment})
1683+
{
1684+
auto* intermediate{program.getIntermediate(stage)};
1685+
if (intermediate != nullptr)
1686+
{
1687+
FlipSamplerCoordinatesTraverser traverser{intermediate};
1688+
intermediate->getTreeRoot()->traverse(&traverser);
1689+
}
1690+
}
1691+
}
1692+
1693+
protected:
1694+
virtual bool visitAggregate(TVisit visit, TIntermAggregate* node) override
1695+
{
1696+
if (visit == EvPreVisit && (node->getOp() == EOpTexture || node->getOp() == EOpTextureLod))
1697+
{
1698+
auto& sequence = node->getSequence();
1699+
if (sequence.size() >= 2)
1700+
{
1701+
// The coordinate is the operand right after the sampler. Only 2-component
1702+
// float coordinates (sampler2D-style) are flipped; cube/array/3D coordinates
1703+
// (vec3+) are left untouched, matching the original flip(vec2)/flip(vec3).
1704+
auto* coordinate = sequence[1]->getAsTyped();
1705+
if (coordinate != nullptr &&
1706+
coordinate->getType().getBasicType() == EbtFloat &&
1707+
!coordinate->getType().isArray() &&
1708+
coordinate->getType().getVectorSize() == 2)
1709+
{
1710+
sequence[1] = FlipVerticalCoordinate(coordinate);
1711+
}
1712+
}
1713+
}
1714+
1715+
return true;
1716+
}
1717+
1718+
private:
1719+
FlipSamplerCoordinatesTraverser(TIntermediate* intermediate)
1720+
: m_intermediate{intermediate}
1721+
{
1722+
}
1723+
1724+
// Builds `coordinate * vec2(1.0, -1.0) + vec2(0.0, 1.0)`, i.e. (u, 1.0 - v).
1725+
TIntermTyped* FlipVerticalCoordinate(TIntermTyped* coordinate)
1726+
{
1727+
const TSourceLoc& loc{coordinate->getLoc()};
1728+
TType vec2Type{EbtFloat, EvqConst, 2};
1729+
1730+
TConstUnionArray scaleValues{2};
1731+
scaleValues[0].setDConst(1.0);
1732+
scaleValues[1].setDConst(-1.0);
1733+
TIntermTyped* scale{m_intermediate->addConstantUnion(scaleValues, vec2Type, loc)};
1734+
1735+
TConstUnionArray offsetValues{2};
1736+
offsetValues[0].setDConst(0.0);
1737+
offsetValues[1].setDConst(1.0);
1738+
TIntermTyped* offset{m_intermediate->addConstantUnion(offsetValues, vec2Type, loc)};
1739+
1740+
TIntermTyped* scaled{m_intermediate->addBinaryMath(EOpMul, coordinate, scale, loc)};
1741+
return m_intermediate->addBinaryMath(EOpAdd, scaled, offset, loc);
1742+
}
1743+
1744+
TIntermediate* m_intermediate{};
1745+
};
16761746
}
16771747

16781748
ScopeT MoveNonSamplerUniformsIntoStruct(TProgram& program, IdGenerator& ids)
@@ -1719,4 +1789,9 @@ namespace Babylon::ShaderCompilerTraversers
17191789
{
17201790
InvertYDerivativeOperandsTraverser::Traverse(program);
17211791
}
1792+
1793+
void FlipSamplerCoordinates(TProgram& program)
1794+
{
1795+
FlipSamplerCoordinatesTraverser::Traverse(program);
1796+
}
17221797
}

Plugins/ShaderCompiler/Source/ShaderCompilerTraversers.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,4 +134,19 @@ namespace Babylon::ShaderCompilerTraversers
134134
/// https://github.com/bkaradzic/bgfx/blob/7be225bf490bb1cd231cfb4abf7e617bf35b59cb/src/bgfx_shader.sh#L44-L45
135135
/// https://github.com/bkaradzic/bgfx/blob/7be225bf490bb1cd231cfb4abf7e617bf35b59cb/src/bgfx_shader.sh#L62-L65
136136
void InvertYDerivativeOperands(glslang::TProgram& program);
137+
138+
/// Flip the vertical (V) component of 2D texture sample coordinates, i.e. rewrite the
139+
/// coordinate `uv` of every `texture()`/`textureLod()` call to `vec2(uv.x, 1.0 - uv.y)`.
140+
///
141+
/// bgfx's D3D/Metal/Vulkan backends sample textures with the opposite V-orientation from
142+
/// WebGL/OpenGL. This was historically corrected by injecting a `#define texture(x,y)
143+
/// texture(x, flip(y))` preprocessor macro into the shader source, but a 2-argument
144+
/// function-like macro cannot match the 3-argument bias form `texture(sampler, uv, bias)`
145+
/// that some Babylon.js shaders emit (e.g. GreasedLine), and glslang's preprocessor has no
146+
/// variadic-macro support. Performing the flip on the AST handles every texture()/textureLod()
147+
/// arity and sampler type. Only 2-component (sampler2D-style) coordinates are flipped, matching
148+
/// the previous `flip(vec2)`/`flip(vec3)` overloads where vec3+ coordinates were left untouched.
149+
/// Must only be used on the backends that apply ProcessSamplerFlip (D3D, Metal, Vulkan); the
150+
/// OpenGL backend shares bgfx's V-orientation and must not flip.
151+
void FlipSamplerCoordinates(glslang::TProgram& program);
137152
}

Plugins/ShaderCompiler/Source/ShaderCompilerVulkan.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ namespace Babylon::Plugins
7878
}
7979

8080
ShaderCompilerTraversers::IdGenerator ids{};
81+
// Flip 2D texture sample coordinates (replaces the former ProcessSamplerFlip texture() macro).
82+
ShaderCompilerTraversers::FlipSamplerCoordinates(program);
8183
auto cutScope = ShaderCompilerTraversers::ChangeUniformTypes(program, ids);
8284
auto utstScope = ShaderCompilerTraversers::MoveNonSamplerUniformsIntoStruct(program, ids);
8385
std::map<std::string, std::string> vertexAttributeRenaming = {};

0 commit comments

Comments
 (0)