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.08k stars 1.94k forks source link

Compiling shader with greater GLSL version causes texture samplers to not bind properly #2976

Open cykoder opened 1 year ago

cykoder commented 1 year ago

Found this issue last weekend, let me try to explain the problem. I have a very simple shader, like this:

$input v_texcoord0

#include "../common.sh"

SAMPLER2D(s_visibiltyOne, 0);  // Visibility Buffer
SAMPLER2D(materialSampler0, 1); // Diffuse

// Note: had to include this here to prevent "fragData not supported in this glsl version" errors
#if BGFX_SHADER_LANGUAGE_GLSL >= 400
out vec4 bgfx_FragData[5];
#define gl_FragData bgfx_FragData
#endif

void main() {
  vec4 samplerTest = texture2D(s_visibiltyOne, gl_FragCoord.xy);
  vec4 diffuse = texture2D(materialSampler0, gl_FragCoord.xy);

  gl_FragData[0] = diffuse;
  gl_FragData[1] = samplerTest;
  gl_FragData[2] = diffuse;
  gl_FragData[3] = samplerTest;
  gl_FragData[4] = diffuse;
}

and I invoke it like so:

  bgfx::setViewTransform(baseRenderPass, view, proj);
  bgfx::setState(BGFX_STATE_WRITE_RGB | BGFX_STATE_BLEND_NORMAL);

  bgfx::setVertexBuffer(0, m_screenQuadVB);
  bgfx::setIndexBuffer(m_screenQuadIB);

  bgfx::setTexture(0, s_visibiltyOne, *this->defaultMaterialTexture->getTextureHandle());
  bgfx::setTexture(1, materialSamplerHandles[0], *this->defaultNormalTexture->getTextureHandle());

  bgfx::submit(baseRenderPass, *material->getShader()->getProgramHandle());

This works fine when compiled with GLSL 150/300es etc (two textures get bound to the proper sampler), but if i try a higher version such as >= 400 only one texture (the 1st) is getting bound. I checked the generated shader code, and it looks almost identical. Samplers are defined as you would expect. I require to use a higher GLSL version for this shader (it does more than in the snippet, but this is trimmed down for debugging) - yet I cannot bind more than 1 sampler. Also I noticed that setUniform wasnt working either, it was being passed as all 0's for a camera matrix. I can bind buffers however.

Working case in renderdoc: image

And the error case, when compiling with GLSL version 430 through shaderc: image

we can also see that the texture API calls are being called: image

I found that this issue is resolved if I manually set the binding uniform, instead of using SAMPLER2D. EG:

#if BGFX_SHADER_LANGUAGE_GLSL >= 400 // NOTE: 400 is for testing, not sure exactly which version this begins to fail at
layout(binding = 0) uniform sampler2D s_visibiltyOne;
layout(binding = 1) uniform sampler2D materialSampler0;
#else
SAMPLER2D(s_visibiltyOne, 0);  // Visibility Buffer
SAMPLER2D(materialSampler0, 1); // Diffuse
#endif

I am assuming this is either a shader macro fix or something in shaderc, but I'm not sure the best route to take there. Of course its not ideal to have #ifdef hacks in local shader code :)

bkaradzic commented 1 year ago

Most likely your issue is materialSamplerHandles not getting correct reflection information for samplers.

lty123456 commented 1 year ago

My bgfx is half year version,i have same gl_FragData problem few months ago,i use imageStore instead gl_FragData because i don't need depth test,but now i need depth testing so i google this problem again,your code is work for me. by the way,if you implement bgfx::CallbackI::fatal you may get the error desc like:"Failed to compile shader. 0: 0(415) : error C7616: global variable gl_FragData is removed after version 420"

cykoder commented 1 year ago

@lty123456 glad to hear the workaround helped you, to prevent the fatal error you can add:

// Note: had to include this here to prevent "fragData not supported in this glsl version" errors
#if BGFX_SHADER_LANGUAGE_GLSL >= 400
out vec4 bgfx_FragData[5];
#define gl_FragData bgfx_FragData
#endif

add the top of your shader code