Swordfish90 / LibretroDroid

GNU General Public License v3.0
75 stars 18 forks source link

`LCD` and `SHARP` shaders not linking on Mali-470 MP #97

Closed hhromic closed 1 year ago

hhromic commented 2 years ago

I installed latest Lemuroid 1.14.3 from Google Play on my Xiaomi MiTV P1 32" device running Android TV 9. I tested GB, GC, GBA, NES, SNES and MEGADRIVE games, and everything works nicely except for GB, GBC and GBA games. When starting any GB/GBC/GBA game, Lemuroid simply goes back to the game selection screen without any messages.

Using logcat, I managed to trace the problem to a shader linking error when starting GB/GBC/GBA games:

11-20 13:32:10.221 10083 10117 I libretrodroid: Audio initialization has been called with input sample rate 32619
11-20 13:32:10.230 10083 10117 I libretrodroid: Using low latency stream: 0
11-20 13:32:10.230 10083 10117 I libretrodroid: Average audio latency set to: 66.970825 ms
11-20 13:32:10.239 10083 10117 I libretrodroid: GL Version = OpenGL ES 2.0 MI30-03.00.010.02.14.ONETAG.07.0906.1.audio.vbi-6-g5983e067
11-20 13:32:10.239 10083 10117 I libretrodroid: GL Vendor = ARM
11-20 13:32:10.239 10083 10117 I libretrodroid: GL Renderer = Mali-470 MP
11-20 13:32:10.239 10083 10117 I libretrodroid: GL Extensions = GL_EXT_debug_marker GL_OES_texture_npot GL_OES_vertex_array_object GL_OES_compressed_ETC1_RGB8_texture GL_EXT_compressed_ETC1_RGB8_sub_texture GL_OES_standard_derivatives GL_OES_EGL_image GL_OES_depth24 GL_ARM_rgba8 GL_ARM_mali_shader_binary GL_OES_depth_texture GL_OES_packed_depth_stencil GL_EXT_texture_format_BGRA8888 GL_OES_vertex_half_float GL_EXT_blend_minmax GL_OES_EGL_image_external GL_OES_EGL_sync GL_OES_rgb8_rgba8 GL_EXT_multisampled_render_to_texture GL_EXT_discard_framebuffer GL_OES_get_program_binary GL_ARM_mali_program_binary GL_EXT_shader_texture_lod GL_EXT_robustness GL_OES_depth_texture_cube_map GL_KHR_debug GL_ARM_shader_framebuffer_fetch GL_ARM_shader_framebuffer_fetch_depth_stencil GL_OES_mapbuffer GL_KHR_no_error
11-20 13:32:10.239 10083 10117 I libretrodroid: Initializing graphics
11-20 13:32:10.269 10083 10117 E libretrodroid: Could not link program:
11-20 13:32:10.269 10083 10117 E libretrodroid: L0010 Uniform 'textureSize' differ on precision
11-20 13:32:10.269 10083 10117 E libretrodroid:
11-20 13:32:10.269 10083 10117 E libretrodroid: Could not create gl program.

I further discovered that this error only happens with the LCD and SHARP shaders, and actually the emulated system does not matter. It happens by default with GB/GBC/GBA and not with NES/SNES/MEGADRIVE because the LCD shader is selected when using the default Auto filter for these systems in Lemuroid. If I force LCD or SHARP shaders in Lemuroid, then any system gives the same linking error and does not start. Likewise, if I force a working shader, e.g. Smooth, then GB/GBC/GBA games do work as expected.

Note: I use Lemuroid on a Samsung Galaxy A70 and an NVIDIA Shield PRO 2019, where this problem has never happened. Therefore, this is likely a GLSL quirk in the Mali-470 MP GPU used by the Xiaomi Mi TV P1 32".

hhromic commented 2 years ago

After taking a look at the GLSL programs for the shaders in libretrodroid, I noticed that textureSize is defined but actually not used in any of the WORKING (fragment) shaders. In each of the FAILING shaders, textureSize is used once in this line:

mediump vec2 sharpCoords = (floor(screenCoords) + x) / textureSize;
hhromic commented 2 years ago

After researching this problem a bit more, I found this issue mentioning the following:

The GLSL ES Spec (http://www.khronos.org/registry/gles/specs/2.0/GLSL_ES_Specification_1.0.17.pdf) says:

Do precision qualifiers for uniforms need to match? Option 1: Yes. Uniforms are defined to behave as if they are using the same storage in the vertex and fragment processors and may be implemented this way. If uniforms are used in both the vertex and fragment shaders, developers should be warned if the precisions are different. Conversion of precision should never be implicit. Option 2: No. Uniforms may be used by both shaders but the same precision may not be available in both so there is a justification for allowing them to be different. Using the same uniform in the vertex and fragment shaders will always require the precision to be specified in the vertex shader (since the default precision is highp). This is an unnecessary burden on developers. RESOLUTION: Yes, precision qualifiers for uniforms must match.

So, on a device that only supports mediump precision in the fragment shader, it is not legal to declare a single uniform as highp in the vertex shader (the default) and mediump in the fragment shader. One way to resolve this is to declare it mediump everywhere, but this could cause artifacts in some situations. A safer solution is to duplicate the uniform, so the vertex shader gets a highp version and the fragment shader gets a mediump version. The Cesium renderer could do this automatically with a little bit of work.

We saw this problem in Chrome on a Samsung Galaxy Note 10.1, which has a Mali-400MP GPU. On that device, the shader failed to link as a result of the mismatched precision of the czm_viewport uniform (and probably others).

The above seems to suggest that the problem is using the same textureSize variable name in both, the vertex and fragment programs for the failing shaders, but with different declared precisions.

In libretrodroid, the vertex shader program for the failing shaders is this: https://github.com/Swordfish90/LibretroDroid/blob/d74108e3067764ea02653fc506d9ed9c463f52c7/libretrodroid/src/main/cpp/shadermanager.cpp#L22-L40

As it can be seen, textureSize does not have an explicit precision, probably defaulting to highp.

Then, in the failing fragment shader programs, the textureSize variable is declared like this:

#ifdef GL_FRAGMENT_PRECISION_HIGH
#define HIGHP highp
#else
#define HIGHP mediump
precision mediump float;
#endif

uniform HIGHP vec2 textureSize;

In other words, HIGHP in Mali-470 MP devices probably gets defined as mediump causing the mismatch.

According to the referenced issue, the simplest/safest solution would be to rename textureSize in either the vertex or fragment programs so they do not clash during linking time with different precision.

I would love to test this theory but I lack any experience building libretrodroid/lemuroid :( Hope that at least all the above helps shedding a light to the problem.

hhromic commented 2 years ago

The recently added CUTII shader (vertex program) actually seems more correct (uses a #define set for HIGHP): https://github.com/Swordfish90/LibretroDroid/blob/d74108e3067764ea02653fc506d9ed9c463f52c7/libretrodroid/src/main/cpp/shadermanager.cpp#L144-L176

Perhaps the same can be done for defaultShaderVertex ?

Swordfish90 commented 2 years ago

Hi @hhromic I honestly can't thank you enough for this report. This is a very thorough analysis and you're definitely spot on with the diagnosis. Precision qualifies are missing on varying and while the majority of drivers are forgiving, some are not.

I'll definitely fix this with the next release of Lemuroid.

hhromic commented 2 years ago

Hey @Swordfish90, happy to help where I can! Lemuroid is truly a fantastic frontend for casual retrogaming. I'm glad that all the above made sense to you and that you have a fix in mind for the next version!

Feel free to let me know how can I further help testing and troubleshooting this issue. If you share pre-built APKs with potential fixes, I'm more than happy to sideload them into the affected device for testing. Cheers!

Swordfish90 commented 1 year ago

Hi @hhromic . Sorry for the very long delay, but I finally managed to find the time to implement the fix. Are you still up for a test run on your TV? I'm attaching here the apk if you're still interested: https://drive.google.com/file/d/1k1KQWHXsWuF0Ayvmkt01OhKZItxsB7_G/view?usp=share_link

hhromic commented 1 year ago

Hi @Swordfish90, no worries about taking your time! Is understandable that you have a life outside of Lemuroid :) I just tested the APK you shared on the following devices I own:

I'm very happy to report that all the shader linking issues are gone now on the Xiaomi MiTV P1 32" device. 🎉 And can also confirm that all my other devices continue to work great as with the regular Lemuroid version.

I tested explicitly every shader independently and all of them are working fine now on the affected device.

Looking forward for this fix to be released officially, and thanks for the great work you have put into this software.

Swordfish90 commented 1 year ago

Thank you very much for the investigation and the regression tests. I've merged the changes here #98 . I'll release it on Google Play as soon as Jitpack starts cooperating.