bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
15.15k stars 1.95k forks source link

Shaderc incorrectly compiles texelfetch #1538

Open SnapperTT opened 6 years ago

SnapperTT commented 6 years ago

Writing a fragment shader that reads a msaa depth buffer. Shaderc appears to be translating texelfetch incorrectly, it adds a superfluous argument when a sampler2DMS is passed

../../../src/renderer_gl.cpp (5820): BGFX Failed to compile shader. 0: 0(18) : error C1115: unable to find compatible overloaded function "texelFetch(sampler2DMS, ivec2, int, int)"

Note the second int.

Steps to reproduce:

Test frag shader:

$input v_texcoord0

#include <bgfx_shader.shh>

SAMPLER2DMS(s_texDepth, 1);

void main() {
    vec2 texel = v_texcoord0*vec2(1080,720);
    float d = texelFetch(s_texDepth, ivec2(texel), 0).x;
    gl_FragColor = vec4(vec3_splat(d),1.0);
    }

When compiled with shaderc:

./../../tools/shadercRelease -f fs_test.sc -o ../glsl/fs_test.bin --type f -i . --platform linux -p 120

in vec2 v_texcoord0;
uniform sampler2DMS s_texDepth;
void main ()
{
  float tmpvar_1;
  tmpvar_1 = texelFetch (s_texDepth, ivec2((v_texcoord0 * vec2(1080.0, 720.0))), 0, 0).x;
  vec3 tmpvar_2;
  tmpvar_2.x = tmpvar_1;
  tmpvar_2.y = tmpvar_1;
  tmpvar_2.z = tmpvar_1;
  vec4 tmpvar_3;
  tmpvar_3.w = 1.0;
  tmpvar_3.xyz = tmpvar_2;
  gl_FragColor = tmpvar_3;

Note that an extra ", 0" is being added to the texelFetch line:

tmpvar_1 = texelFetch (s_texDepth, ivec2((v_texcoord0 * vec2(1080.0, 720.0))), 0, 0).x;

This causes bgfx to throw the wobbly when it reaches this line. Opening the compiled .bin and removing the extra ", 0" allows the shader to work.

SnapperTT commented 6 years ago

As discussed on gitter, the issue appears to be here: https://github.com/bkaradzic/bgfx/blob/master/3rdparty/glsl-optimizer/src/glsl/ir_print_glsl_visitor.cpp#L939

Shaderc appears to be adding both a LoD and a Sample Index argument if (ir->op == ir_txf_ms). If we are using a multisampled then only sample index is needed

Suggested fix: remove || ir->op == ir_txf_ms from line 939, or update glsl optimiser

In the current version of glsl optimiser this appears to be fixed: https://github.com/aras-p/glsl-optimizer/blob/master/src/glsl/ir_print_glsl_visitor.cpp#L934