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

Improve GLES3 scene renderer compatibility with older devices #81650

Merged
merged 1 commit into from Sep 14, 2023

Conversation

bitsawer
Copy link
Member

Addresses the issue mentioned in #78888 (comment) and also on RocketChat where Shoozza mentioned that replacing the uint(8) with simply 8 helped with their NVIDIA GFX card (the actualy value of 8 will be generated dynamically, I just chose that as example). This probably doesn't fix the actual issue causing #78888 (although it might, I have seen stranger things), but it should help some older devices to work with our OpenGL Compatibility renderer.

It looks like some older ATI and NVDIA drivers and shader compilers don't consider uint(8) a constant expression, so let's go with 8u instead as the shader code using this constant also uses uints. NVIDIA shader compiler is notoriously lax with type conversion and using 8 (signed int) does also work here on most NVIDIA devices (including mine), but it would fail on many other devices.

@bitsawer bitsawer added this to the 4.2 milestone Sep 14, 2023
@bitsawer bitsawer requested a review from a team as a code owner September 14, 2023 13:53
@akien-mga
Copy link
Member

akien-mga commented Sep 14, 2023

Seems fine if it helps, but how about all uses of uint() in the GLES3 backend?

rasterizer_scene_gles3.cpp:		global_defines += "\n#define MAX_FORWARD_LIGHTS uint(" + itos(config->max_lights_per_object) + ")\n";
shader_gles3.cpp:	builder.append("#define ViewIndex uint(0)\n");
shaders/canvas.glsl:	vec2 uv = read_draw_data_src_rect.xy + abs(read_draw_data_src_rect.zw) * ((read_draw_data_flags & FLAGS_TRANSPOSE_RECT) != uint(0) ? vertex_base.yx : vertex_base.xy);
shaders/canvas.glsl:	uint light_count = (read_draw_data_flags >> uint(FLAGS_LIGHT_COUNT_SHIFT)) & uint(0xF); //max 16 lights
shaders/canvas.glsl:		light_base &= uint(0xFF);
shaders/canvas_uniforms_inc.glsl:#define MAX_LIGHTS_PER_ITEM uint(16)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_INSTANCING_MASK uint(0x7F)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_INSTANCING_HAS_COLORS uint(1 << 7)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_INSTANCING_HAS_CUSTOM_DATA uint(1 << 8)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_CLIP_RECT_UV uint(1 << 9)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_TRANSPOSE_RECT uint(1 << 10)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_NINEPACH_DRAW_CENTER uint(1 << 12)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_USING_PARTICLES uint(1 << 13)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_DEFAULT_NORMAL_MAP_USED uint(1 << 26)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_DEFAULT_SPECULAR_MAP_USED uint(1 << 27)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_USE_MSDF uint(1 << 28)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_USE_LCD uint(1 << 29)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_FLIP_H uint(1 << 30)
shaders/canvas_uniforms_inc.glsl:#define FLAGS_FLIP_V uint(1 << 31)
shaders/canvas_uniforms_inc.glsl:#define LIGHT_FLAGS_BLEND_MASK uint(3 << 16)
shaders/canvas_uniforms_inc.glsl:#define LIGHT_FLAGS_BLEND_MODE_ADD uint(0 << 16)
shaders/canvas_uniforms_inc.glsl:#define LIGHT_FLAGS_BLEND_MODE_SUB uint(1 << 16)
shaders/canvas_uniforms_inc.glsl:#define LIGHT_FLAGS_BLEND_MODE_MIX uint(2 << 16)
shaders/canvas_uniforms_inc.glsl:#define LIGHT_FLAGS_BLEND_MODE_MASK uint(3 << 16)
shaders/canvas_uniforms_inc.glsl:#define LIGHT_FLAGS_HAS_SHADOW uint(1 << 20)
shaders/canvas_uniforms_inc.glsl:#define LIGHT_FLAGS_FILTER_MASK uint(3 << 22)
shaders/canvas_uniforms_inc.glsl:#define LIGHT_FLAGS_SHADOW_NEAREST uint(0 << 22)
shaders/canvas_uniforms_inc.glsl:#define LIGHT_FLAGS_SHADOW_PCF5 uint(1 << 22)
shaders/canvas_uniforms_inc.glsl:#define LIGHT_FLAGS_SHADOW_PCF13 uint(2 << 22)
shaders/particles.glsl:#define ATTRACTOR_TYPE_SPHERE uint(0)
shaders/particles.glsl:#define ATTRACTOR_TYPE_BOX uint(1)
shaders/particles.glsl:#define ATTRACTOR_TYPE_VECTOR_FIELD uint(2)
shaders/particles.glsl:#define COLLIDER_TYPE_SPHERE uint(0)
shaders/particles.glsl:#define COLLIDER_TYPE_BOX uint(1)
shaders/particles.glsl:#define COLLIDER_TYPE_SDF uint(2)
shaders/particles.glsl:#define COLLIDER_TYPE_HEIGHT_FIELD uint(3)
shaders/particles.glsl:#define COLLIDER_TYPE_2D_SDF uint(4)
shaders/particles.glsl:#define PARTICLE_FLAG_ACTIVE uint(1)
shaders/particles.glsl:#define PARTICLE_FLAG_STARTED uint(2)
shaders/particles.glsl:#define PARTICLE_FLAG_TRAILED uint(4)
shaders/particles.glsl:#define PARTICLE_FRAME_MASK uint(0xFFFF)
shaders/particles.glsl:#define PARTICLE_FRAME_SHIFT uint(16)
shaders/particles.glsl:	x = ((x >> uint(16)) ^ x) * uint(0x45d9f3b);
shaders/particles.glsl:	x = ((x >> uint(16)) ^ x) * uint(0x45d9f3b);
shaders/particles.glsl:	x = (x >> uint(16)) ^ x;
shaders/particles.glsl:	uint index = uint(gl_VertexID);
shaders/particles.glsl:				seed -= uint(1);
shaders/particles.glsl:			seed *= uint(total_particles);
shaders/particles.glsl:			float random = float(hash(seed) % uint(65536)) / 65536.0;
shaders/particles.glsl:	uint particle_number = (flags >> PARTICLE_FRAME_SHIFT) * uint(total_particles) + index;
shaders/particles_copy.glsl:#define TRANSFORM_ALIGN_DISABLED uint(0)
shaders/particles_copy.glsl:#define TRANSFORM_ALIGN_Z_BILLBOARD uint(1)
shaders/particles_copy.glsl:#define TRANSFORM_ALIGN_Y_TO_VELOCITY uint(2)
shaders/particles_copy.glsl:#define TRANSFORM_ALIGN_Z_BILLBOARD_Y_TO_VELOCITY uint(3)
shaders/particles_copy.glsl:#define PARTICLE_FLAG_ACTIVE uint(1)
shaders/scene.glsl:		for (uint i = uint(0); i < scene_data.directional_light_count; i++) {
shaders/scene.glsl:	for (uint i = uint(0); i < scene_data.directional_light_count; i++) {
shaders/stdlib_inc.glsl:	uint e = f & uint(0x7f800000);
shaders/stdlib_inc.glsl:	if (e <= uint(0x38000000)) {
shaders/stdlib_inc.glsl:		return uint(0);
shaders/stdlib_inc.glsl:		return ((f >> uint(16)) & uint(0x8000)) |
shaders/stdlib_inc.glsl:				(((e - uint(0x38000000)) >> uint(13)) & uint(0x7c00)) |
shaders/stdlib_inc.glsl:				((f >> uint(13)) & uint(0x03ff));
shaders/stdlib_inc.glsl:	uint h_e = h & uint(0x7c00);
shaders/stdlib_inc.glsl:	return ((h & uint(0x8000)) << uint(16)) | uint((h_e >> uint(10)) != uint(0)) * (((h_e + uint(0x1c000)) << uint(13)) | ((h & uint(0x03ff)) << uint(13)));
shaders/stdlib_inc.glsl:	return float2half(floatBitsToUint(v.x)) | float2half(floatBitsToUint(v.y)) << uint(16);
shaders/stdlib_inc.glsl:	return vec2(uintBitsToFloat(half2float(v & uint(0xffff))),
shaders/stdlib_inc.glsl:			uintBitsToFloat(half2float(v >> uint(16))));
shaders/stdlib_inc.glsl:	return uv.x | uv.y << uint(16);
shaders/stdlib_inc.glsl:	return vec2(float(p & uint(0xffff)), float(p >> uint(16))) * 0.000015259021; // 1.0 / 65535.0 optimization
shaders/stdlib_inc.glsl:	return uv.x | uv.y << uint(16);
shaders/stdlib_inc.glsl:	vec2 v = vec2(float(p & uint(0xffff)), float(p >> uint(16)));
shaders/stdlib_inc.glsl:	return uv.x | (uv.y << uint(8)) | (uv.z << uint(16)) | (uv.w << uint(24));
shaders/stdlib_inc.glsl:	return vec4(float(p & uint(0xff)), float((p >> uint(8)) & uint(0xff)), float((p >> uint(16)) & uint(0xff)), float(p >> uint(24))) * 0.00392156862; // 1.0 / 255.0
shaders/stdlib_inc.glsl:	return uv.x | uv.y << uint(8) | uv.z << uint(16) | uv.w << uint(24);
shaders/stdlib_inc.glsl:	vec4 v = vec4(float(p & uint(0xff)), float((p >> uint(8)) & uint(0xff)), float((p >> uint(16)) & uint(0xff)), float(p >> uint(24)));

@bitsawer
Copy link
Member Author

They seem fine, MAX_FORWARD_LIGHTS is a special case here because it's being used as array size, which must be know at compile time and must be a constant. Still, I can double check the others. In theory, there might be some minor optimization penalty if the shader compiler doesn't understand that they are actually constants, but they should still compile and run correctly.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks straightforward.

@akien-mga akien-mga merged commit 1328367 into godotengine:master Sep 14, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@bitsawer bitsawer deleted the fix_gles3_constant branch September 15, 2023 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants