Implement support for FSAA in combination with post-processing

[grorp]

fixes #14285, minetest/irrlicht#286

• Actually it's MSAA I think, or perhaps the terms are equivalent
• I've made it fit into the existing Irrlicht architecture, but that has resulted in code duplication compared to my original "hacky" approach
• OpenGL 3.2+ and OpenGL ES 3.1+ are supported
• EDT_OPENGL3 is not required, EDT_OPENGL works too
• Helpful tutorial: https://learnopengl.com/Advanced-OpenGL/Anti-Aliasing, section "Off-screen MSAA"
• This may be rough around the edges, but in general it works

Screenshot showing it in action:

"master" branch: post-processing, volumetric lighting, FSAA enabled but doesn't work

this PR: post-processing, volumetric lighting, FSAA works

P.S. I have used "ChatGPT" as a source of information/answers while working on this

To do

This PR is a Ready for Review.

• GL error when FSAA is enabled without restart with "opengl" video driver? fixed by 0d3a93d in this PR
• GL warning "framebuffer object is unsupported because the depth/stencil attachment is mismatched" with post-processing with "opengl3" video driver (FSAA state and/or FSAA state on startup may matter) already happens before this PR, "fixed" in PR Get rid of depth buffer workaround in the render pipeline code #15407 (the code triggering the warning was only used due to the workaround, no its no longer used)
• black screen with post-processing with "ogles2" video driver on desktop already happens before this PR, fixed in PR [no sq] Fix ECF_D32 support in ogles2 video driver #15396

How to test

Enable post-processing and FSAA, see that FSAA works.

[lhofhansl]

oh nice. Going to try today.

[lhofhansl]

Mostly works, but some weird flashes as you move around.

(note the bright spot on the right)

Here's a video:
https://github.com/user-attachments/assets/ca0c60c0-5668-48ac-bbbe-a4f4009c416c

Happen with bloom enabled and easier to see a bit further away distances (300+ or so)
It looks like somehow the bloom detects very bright spots and amplifies them.

Update: happens with NVidia and Intel on Linux with GLX or SDL. A nice feature of this is that MSAA finally works with SDL.
Update II: Happens less the "better" the MSAA. With 16 I do not see any flashes, with 8 I see some, and 4 and 2 I see more. So there's some error that the bloom picks up on.

[lhofhansl]

I'll note that now it makes sense to enable MSAA (for geometry aliasing) and FXAA (for texture aliasing)

[lhofhansl]

The code looks "right". I had started down that rabbit hole a few months ago, and about 70% as far as you got (all the enum/API changes in Irrlicht).

[grorp]

Mostly works, but some weird flashes as you move around.
Happens less the "better" the MSAA. With 16 I do not see any flashes, with 8 I see some, and 4 and 2 I see more. So there's some error that the bloom picks up on.

Can reproduce. With some testing code to compare the bloom texture and the color texture (2868e92):

1st screenshot color texture, 2nd screenshot bloom texture, 3rd screenshot final rendered result

If you zoom in, you can see that there are a few #ffffff and #00000 pixels sprinkled across the color texture. The bloom then goes crazy on the #ffffff pixels. I have no idea 1) where the #ffffff pixels are coming from or 2) why the bloom reacts like that

[lhofhansl]

If you zoom in, you can see that there are a few #ffffff and #00000 pixels sprinkled across the color texture.

I see nothing wrong with the code, especially the blitting. I tried gl.LINEAR (with only the color texture), but the effect remains the same. It's almost like there's something wrong with the sampling. I wonder if these black and white pixels are there without PP as well, or the way MSAA is enabled with GLX, we just do not notice them without bloom.

The fact that these artifacts vanish with MSAA = 16 seems to point in that direction too.

[lhofhansl]

Also... When you use SDL (which does not support start-up MSAA in Irrlicht), MSAA now depends on PostProcessing.

[lhofhansl]

One more. I tried the on-start MSAA with GLX... It produces the same weird pixels.
So I really think these effects were there all along, we just do not notice them without bloom.

See also: https://stackoverflow.com/questions/49063871/deferred-msaa-artifacting

[grorp]

Also... When you use SDL (which does not support start-up MSAA in Irrlicht), MSAA now depends on PostProcessing.

SDL with normal MSAA works fine for me, also on the "master" branch (obviously only as long as post-processing is disabled, 3d_mode is "none" and undersampling is 0)

Fedora Workstation 40, "NVIDIA GeForce GTX 1080 Ti"

[lhofhansl]

SDL with normal MSAA works fine for me, also on the "master" branch (obviously only as long as post-processing is disabled, 3d_mode is "none" and undersampling is 0)

Weird. Also Fedora 40, Luanti build from master today.
Definitely does not work for me with GeForce GTX 1650 Mobile/ Max-Q, NVidia proprietary driver 560.35.03.
But it does work in Intel with UHD Graphics 630
(Both work when I use GLX instead)

With this PR MSAA work with NVidia, only when PP is enabled. What's nice is that I can switch MSAA at runtime.

So two suggestions:

• Make MSAA part of PP in the settings.
• Allow enabled MSAA and FXAA.

[grorp]

Make MSAA part of PP in the settings.

No, MSAA is not inherently a part of the post-processing pipeline nor does it belong into the "Effects" section. It not working without post-processing on your setup doesn't justify moving it into the post-processing section.

Allow enabled MSAA and FXAA.

Sounds reasonable in general, but not something I want to do in this PR (to keep the scope reasonable)

[grorp]

• Fixed the remaining bugs with the legacy "opengl" driver (GL errors; FSAA only working partially if not enabled at startup)
• Fixed the remaining GL errors/warnings: [no sq] Fix ECF_D32 support in ogles2 video driver #15396, Get rid of depth buffer workaround in the render pipeline code #15407
• Properly separated FSAA via OpenGL context and via post-processing, only of them is used (this has the nice side effect that you can set really high MSAA values without getting blurry fonts or GUIs when post-processing is enabled)

This PR should now be ready for review.

[lhofhansl]

The bloom flashing is still there. Should we document that bloom is incompatible with MSAA (in settings other than 16)?

[grorp]

The bloom flashing is still there. Should we document that bloom is incompatible with MSAA (in settings other than 16)?

Sure, can't hurt to document it. 4d4e455

[lhofhansl]

Looks good.

As I said I got about 70% there (without ChatGPT :) ), and had made most of the same interface changes.
The main step I was missing how to do the actual resolve myself.

And works fine on my box (with Intel or NVidia drivers)

[lhofhansl]

@grorp did you see my comment about avoiding the flashing?
It just clamps the light to valid values.
I have tried various settings and have not seen any differences. (Other that the flashing going away).

[grorp]

did you see my comment about avoiding the flashing? It just clamps the light to valid values. I have tried various settings and have not seen any differences. (Other that the flashing going away).

Hmm yes, but I still don't think a change to the bloom stuff should be included in this PR.

[sfan5]

👍 works well, finally Luanti can look smooth again

one thing I noticed are these white spots, that are transient and appear sometimes when looking at a flat surface head-on:

edit: actually I don't remember which level of fsaa I used before, this is with 4. but might not be a new issue.

[lhofhansl]

I've noticed those white spots before (without this PR) with flat surfaces, I do not think that's new.

[sfan5]

works

[grorp]

Oh yay, on my phone it's broken when I remove this code and post-processing and MSAA are enabled:

https://github.com/minetest/minetest/blob/8d2e7703619205dd76ee9cc12fb0b660868c910b/src/client/render/plain.cpp#L158-L159

with

ERROR[Main]: Irrlicht: GL: 8246 824c internalformat 32993 is not a sized format
ERROR[Main]: Irrlicht: GL_INVALID_ENUM COpenGLCoreTexture.h:240
ERROR[Main]: Irrlicht: COpenGLCoreTexture: InternalFormat:0x80e1 PixelFormat:0x80e1

It works again if I remove this code too:

https://github.com/minetest/minetest/blob/8d2e7703619205dd76ee9cc12fb0b660868c910b/irr/src/OpenGLES2/Driver.cpp#L89-L92

and results in a significant performance improvement over the current version of this PR.

Anyway, not something I want to investigate now.

[sfan5]

https://docs.gl/es3/glTexStorage2DMultisample

internalformat
Specifies the sized internal format to be used to store texture image data.

https://registry.khronos.org/OpenGL/extensions/EXT/EXT_texture_format_BGRA8888.txt:
BGRA_EXT (= the GL_BGRA constant we're using) is not sized
BGRA8_EXT would be

[grorp]

https://registry.khronos.org/OpenGL/extensions/EXT/EXT_texture_format_BGRA8888.txt: BGRA_EXT (= the GL_BGRA constant we're using) is not sized BGRA8_EXT would be

diff --git a/irr/src/OpenGLES2/Driver.cpp b/irr/src/OpenGLES2/Driver.cpp
index 3c034522c..f33bc61d1 100644
--- a/irr/src/OpenGLES2/Driver.cpp
+++ b/irr/src/OpenGLES2/Driver.cpp
@@ -64,7 +64,7 @@ void COpenGLES2Driver::initFeatures()
                TextureFormats[ECF_D24S8] = {GL_DEPTH24_STENCIL8, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8};
 
                if (FeatureAvailable[IRR_GL_EXT_texture_format_BGRA8888])
-                       TextureFormats[ECF_A8R8G8B8] = {GL_BGRA, GL_BGRA, GL_UNSIGNED_BYTE};
+                       TextureFormats[ECF_A8R8G8B8] = {BGRA8_EXT, GL_BGRA, GL_UNSIGNED_BYTE};
                else if (FeatureAvailable[IRR_GL_APPLE_texture_format_BGRA8888])
                        TextureFormats[ECF_A8R8G8B8] = {BGRA8_EXT, GL_BGRA, GL_UNSIGNED_BYTE};
 

or

diff --git a/irr/src/OpenGLES2/Driver.cpp b/irr/src/OpenGLES2/Driver.cpp
index 3c034522c..9f212b57d 100644
--- a/irr/src/OpenGLES2/Driver.cpp
+++ b/irr/src/OpenGLES2/Driver.cpp
@@ -64,7 +64,7 @@ void COpenGLES2Driver::initFeatures()
                TextureFormats[ECF_D24S8] = {GL_DEPTH24_STENCIL8, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8};
 
                if (FeatureAvailable[IRR_GL_EXT_texture_format_BGRA8888])
-                       TextureFormats[ECF_A8R8G8B8] = {GL_BGRA, GL_BGRA, GL_UNSIGNED_BYTE};
+                       TextureFormats[ECF_A8R8G8B8] = {GL_BGRA8_EXT, GL_BGRA, GL_UNSIGNED_BYTE};
                else if (FeatureAvailable[IRR_GL_APPLE_texture_format_BGRA8888])
                        TextureFormats[ECF_A8R8G8B8] = {BGRA8_EXT, GL_BGRA, GL_UNSIGNED_BYTE};
 

make all textures turn black, if that's what you mean

[sfan5]

works on my machine™
looking at the revision history this is a recent addition:

1.4,  23/06/2024  Erik Faye-Lund: Add GL_BGRA8_EXT for ES 2.0 and later.

how implementations are supposed to deal with this is a mystery to me

anyway this is not a thing for this PR.