Glow Post Process is non uniform through screen axis.

[vitorbalbio]

Godot version:
3.2.4 but also in 3.2.3

OS/device including version:
Windows 10, GLSL3

Issue description:
Glow Post Process blur looks spread more in vertical than horizontal axis causing oval shapes.
Off

On

Steps to reproduce:
Check the effect in a glow sphere

Minimal reproduction project:
TesteGodot2.zip

[clayjohn]

Fixed in master by #41668 and #42201

[Calinou]

See also #32030 which was fixed by introducing a slower high-quality glow mode in master. It should be backportable to the 3.2 branch: #32030 (comment)

[briansemrau]

Edit: read this if you want to understand the bug, but I looked closer at master and yeah those changes should just be ported.

Looking at the 3.x implementation of glow, I noticed that the horizontal pass and vertical pass are calculated differently. This goes against my understanding of how two-pass gaussian filters should be implemented (see blog from intel).

This is the shader code (3.3rc, GLES3):
(Note: the GLES2 code is functionally the same)

// godot/drivers/gles3/shaders/effect_blur.glsl
// ...
#ifdef GLOW_GAUSSIAN_HORIZONTAL
	vec2 pix_size = pixel_size;
	pix_size *= 0.5; //reading from larger buffer, so use more samples
	vec4 color = textureLod(source_color, uv_interp + vec2(0.0, 0.0) * pix_size, lod) * 0.174938;
	color += textureLod(source_color, uv_interp + vec2(1.0, 0.0) * pix_size, lod) * 0.165569;
	color += textureLod(source_color, uv_interp + vec2(2.0, 0.0) * pix_size, lod) * 0.140367;
	color += textureLod(source_color, uv_interp + vec2(3.0, 0.0) * pix_size, lod) * 0.106595;
	color += textureLod(source_color, uv_interp + vec2(-1.0, 0.0) * pix_size, lod) * 0.165569;
	color += textureLod(source_color, uv_interp + vec2(-2.0, 0.0) * pix_size, lod) * 0.140367;
	color += textureLod(source_color, uv_interp + vec2(-3.0, 0.0) * pix_size, lod) * 0.106595;
	color *= glow_strength;
	frag_color = color;
#endif

#ifdef GLOW_GAUSSIAN_VERTICAL
	vec4 color = textureLod(source_color, uv_interp + vec2(0.0, 0.0) * pixel_size, lod) * 0.288713;
	color += textureLod(source_color, uv_interp + vec2(0.0, 1.0) * pixel_size, lod) * 0.233062;
	color += textureLod(source_color, uv_interp + vec2(0.0, 2.0) * pixel_size, lod) * 0.122581;
	color += textureLod(source_color, uv_interp + vec2(0.0, -1.0) * pixel_size, lod) * 0.233062;
	color += textureLod(source_color, uv_interp + vec2(0.0, -2.0) * pixel_size, lod) * 0.122581;
	color *= glow_strength;
	frag_color = color;
#endif
// ...

I did some experimentation in one of my current projects by replacing the vertical pass code with the horizontal pass (swapping offset x/y).
Here is a before and after:

[clayjohn]

@briansemrau feel free to make a PR adding a "high quality mode" in 3.x.

For your benefit, the cause of this is because the horizontal pass is reading from a texture that is 2x larger than itself, while the vertical pass reads from a framebuffer that is the same size as itself. The horizontal pass thus has a range of only 1.5 pixels (in framebuffer space) while the vertical pass has a range of 2.

Your solution compresses the vertical pass to a range of 1.5, but it is still using 7 texture lookups! A better solution is to increase the range of the horizontal pass by adding an optional extra texture lookup that has a range of 4 pixels, thus increasing the range in framebuffer space to 2 pixels, just like the vertical pass.

[Calinou]

This was implemented by #51491 (which will be available in the next 3.4 beta/RC), closing.