-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow shaders with disabled post processing pipeline #14338
Conversation
@HybridDog Not sure whether you have any insights here. I do not know enough about Irrlicht + Shaders. There should be a way to pass a muti-sample texture to the postprocessing pipeline, and then resolve it, so that at least the initial scene can benefit from fast FSAA (any post processed changes would not). But I do not understand how to do that with Irrlicht. See: https://therealmjp.github.io/posts/msaa-overview/ and https://stackoverflow.com/questions/42878216/opengl-how-to-draw-to-a-multisample-framebuffer-and-then-use-the-result-as-a-n For me MSAA works with NVidia and Intel drivers, and is quite a performance improvement over SSAA. (see link as to why... Essentially the fragment shader is not called for all fragments, only the interesting ones) |
Maybe it was premature to move everything to the post processing stage. And actually it is not that unreasonable to disable post processing. I guess one could derive it by checking that all of tonemapping, saturation, dithering, bloom filters, and volumetric lighting are, but making it explicit is nice. Some more info on Irrlicht and MS textures here: https://irrlicht.sourceforge.io/forum/viewtopic.php?t=51397 |
@devshgraphicsprogramming are you still around here? You mention in #6976 that MSAA is arcane. I agree, yet, for the kind of geometry aliasing I am concerned with here, it is very effective (unlike FXAA and other post-processing AA techniques) and much faster than SSAA. |
And actually it's pretty trivial as it really just allows disabling post-processing, with no other changes. |
I also don't know much about Irrlicht and FSAA, but I can nonetheless try to give insights. If multisampling should work, a so-called multisample resolve must happen. I assume that with EGL and GLX, GL_TEXTURE_2D_MULTISAMPLE textures are automatically created because Irrlicht enables anti aliasing in the EGL or GLX attribute array configuration. I think the multisample resolve needs to be implemented in Irrlicht (probably in COpenGLCoreRenderTarget.h), i.e. if FSAA is enabled in Minetest, the first stage should be rendered to a GL_TEXTURE_2D_MULTISAMPLE texture and this texture should then be converted (with a blit operation) to a GL_TEXTURE_2D texture for the next stages. Furthermore, as a cleanup, the EGL and GLX attribute array configuration should not contain enabled anti aliasing if post processing is enabled in Minetest. Additional notes: If I understand it correctly, in the OpenGL and OpenGL ES code, As far as I know, EGL and GLX provide the interface between OpenGL and the window. If MSAA is enabled, Irrlicht sets something in the "attribute array for the draw buffer", such as Antialiasing in SDL-related code: OpenGL differentiates between the GL_TEXTURE_2D and GL_TEXTURE_2D_MULTISAMPLE texture types. |
Thanks @HybridDog I also have searched through the Irrlicht code to see how/where it does handles XXX_MULTISAMPLE. Do you have any leisure to dig a bit deeper into this. I'll also continue to dig.
Aren't these two at odds? Or do you mean if PP is enabled we should only enable it through the materials? |
Storing some notes here for later: https://github.com/minetest/irrlicht/blob/master/include/ITexture.h#L159 And ETT_2D is hardcoded here: https://github.com/minetest/irrlicht/blob/master/source/Irrlicht/COpenGLDriver.cpp#L1920 (I'm about to run out of time on this. It seems Irrlicht currently just does not support multisampling textures.) |
In the extreme we can have an alternate shader that does tonemapping, saturation, and dithering - none of these actually need to be the PP pipeline, and only use the PP pipeline for bloom and volumetric lighting. Or... Someone figures out how to add MS texture support to Irrlicht proper. Or... Do proper geometry LOD at a distance (300+), in that case pixel based (post processing) anti aliasing would work. |
Tonemapping, saturation, and dithering are all optional effects, I think it's best to avoid duplicating implementations and keep them in post-processing. Tonemapping (filmic and saturation) does seem to be best implemented as a post-processing effect given that it maps the HDR output to colors to be rendered on the screen - we don't want to duplicate this for all types of rendered thing Surely it's possible to have correct antialiasing with post processing enabled? |
Yep. Just needs some changes in Irrlicht - which I find somewhat obscure. Allowing to optionally disable PP is not unreasonable in general, though. |
Yes, if post processing is enabled, MSAA should be disabled in the EGL and GLX attribute array configuration. I think with post processing, in each stage except the first one we render a single square (two triangles) and there are no edges which need to be anti aliased. We need multisampling only in the first stage, where the rendering from the 3D world to a 2D image happens.
I also think so since I don't find anything useful when I ripgrep for |
Postprocessing can make a small performance difference, e.g. see these "drawtime" values on my laptop: without shaders: 8-9 ms So I wonder why we'd even make this a manual setting. Can't we just automatically disable postprocessing if no postprocessing effects are enabled? Is there any reason for keeping it enabled in this case? |
I thought about this too... The one part that is not manually enabled is saturation. So I thought instead of making that configurable it is better to just be able to disable PP wholesale. |
Right. |
With this one. I want everybody to try and see the staggering difference. It may sounds like a trifle, but with slightly larger viewing_range (doesn't have to be 1000), as you move around the noise that comes from geometry aliasing is really grating and looks quite unprofessional. You can compare FSAA with 4 samples, to FXAA. (SSAA is potentially better than FSAA but is far, far slower.) |
c30da34
to
e0a0af5
Compare
BTW. FSAA does not work on Windows, even with the PP pipeline off. Seems that Irrlicht has not implemented that or we're missing something in the setup. Update: And even on Linux, when Irrlicht is configured to use SDL2, FSAA does not work even with PP off. |
Independently of all the FSAA discussion here. Is it reasonable to allow disabling the PP pipeline so that clients can control performance? We could automatically disable the PP pipeline if all of:
are disabled, but I think an overall switch is better, and easier to understand. |
@lhofhansl I can check that this works but not offer advice on graphical stuff. Is that what you wanted me to do? |
@sfan5 Yeah, that (a) it works, and (b) it's reasonable to allow disabling the PP pipeline for performance reasons. |
Can confirm FSAA doing something when thought: shouldn't we disable this on Android by default? |
+1 |
Should I default this to fault on Android in this PR (maybe different commit), or separate discussion on separate PR? |
This PR imo. |
e0a0af5
to
6cfdcfb
Compare
@sfan5 OK, done. |
6cfdcfb
to
8ba986a
Compare
I assume this is still good to merge. |
- Allow disabling of the post processing pipeline while leaving shaders enabled - Also disable post processing on Android by default
- Allow disabling of the post processing pipeline while leaving shaders enabled - Also disable post processing on Android by default
- Allow disabling of the post processing pipeline while leaving shaders enabled - Also disable post processing on Android by default
- Allow disabling of the post processing pipeline while leaving shaders enabled - Also disable post processing on Android by default
- Allow disabling of the post processing pipeline while leaving shaders enabled - Also disable post processing on Android by default
- Allow disabling of the post processing pipeline while leaving shaders enabled - Also disable post processing on Android by default
This allows disabling the post-processing pipeline while leaving Shader enabled.
So "geometric" shader feature - shadows, waves, waving plants, etc - remain enabled, and all post-processing that relies to intermediary textures are disabled.
See also #14285. Currently post processing silently disables FSAA.
FSAA is the only (performance) option we have to avoid geometry aliasing (not texture aliasing).
In general it seems reasonable to allow shaders but disable post processing.
To do
This PR is Ready for Review.
How to test
Enable shaders, and disable post processing, make sure all non-post-processing shader features still work: shadows, waves, waving plants, smooth day-night change, etc., and that everything still renders OK.
Enable FSAA, try some different FSAA values.
Notice the absence of Moire effect caused by geometry aliasing. (see images on #14285).
(Remember changing FSAA requires a restart of MT)