Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenGL shaders with derivatives cause fatal errors (using shader profiles higher than 1.0) #3085

Open
EvilTrev opened this issue Apr 18, 2023 · 7 comments

Comments

@EvilTrev
Copy link

Describe the bug
Using derivatives with OpenGL ES shaders (version 3.2 on Android in this case) leads to runtime errors because extensions are being defined after other preprocessor lines.

Shader output starts:

#version 320 es
#define attribute in
#define varying out
precision highp float;
precision highp int;
#extension GL_OES_standard_derivatives : enable

Which leads to this fatal error:

BGFX FATAL 0x00000001: Failed to compile shader. 0: ERROR: 0:6: '' :     GLSL compile error:  Extension directives must occur before any non-preprocessor tokens.

To Reproduce
Create shader using derivatives and attempt to load.

Expected behavior
That it would not define them after other preprocessor defines.

@EvilTrev
Copy link
Author

The fix appears to be in shaderc.cpp, where the logic to append precision should exist much further down

@bkaradzic
Copy link
Owner

Provide shader source that has this issue.

@EvilTrev
Copy link
Author

Any shader that uses dFdx or dFdy, for example you could try adding something like this to one of the shaders:

float textureMipLevel( sampler2D tex, vec2 uv )
{
	// Estimate using OpenGL spec: http://www.opengl.org/registry/doc/glspec42.core.20120427.pdf
	vec2 texel = vec2( textureSize( tex, 0 ).xy ) * uv;
	vec2 dx_vtc = dFdx( texel );
	vec2 dy_vtc = dFdy( texel );
	float delta_max_sqr = max( dot( dx_vtc, dx_vtc ), dot( dy_vtc, dy_vtc ) );
	return 0.5 * log2( delta_max_sqr );
}

But there isn't much need, the issue starts at line 2373 of shaderc.cpp, where precision is injected before many other extensions. The same problems would occur with GL_EXT_shader_texture_lod, GL_OES_texture_3D and so on.

@bkaradzic
Copy link
Owner

See example 42-bunny...

Type make TARGET=4 rebuild, and there is no issue.

@EvilTrev
Copy link
Author

EvilTrev commented Apr 18, 2023

Did you use target profile 310_es or 320_es? It won’t occur for 100_es due to the logic surrounding precision injection in shaderc.cpp

@EvilTrev EvilTrev changed the title OpenGL shaders with derivatives cause fatal errors OpenGL shaders with derivatives cause fatal errors (using shader profiles higher than 1.0) Apr 18, 2023
@EvilTrev
Copy link
Author

EvilTrev commented Apr 18, 2023

Target 4 will build glsl, for android gles it is:

make TARGET=3

And the parameter to shaderc would need to be -p 300_es, so in my makefile I have something like:

ifeq ($(TARGET), 3)
VS_FLAGS=--platform android -p 300_es
FS_FLAGS=--platform android -p 300_es
CS_FLAGS=--platform android -p 300_es
SHADER_PATH=shaders/essl

Which should be fine since the default for BGFX_CONFIG_RENDERER_OPENGLES_MIN_VERSION is 30.

ddf8196 added a commit to ddf8196/bgfx-mcbe that referenced this issue Aug 11, 2023
@jsm174
Copy link

jsm174 commented Jun 22, 2024

I'm having this same exact issue with GLES shaders while trying to get our BGFX port of Visual Pinball running on Android. (I'm testing on a Pixel 7)

I quickly hacked shaderc.cpp, to move all #extension to the top, and it started working.

			if (usesTextureArray)
			{
				bx::stringPrintf(code
					, "#extension GL_EXT_texture_array : enable\n"
					);
			}

			std::istringstream stream(code);
			std::string line;
			std::vector<std::string> extensions;
			std::vector<std::string> otherLines;
			std::string versionLine;

			bool versionLineFound = false;

			while (std::getline(stream, line)) {
				if (line.find("#version") == 0) {
					versionLine = line;
					versionLineFound = true;
				} else if (line.find("#extension") == 0) {
					extensions.push_back(line);
				} else {
					otherLines.push_back(line);
				}
			}

			std::ostringstream output;
			if (versionLineFound) {
				output << versionLine << "\n";
			}
			for (const auto& ext : extensions) {
				output << ext << "\n";
			}
			for (const auto& other : otherLines) {
				output << other << "\n";
			}

			code = output.str();

			if (glsl_profile == 100)
			{

Would a nicer fix be considered for inclusion in shaderc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants