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

Fix Compatibility Rendering (GLES3) on old and low budget devices. #87352

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

Alex2782
Copy link
Contributor

@Alex2782 Alex2782 commented Jan 18, 2024

Fixes #86112
Fixes #86565

Tested devices:

  • moto e5 play (Adreno 3xx)
  • Huawei Mediapad T3 AGS-W09 (Adreno 3xx)
  • Nexus 7 (Adreno 320)
  • LG Nexus 5X (Adreno 418)
  • Honor 7A (Adreno 505)
  • Redmi 4X (Adreno 505)
  • Samsung Tab S7 (Adreno 650)
  • Samsung Tab A6 (2016) SM -T580 Mali-T830
  • 18 devices on Firebase Test Lab (up to Android 9)

TODOs

  • Rotation Bug comment
  • fix lost old callback when continuous call requestRenderAndNotify src diff
  • glLinkProgram shader freeze bug
  • CanvasShaderGLES3: Fragment shader compilation failed
  • VK-GL-CTS
  • more tests (2d, 3d)

@AThousandShips AThousandShips changed the title Fixes Compatibility Rendering (GLES3) on old and low budget devices. Fix Compatibility Rendering (GLES3) on old and low budget devices. Jan 18, 2024
@akien-mga akien-mga added bug platform:android topic:rendering cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Jan 18, 2024
@akien-mga akien-mga added this to the 4.3 milestone Jan 18, 2024
@Alex2782
Copy link
Contributor Author

Notes:

  • I could only reproduce the issue Unable to initialize engine native layer (comment) when OpenGL Debug is activated. Not all devices support the flags _EGL_CONTEXT_FLAGS_KHR, _EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR
  • @joined72 has found and fixed the issue in the file drivers/gles3/shaders/scene.glsl comment. There are more shaders with pow(max(distance, 0.0001), -decay);.

image

@dsnopek
Copy link
Contributor

dsnopek commented Jan 19, 2024

  • joined72 has found and fixed the issue in the file drivers/gles3/shaders/scene.glsl comment. There are more shaders with pow(max(distance, 0.0001), -decay);.

All the shaders in your screenshot are in the RD renderer (meaning Vulkan or DirectX), so they won't have an effect on the GLES3 renderer.

@AThousandShips
Copy link
Member

Might be worth noting in a comment that this is to handle non-compliant devices so no one sees it and "fixes" the unnecessary divide

@joined72
Copy link
Contributor

joined72 commented Jan 19, 2024

@Alex2782 about the "rotation bug" I have tested your gles3jni (https://github.com/Alex2782/ndk-samples) but its rotation is the same as on new devices, I tested on Honor Lite 9 and Redmi 12, same rotation. And about the old 3.0 rotation bug, I'm not able to reproduce it on my Old Andreno (TM) 308 Tablet (just works fine). So currently I need some help to can proceed with my investigation.

Until I'll receive some help about the possible "Wrong Rotation Bug" cause, I'll work on doing more 2D/3D tests.

@clayjohn
Copy link
Member

Amazing work @Alex2782 and @joined72! I'm am extremely impressed with your dedication to figuring out these issues and finding workable solutions.

@Alex2782
Copy link
Contributor Author

Alex2782 commented Jan 20, 2024

@joined72 Could you please test with 1.0?
return nd * (1.0 / pow(max(distance, 0.0001), decay));

On moto e5 play (Firebase Test Lab) freezes again. Maybe I compiled the APK incorrectly. I have switched back to 1 for now to debug orientation.

Shader debug/logging (I think there are no other possibilities, not for GLES3)
https://www.shadertoy.com/view/csfXW7

@Alex2782
Copy link
Contributor Author

@joined72: Can you also see this error?

01-19 16:08:06.236: E/godot(13166): USER ERROR: CanvasShaderGLES3: Fragment shader compilation failed:
01-19 16:08:06.236: E/godot(13166): Fragment shader compilation failed.
01-19 16:08:06.236: E/godot(13166): ERROR: 0:400: '' : Case label has to be a constant integer expression 
01-19 16:08:06.236: E/godot(13166): ERROR: 0:403: '' : Case label has to be a constant integer expression 
01-19 16:08:06.236: E/godot(13166): ERROR: 0:406: '' : Case label has to be a constant integer expression 
01-19 16:08:06.236: E/godot(13166): ERROR: 3 compilation errors.  No code generated.

Bildschirmfoto 2024-01-20 um 02 01 11

@joined72
Copy link
Contributor

joined72 commented Jan 20, 2024

@Alex2782 I'm currently at home so I can't do more tests here, but from my last investigations I found this...

  • return nd * (1.0 / pow(max(distance, 0.0001), decay)); ==> Crashed Shader / App Freeze
  • return nd / pow(max(distance, 0.0001), decay); ==> Crashed Shader / App Freeze
  • return nd * (1 / pow(max(distance, 0.0001), decay)); ==> WORKS FINE!!!

It doesn't seem to be a problem directly linked to a negative exponent in the pow function, especially reading the error that the shader compilation gives you which seems to be linked to a "switch/case" code that is not present in our function. From a first impression it seems like a bug in the shader compiler of the Adreno (TM) 308 Driver, what do you think about?

PS: can you share the project used to generate the "USER ERROR: CanvasShaderGLES3: Fragment shader compilation failed:" error?

@Alex2782
Copy link
Contributor Author

Alex2782 commented Jan 21, 2024

@joined72: ShaderTest.zip
After 3 seconds, a uniform shader is attached to some 2D nodes.

And a 3D scene is loaded after 6 seconds.
image


  • return nd * (1 / pow(max(distance, 0.0001), decay)); ==> WORKS FINE!!!

Unfortunately not, there is no 'freeze' because the shader code is invalid and glLinkProgram is not executed.

video, test 'moto e5 play', empty 3D scene
web-build_20240121_xbm8_pettyl-27-en_US-portrait_video.mp4
logcat, USER ERROR: SceneShaderGLES3 on Samsung Tab S7.
2024-01-21 02:57:32.166 25371-25419 godot                   com.godot.game                       E  USER ERROR: SceneShaderGLES3: Fragment shader compilation failed:
                                                                                                    ERROR: 0:804: '/' :  wrong operand types  no operation '/' exists that takes a left-hand operand of type 'const int' and a right operand of type 'float' (or there is no acceptable conversion)
                                                                                                    ERROR: 0:804: '*' :  wrong operand types  no operation '*' exists that takes a left-hand operand of type 'float' and a right operand of type 'const int' (or there is no acceptable conversion)
                                                                                                    ERROR: 2 compilation errors.  No code generated.
2024-01-21 02:57:32.166 25371-25419 godot                   com.godot.game                       E     at: _display_error_with_code (drivers/gles3/shader_gles3.cpp:252)
2024-01-21 02:57:32.166 25371-25419 godot                   com.godot.game                       E  USER ERROR: Method/function failed.
2024-01-21 02:57:32.166 25371-25419 godot                   com.godot.game                       E     at: _compile_specialization (drivers/gles3/shader_gles3.cpp:394)

The 3D scene is also not displayed as in the video. However, these logs are not output on 'moto e5 play', only USER ERROR: CanvasShaderGLES3

UPDATE: also found the error output USER ERROR: SceneShaderGLES3: Fragment shader compilation failed: on 'moto e5 play'.


Rotation Bug
void RasterizerGLES3::_blit_render_target_to_screen
I think if you set flip_y = false; then the image will be rotated correctly, but this is not a universal solution, only if you want to use it for your personal projects. 😃


UPDATE: I was also able to flip the 2D output using shaders.
/gles3/shaders/canvas.glsl

    gl_Position = screen_transform * vec4(vertex, 0.0, 1.0);
    gl_Position.y *= -1.0; //<--- new code

maybe canvas_transform or screen_transform is calculated incorrectly

@Alex2782
Copy link
Contributor Author

Alex2782 commented Jan 22, 2024

@joined72: android_release-2024-01-22.apk (42 MB)

Can you see the 3D scene on your hardware?
On 'moto e5 play' (Firebase Test Lab) the tests ended too early today. Maybe it is already too much, only 2 GB Ram and in parallel a video is recorded and a robo test is executed.

If you find time again...

/gles3/shaders/scene.glsl src diff

//TODO: Dev-Tests
//++++++++++++++++++++++++++++++++
DISABLE_LIGHTMAP = true
DISABLE_LIGHT_DIRECTIONAL = true
DISABLE_LIGHT_OMNI = true
DISABLE_LIGHT_SPOT = true
DISABLE_FOG = true
//--------------------------------

!!! and !!!

return nd * pow(max(distance, 0.0001), -decay);

If you find time again, set these flags to false. When exactly does it still work and when not? On my Samsung Tab I couldn't see any differences in the 3D Scene. On moto e5 play the 2D Scene was displayed for up to 5 seconds, so no 'freeze'.

image

@joined72
Copy link
Contributor

@Alex2782 is this editor error relevant?

Cannot import custom .glsl shaders when using the gl_compatibility rendering_method. Please switch to the forward_plus or mobile rendering methods to use custom shaders. (User)

@Alex2782
Copy link
Contributor Author

I think can be ignored, only works with Vulkan (button "Compute")

@Alex2782
Copy link
Contributor Author

on moto e5 play

01-22 06:13:15.736: E/godot(20576): USER ERROR: CanvasShaderGLES3: Fragment shader compilation failed:
01-22 06:13:15.736: E/godot(20576): Fragment shader compilation failed.
01-22 06:13:15.736: E/godot(20576): ERROR: 0:405: '' : Case label has to be a constant integer expression 
01-22 06:13:15.736: E/godot(20576): ERROR: 0:408: '' : Case label has to be a constant integer expression 
01-22 06:13:15.736: E/godot(20576): ERROR: 0:411: '' : Case label has to be a constant integer expression 
01-22 06:13:15.736: E/godot(20576): ERROR: 3 compilation errors.  No code generated.

maybe here

01-22 06:13:15.460: I/godot(20576): 85: #define LIGHT_FLAGS_BLEND_MODE_ADD uint(0 << 16)
01-22 06:13:15.460: I/godot(20576): 86: #define LIGHT_FLAGS_BLEND_MODE_SUB uint(1 << 16)
01-22 06:13:15.460: I/godot(20576): 87: #define LIGHT_FLAGS_BLEND_MODE_MIX uint(2 << 16)

01-22 06:13:15.471: I/godot(20576): 403: 	switch (blend_mode) {
01-22 06:13:15.471: I/godot(20576): 404: 		case LIGHT_FLAGS_BLEND_MODE_ADD: {
01-22 06:13:15.471: I/godot(20576): 405: 			color.rgb += light_color.rgb * light_color.a;
01-22 06:13:15.471: I/godot(20576): 406: 		} break;
01-22 06:13:15.471: I/godot(20576): 407: 		case LIGHT_FLAGS_BLEND_MODE_SUB: {
01-22 06:13:15.471: I/godot(20576): 408: 			color.rgb -= light_color.rgb * light_color.a;
01-22 06:13:15.471: I/godot(20576): 409: 		} break;
01-22 06:13:15.471: I/godot(20576): 410: 		case LIGHT_FLAGS_BLEND_MODE_MIX: {
01-22 06:13:15.471: I/godot(20576): 411: 			color.rgb = mix(color.rgb, light_color.rgb, light_color.a);
01-22 06:13:15.471: I/godot(20576): 412: 		} break;
01-22 06:13:15.471: I/godot(20576): 413: 	}

@joined72
Copy link
Contributor

joined72 commented Jan 22, 2024

I have spent a lot of time trying to identify the real issue with the get_omni_spot_attenuation function return line.
I tested a lot of different situations and found differents solutions, but ALL required that the final calculation contain an INT (or best an UINT) value. If this value is used as FLOAT all STOP to works again.
To let you understand, these are some of the working solutions I found:

return nd * (1 / pow(max(distance, 0.0001), decay); // <== Just known solution!
nd *= 1; // <== This fix the issue!!!
return nd * pow(max(distance, 0.0001), -decay;
return float(uint(nd * pow(max(distance, 0.0001), -decay) * 10000.0) / 10000.0); // <= This solution WORKS!

As you can see, the latest solution converts the result into an unsigned integer and, after reconverts it to a float, loses precision after the 4th decimal digit. The latest solution let me think could be an internal Adreno (TM) 308 math processor-related bug.

I think I to continuing to use the first and more concise solution, only adding a detailed comment to prevent others to change it. What do you think about it?

PS: I'm continuing my tests...

@joined72
Copy link
Contributor

maybe here

01-22 06:13:15.471: I/godot(20576): 403: 	switch (blend_mode) {
01-22 06:13:15.471: I/godot(20576): 404: 		case LIGHT_FLAGS_BLEND_MODE_ADD: {
01-22 06:13:15.471: I/godot(20576): 405: 			color.rgb += light_color.rgb * light_color.a;
01-22 06:13:15.471: I/godot(20576): 406: 		} break;

What is the .glsl file?

@Alex2782
Copy link
Contributor Author

CanvasShaderGLES3 = /gles3/shaders/canvas.glsl

and #include "canvas_uniforms_inc.glsl" (I can find the #defines here)


return nd * (1 / pow(max(distance, 0.0001), decay); // <== Just known solution!

USER ERROR: SceneShaderGLES3: Fragment shader compilation failed:

#87352 (comment). (check logcat + video hidden in the <details> tag)

@joined72
Copy link
Contributor

joined72 commented Jan 22, 2024

#87352 (comment). (check logcat + video hidden in the <details> tag)

I'll investigate on it.

I think to have find a viable workaround for the Adreno (TM) 308 "Rotation Bug", I just apply the following changes to the drivers/gles3/rasterizer_gles3.cpp file, function RasterizerGLES3::_blit_render_target_to_screen:

the following code...

glBlitFramebuffer(0, 0, rt->size.x, rt->size.y,
	p_screen_rect.position.x, flip_y ? screen_rect_end.y : p_screen_rect.position.y, screen_rect_end.x, flip_y ? p_screen_rect.position.y : screen_rect_end.y,
	GL_COLOR_BUFFER_BIT, GL_NEAREST);

become...

bool flip_x = false;
bool landscape = screen_rect_end.x > screen_rect_end.y;
if (landscape && RenderingServer::get_singleton()->get_video_adapter_name() == "Adreno (TM) 308") {
	flip_y = false;
	flip_x = true;
}
glBlitFramebuffer(0, 0, rt->size.x, rt->size.y,
	flip_x ? screen_rect_end.x : p_screen_rect.position.x, flip_y ? screen_rect_end.y : p_screen_rect.position.y, 
	flip_x ? p_screen_rect.position.x : screen_rect_end.x, flip_y ? p_screen_rect.position.y : screen_rect_end.y,
	GL_COLOR_BUFFER_BIT, GL_NEAREST);

On all my tests seems to works fine, try on moto e5 play and tell me your results.

@joined72
Copy link
Contributor

@Alex2782

CanvasShaderGLES3 = /gles3/shaders/canvas.glsl

and #include "canvas_uniforms_inc.glsl" (I can find the #defines here)

return nd * (1 / pow(max(distance, 0.0001), decay); // <== Just known solution!

USER ERROR: SceneShaderGLES3: Fragment shader compilation failed:

#87352 (comment). (check logcat + video hidden in the <details> tag)

The unique working solution I found currently is set BASE_PASS = false in scene.glsl file... I'll work to improve it.

@Alex2782
Copy link
Contributor Author

On all my tests seems to works fine, try on moto e5 play and tell me your results.

get_video_adapter_name() == "Adreno (TM) 308"
I think that this code will find it very difficult to be included in the Master branch.
More devices may be affected: https://opengles.gpuinfo.org/

@joined72
Copy link
Contributor

I just tested this PR on the latest 4.3dev3, with a very old Samsung Galaxy S4 (I9505 - Adreno 320) Device (11 years old) and the godot-demo-projects 2D Platform Game:

Standard 4.3dev3... FREEZE... :(
image

This PR... WORKS FINE! ;)
image

Yes, the FPS aren't 60, but we can work on this aspect too, maybe with another PR! ;)

@Alex2782
Copy link
Contributor Author

I have removed ADRENO_3XX_COMPATIBILITY again. If at some point Adreno 3xx should cause problems again just because the glsl files have been changed, then my suggestion would be to put the devices on a 'blacklist' and inform the user directly at startup that the devices are not supported.

Mysterious things happen that I can't explain. 😃 (mostly 'freeze' or 'crash' at glLinkProgram)

@joined72
Copy link
Contributor

joined72 commented Feb 11, 2024

Mysterious things happen that I can't explain. 😃 (mostly 'freeze' or 'crash' at glLinkProgram)

@Alex2782 I just find this...

glLinkProgram crashes on Adreno 3xx

EDIT: and this...

Workaround for a bug in Adreno's shader compiler

but, MUCH, MUCH BETTER HERE!!! ;)

@Alex2782
Copy link
Contributor Author

Alex2782 commented Feb 11, 2024

@joined72 yes, the open source projects are full of them, with Adreno workarounds.

--> Godot + Adreno


Yesterday I tried mediump and lowp (precision qualifiers), I couldn’t see any improvement or deterioration. (canvas.glsl)
On Nexus 7 (Android 6) and Samsung Tab S7 (Android 13)

new version: godot4-bunnymark-gdscript.zip

some details
# bunnymark
var bunnymark_target = 45.0 #60.0
var bunnymark_target_error = 1.5 #0.5

Settings for older devices. When the app is freshly installed, slightly better results are achieved. After 3 starts at the latest, the results are somewhat worse, terminating the app via system settings or deactivating the shader cache did not help. Reproduced this behavior on both devices.

  • on Nexus 7 (Adreno 320) --> 800 - 1150 bunnies. (Full HD resolution)
  • on Samsung Tab (Adreno 650) --> 13000 - 13500 bunnies. (2k resolution)

@joined72
Copy link
Contributor

@Alex2782

@joined72 yes, the open source projects are full of them, with Adreno workarounds.

--> Godot + Adreno

Yes, but what I intend is that we can try to study and reproduce all the Google Filament Adreno Fixes and Workaround! ;)

@Alex2782
Copy link
Contributor Author

Currently, perhaps a maximum of 4% of PlayStore users have Adreno 3xx. At some point, the effort will not be worth it. For example, I had to solve a conflict on the canvas.glsl file twice in the last 3 weeks because there were other optimizations and improvements. There are over 100 volunteer developers, so we can't really dictate what code they can use to make it work on old devices.

But I would like to see at least a Godot 4 version that doesn't cause problems on old Adreno GPUs. If someone really needs it, they can always fall back on the one version.

@joined72
Copy link
Contributor

joined72 commented Feb 12, 2024

Currently, perhaps a maximum of 4% of PlayStore users have Adreno 3xx. At some point, the effort will not be worth it. For example, I had to solve a conflict on the canvas.glsl file twice in the last 3 weeks because there were other optimizations and improvements. There are over 100 volunteer developers, so we can't really dictate what code they can use to make it work on old devices.

@Alex2782 today I tested 4.3dev3 standard (without out PR), on an Honor 7A (Adreno 505) just to test the speed on old devices, and I see another crash with the following logcat...

--------- beginning of crash
02-12 11:12:18.822  2591  2689 F libc    : Fatal signal 11 (SIGSEGV), code 1, fault addr 0xc1183a01 in tid 2689 (GLThread 419)

after I remember one of the issue out PR fixes... #86565 ...
...and this issue is related to another Adreno 505 device (Xiaomi Redmi 4X), and the Adreno 5XX family isn't old as the 3XX is, so my consideration is... perhaps this PR could be more "needed" than we think? ;)

PS: in the next few days I'll receive some old/used Adreno 4XX and Adreno 5XX devices, just ordered to can do much deep tests, I'll update you as soon as I have them in my hands. ;)

PPS: disabling the "Shader Compiler/Shader Cache" option I was able to run the Demo without our PR. Are always Shader related issues...

EDIT: anyway the speed of the 2D Platform Demo on the Adreno 505 device is of about 57/58 FPS, that is a more that respectable speed to can play the game.

@Alex2782
Copy link
Contributor Author

Oh ok, I think in the scene.glsl file Adreno 4xx and 5xx also need the workaround.

@Giwayume
Copy link
Contributor

Test PR #86564 (CUSTOM0, CUSTOM1, OpenGL ES: CanvasShaderTest.zip)
I'm not sure if the triangle in the first screenshot has to be white.

I left comments in the code on what is supposed to happen, but basically the white triangle is testing that adding material to a mesh surface in 2D doesn't cause a crash. That operation was never actually supported in 2D, so triangle is white.

@joined72
Copy link
Contributor

@Alex2782 the commit 08f4560 caused a new crash:

USER ERROR: SceneShaderGLES3: Fragment shader compilation failed:
E godot   : Fragment shader compilation failed.
E godot   : ERROR: 0:970: '-' :  wrong operand types  no operation '-' exists that takes a left-hand operand of type 'const int' and a right operand of type 'float' (or there is no acceptable conversion)
E godot   : ERROR: 0:970: 'assign' :  cannot convert from 'const int' to 'float'
E godot   : ERROR: 2 compilation errors.  No code generated.

To fix it we have to replace:

fog_amount = 1 - exp(min(0.0, -length(vertex) * scene_data.fog_density));

with...

fog_amount = 1.0 - exp(min(0.0, -length(vertex) * scene_data.fog_density));

@dsnopek
Copy link
Contributor

dsnopek commented Feb 21, 2024

@Alex2782 the commit 08f4560 caused a new crash:

This new crash affects all Android, not only low budget devices.

I just posted PR #88633 with this fix!

drivers/gles3/shaders/scene.glsl Outdated Show resolved Hide resolved
drivers/gles3/shaders/canvas.glsl Outdated Show resolved Hide resolved
drivers/gles3/shaders/canvas.glsl Outdated Show resolved Hide resolved
drivers/gles3/shaders/canvas.glsl Show resolved Hide resolved
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks great to me! I can't validate the android platform changes, but they look safe. The rendering changes all seem fine from my perspective, so I would go ahead and merge this now

@akien-mga akien-mga merged commit 7359ec7 into godotengine:master Feb 22, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks! Amazing work @Alex2782 and @joined72!

@Alex2782 Alex2782 deleted the fix_opengl_es3 branch February 22, 2024 11:48
@clayjohn clayjohn removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 11, 2024
@hsandt
Copy link
Contributor

hsandt commented Mar 26, 2024

So does this PR fixes shader-related crashes too (using even the simplest .glsl shader), such as #74954 or the last two mentioned below #82602 and #79760? (not sure about #86037 which doesn't mention a shader in description), on devices that do not crash when not using a shader?

Even this uniform GodotShader crashes on Nexus 7 (Adreno 320). I created this test 2 months ago to test the crashes with Vulkan renderer. OpenGL works there: #86037, #82602, #79760 (I think other issues, because they are different glsl files)

shader_type canvas_item;

global uniform vec4 global_color;

void fragment() {
	COLOR = global_color;
}

I have compiled Godot recently so I could try a quick recompile with this PR and test on my Fairphone 3, which crashed on even the simple shader above (Sprite + ShaderMaterial using it).

@Alex2782
Copy link
Contributor Author

I think no, only Compatibility Rendering (GLES3)

Bildschirmfoto 2024-03-26 um 18 29 16

Uniform Crash (Vulkan: Forward+ / Mobile):
#79760
#85097
#82602
#86037

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