[3.x] Drop broken Android 32-bit framebuffer setting

[RandomShaper]

Android's graphics/32_bits_framebuffer export setting not being applied like what you would expect. Moreover, it was being mapped to the --use_depth_32 command line switch, but it was never possible to set the bit depth of the depth buffer to 32.

To me, during the transition to Godot 3.0, where a 32-bit frambebuffer was not an opt-in anymore, traces of that were kept while the 32-bit depth buffer setting was only partially implemented. (Or maybe --use_depth_32 didn't have anything to do with depth buffers, being about the bit depth of the frame buffer, but the main reasoning still holds.)

This PR aims to bring sanity back.

WARNING: The graphics/32_bits_framebuffer export setting is gone. It was not even being applied as expected in every case and the GL rasterizer doesn't have explicit support for a 16-bit framebuffer. Therefore, a 16-bit framebuffer is only created as a desperate fallback in cases where 32-bit is not supported (still relevant?). With the new graphics/depth_buffer_bits, you can choose between a 32, 24 (the default) and 16-bit depth buffer, which is more useful in adapting the renderer to the needs of the project. I'm marking this PR as Break compat, but most project shouldn't really notice a difference.

UPDATE: Code and PR description updated following conversation in the comments.

---

Version for 4.0 submitted as #54463.

[Calinou]

With the new graphics/32_bits_depthbuffer, you can choose between a 32-bit or 24-bit (the default) depthbuffer, which is more useful in adapting the renderer to the needs of the project.

As far as I know, we don't provide a 32-bit depth buffer option on desktop platforms yet. Use cases where you actually need a 32-bit depth buffer are few and far between, especially on mobile where large-scale games are less viable for performance reasons.

A 32-bit depth buffer is a good option to provide in the long run, but it should be disabled by default to use a 24-bit depth buffer instead. On low-end platforms (mobile and integrated graphics), a 16-bit depth buffer can also be handy to further improve performance.

[RandomShaper]

Then maybe there should be a setting to pick between a prefferred bit depth of 32, 24 and 16?

[Calinou]

Then maybe there should be a setting to pick between a prefferred bit depth of 32, 24 and 16?

Sounds good to me 🙂

[RandomShaper]

@Calinou, changes done + tested + forward ported to 4.0.

[akien-mga]

You're stuck in the past Pedro :P

[akien-mga]

Bug :)

[RandomShaper]

Fixed.

[akien-mga]

 /home/runner/work/godot/godot/platform/android/java/lib/src/org/godotengine/godot/GodotView.java:170: error: incompatible types: EGLConfigChooser cannot be converted to RegularConfigChooser
					eglConfigChooser = new RegularFallbackConfigChooser(8, 8, 8, 8, 24, stencil, eglConfigChooser);
					                                                                             ^
/home/runner/work/godot/godot/platform/android/java/lib/src/org/godotengine/godot/GodotView.java:172: error: incompatible types: EGLConfigChooser cannot be converted to RegularConfigChooser
						eglConfigChooser = new RegularFallbackConfigChooser(8, 8, 8, 8, 32, stencil, eglConfigChooser);
						                                                                             ^

[RandomShaper]

 /home/runner/work/godot/godot/platform/android/java/lib/src/org/godotengine/godot/GodotView.java:170: error: incompatible types: EGLConfigChooser cannot be converted to RegularConfigChooser
					eglConfigChooser = new RegularFallbackConfigChooser(8, 8, 8, 8, 24, stencil, eglConfigChooser);
					                                                                             ^
/home/runner/work/godot/godot/platform/android/java/lib/src/org/godotengine/godot/GodotView.java:172: error: incompatible types: EGLConfigChooser cannot be converted to RegularConfigChooser
						eglConfigChooser = new RegularFallbackConfigChooser(8, 8, 8, 8, 32, stencil, eglConfigChooser);
						                                                                             ^

That's what happens when one rebases without building locally... I'll fix it later.

[clayjohn]

I'm not sure that the backbuffer depth allocated by the OS is ever used.

The render target allocated by Godot that is used for 3D rendering always uses a 24 bit depth in GLES3:

godot/drivers/gles3/rasterizer_storage_gles3.cpp

Lines 6949 to 6952 in 5defa1c

	glGenTextures(1, &rt->depth);
	glBindTexture(GL_TEXTURE_2D, rt->depth);
	glTexImage2D(GL_TEXTURE_2D, 0, GL_DEPTH_COMPONENT24, rt->width, rt->height, 0,
	GL_DEPTH_COMPONENT, GL_UNSIGNED_INT, nullptr);

In GLES2 for android we detect if 24 bit depth is available and use that when we can:

godot/drivers/gles2/rasterizer_storage_gles2.cpp

Lines 6188 to 6195 in 5defa1c

	// on mobile check for 24 bit depth support for RenderBufferStorage
	if (config.extensions.has("GL_OES_depth24")) {
	config.depth_buffer_internalformat = _DEPTH_COMPONENT24_OES;
	config.depth_type = GL_UNSIGNED_INT;
	} else {
	config.depth_buffer_internalformat = GL_DEPTH_COMPONENT16;
	config.depth_type = GL_UNSIGNED_SHORT;
	}

[RandomShaper]

@clayjohn, being that case, I think the best will be to remove the setting I am adding, as well as the one for 32-bit frame buffer. What do you think? The counterpart of this PR for master is already merged, so I guess we have the same situation there.

[clayjohn]

@clayjohn, being that case, I think the best will be to remove the setting I am adding, as well as the one for 32-bit frame buffer. What do you think? The counterpart of this PR for master is already merged, so I guess we have the same situation there.

I am not very familiar with the platform initialization code. But if I am right that the depth buffer isn't even being used, then yeah, we should just eliminate the setting entirely.

[RandomShaper]

I have just realized that there are render-to-screen (i.e., render to the system FBO) render targets (in 3.x, GL ES 2 and 4.0, GL ES 3) that, as far as I understand, will use the depth buffer affected by this setting.

I remember there is (or was?) a setting to have the game rendered directly to the system FBO to avoid a framebuffer copy, which would leverage such feature of render targets. However, I can't find it anymore.

So, if there's no an obvious user-controllable way to opt-in to system FBO, the best is indeed to remove this setting. Otherwise, it still makes sense, but maybe it shouldn't be Android-specific... But that seems like too much hassle for such a marginal feature, so maybe we can just drop the setting now and maybe revive it in the future if we find it's wanted by some?

[akien-mga]

Yeah I think we can drop it and see if anyone was actually relying on this. I think this has been there since Godot was open sourced, and just barely ported to compile whenever we changed platform or rendering code, but I doubt anyone has actually tested it in years. Even the code comments surrounding this are way too well formatted to have been written by reduz :P so it's probably straight out of an early 2010s Android demo code.
Maybe @m4gr3d has more insights for potential valid use cases though.

[RandomShaper]

Updated: upon the new assumption that the system depth buffer doesn't matter at all, the code just does its best to get a valid EGL configuration and that's all there is to it.

I'll make a PR to drop the setting in 4.0, with the only difference that there there won't be a 565 fallback. I don't think any 4.0 game will be targeting super old devices not supporting 32-bit color. Probably neither in 3.x , but let's leave it just in case.

[akien-mga]

Thanks!