Fix for webgl reflection probes

[WeaselShoppe]

Render buffer storage was being configured with wrong format on WebGL leading to reflection probes failing.

Fix #38571

[Calinou]

Thanks for opening a pull request!

If you have spare time, it might be worth checking whether this should also be fixed in GLES3 (WebGL 2.0).

[WeaselShoppe]

Thanks for opening a pull request!

If you have spare time, it might be worth checking whether this should also be fixed in GLES3 (WebGL 2.0).

No problem. I will check/test reflection probes in GLES3.0/WebGL 2.0 as well. For right now I am trying to figure out what I did to make clang-format angry...

[clayjohn]

Looking into the code a little deeper, it appears that the issue is actually with a few calls to glRenderbufferStorage that are accidentally using config.depth_internalformat instead of config.depth_buffer_internalformat.

textures should use depth_internalformat while storage buffers should use depth_buffer_internalformat

[WeaselShoppe]

Looking into the code a little deeper, it appears that the issue is actually with a few calls to glRenderbufferStorage that are accidentally using config.depth_internalformat instead of config.depth_buffer_internalformat.

textures should use depth_internalformat while storage buffers should use depth_buffer_internalformat

ah. this makes a lot of sense. is depth_buffer_internalformat correctly set to GL_DEPTH_COMPONENT16 when running in WebGL 1.0 context?

[clayjohn]

@WeaselShoppe Yep :)

godot/drivers/gles2/rasterizer_storage_gles2.cpp

Lines 5987 to 5994 in 7595228

	#ifdef JAVASCRIPT_ENABLED
	// RenderBuffer internal format must be 16 bits in WebGL,
	// but depth_texture should default to 32 always
	// if the implementation doesn't support 32, it should just quietly use 16 instead
	// https://www.khronos.org/registry/webgl/extensions/WEBGL_depth_texture/
	config.depth_buffer_internalformat = GL_DEPTH_COMPONENT16;
	config.depth_type = GL_UNSIGNED_INT;
	#else

[WeaselShoppe]

@WeaselShoppe Yep :)

godot/drivers/gles2/rasterizer_storage_gles2.cpp

Lines 5987 to 5994 in 7595228

	#ifdef JAVASCRIPT_ENABLED
	// RenderBuffer internal format must be 16 bits in WebGL,
	// but depth_texture should default to 32 always
	// if the implementation doesn't support 32, it should just quietly use 16 instead
	// https://www.khronos.org/registry/webgl/extensions/WEBGL_depth_texture/
	config.depth_buffer_internalformat = GL_DEPTH_COMPONENT16;
	config.depth_type = GL_UNSIGNED_INT;
	#else

Awesome. Should I just revise this PR by reverting back to 3.2 HEAD, and fixing the calls to glRenderbufferStorage? (after testing on my devices of course).

[clayjohn]

Awesome. Should I just revise this PR by reverting back to 3.2 HEAD, and fixing the calls to glRenderbufferStorage? (after testing on my devices of course)

Yes please :)

[clayjohn]

Looks good. You are going to need to squash your commits now so that it is just a single commit. How to do so is explained here https://docs.godotengine.org/en/stable/community/contributing/pr_workflow.html

[clayjohn]

Looks great now! Thank you and good job!

[WeaselShoppe]

Looks great now! Thank you and good job!

No problem. Thanks for all the help/guidance!

[akien-mga]

Thanks! And congrats for your first merged Godot contribution 🎉