Page MenuHomeWildfire Games

Adds MSAA to anti-aliasing techniques
Needs ReviewPublic

Authored by vladislavbelov on Jun 12 2020, 6:58 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Expensive, but high-quality anti-aliasing.

Test Plan
  1. Apply the patch and compile the game
  2. Run any map with different AA settings, try to change it during the game
  3. Run atlas with different AA settings

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
SeverityLocationCodeMessage
Errorsource/renderer/Renderer.cpp:172CPPCheckBear (returnDanglingLifetime)CPPCheckBear (returnDanglingLifetime)
Errorsource/renderer/Renderer.cpp:178CPPCheckBear (returnDanglingLifetime)CPPCheckBear (returnDanglingLifetime)
Errorsource/renderer/Renderer.cpp:184CPPCheckBear (returnDanglingLifetime)CPPCheckBear (returnDanglingLifetime)
Errorsource/renderer/Renderer.cpp:190CPPCheckBear (returnDanglingLifetime)CPPCheckBear (returnDanglingLifetime)
Errorsource/renderer/Renderer.cpp:196CPPCheckBear (returnDanglingLifetime)CPPCheckBear (returnDanglingLifetime)
Errorsource/renderer/Renderer.cpp:202CPPCheckBear (returnDanglingLifetime)CPPCheckBear (returnDanglingLifetime)
Errorsource/renderer/Renderer.cpp:208CPPCheckBear (returnDanglingLifetime)CPPCheckBear (returnDanglingLifetime)
Errorsource/renderer/Renderer.cpp:214CPPCheckBear (returnDanglingLifetime)CPPCheckBear (returnDanglingLifetime)
Errorsource/renderer/Renderer.cpp:220CPPCheckBear (returnDanglingLifetime)CPPCheckBear (returnDanglingLifetime)
Errorsource/renderer/Renderer.cpp:226CPPCheckBear (returnDanglingLifetime)CPPCheckBear (returnDanglingLifetime)
Errorsource/renderer/Renderer.cpp:232CPPCheckBear (returnDanglingLifetime)CPPCheckBear (returnDanglingLifetime)
Unit
Unit Tests Skipped
Build Status
Buildable 12963
Build 25522: Vulcan BuildJenkins
Build 25521: Vulcan Build (macOS)Jenkins
Build 25520: Vulcan Build (Windows)Jenkins

Event Timeline

vladislavbelov created this revision.Jun 12 2020, 6:58 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/PostprocManager.h
|  26| class·CPostprocManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCPostprocManager{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2412/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/PostprocManager.h
|  26| class·CPostprocManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCPostprocManager{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2414/display/redirect

Imarok added a subscriber: Imarok.Jun 12 2020, 11:01 PM

Looks really sharp. Does MSAA 32× make sense?
Can change ingame without issues.
Seems like not working when changing in atlas. (FXAA is working)

Looks really sharp.

Thx.

Does MSAA 32× make sense?

I don't think so, I see no difference with 32 (also I don't remember such value in most games).

Seems like not working when changing in atlas.

It's disabled in Atlas, because it uses wxWidgets's canvas with own GL context.

Stan added a subscriber: Stan.EditedJun 12 2020, 11:32 PM

Works fine with me with both Intel UHD 630 and Nvidia 1070 on Windows For Atlas I still have the driver override (Tested with an without, and no crash) 🗡

I also get an error on Sayahdri Butes(5) 0x502 GL error occured, I probably ran out of VRAM.

Might try on my Microsoft Surface 1 Pro (Kubuntu) tablet if wanted.

Does MSAA 32× make sense?

I don't think so, I see no difference with 32 (also I don't remember such value in most games).

Me neither xD

Seems like not working when changing in atlas.

It's disabled in Atlas, because it uses wxWidgets's canvas with own GL context.

aha. I guess that makes sense. (I personally have no idea, about that topic ;P)

Angen added a subscriber: Angen.Jun 13 2020, 11:19 AM
Angen added inline comments.
source/renderer/PostprocManager.cpp
511

why pingfbo and not mutlisamplefbo?

Stan added a comment.Jun 13 2020, 11:24 AM

Ran with linkmauve's D2488

4024] OpenGL other of severity notification from the API id 131185: Buffer detailed info: Buffer object 9 (bound to GL_VERTEX_ARRAY_BUFFER_BINDING_ARB, usage hint is GL_STATIC_DRAW) will use VIDEO memory as the source for buffer object operations.
[4024] OpenGL other of severity notification from the API id 131185: Buffer detailed info: Buffer object 9 (bound to GL_VERTEX_ARRAY_BUFFER_BINDING_ARB, usage hint is GL_STATIC_DRAW) will use VIDEO memory as the source for buffer object operations.
[4024] OpenGL other of severity notification from the API id 131185: Buffer detailed info: Buffer object 9 (bound to GL_VERTEX_ARRAY_BUFFER_BINDING_ARB, usage hint is GL_STATIC_DRAW) will use VIDEO memory as the source for buffer object operations.
[4024] OpenGL other of severity notification from the API id 131185: Buffer detailed info: Buffer object 9 (bound to GL_VERTEX_ARRAY_BUFFER_BINDING_ARB, usage hint is GL_STATIC_DRAW) will use VIDEO memory as the source for buffer object operations.
[4024] OpenGL other of severity notification from the API id 131185: Buffer detailed info: Buffer object 9 (bound to GL_VERTEX_ARRAY_BUFFER_BINDING_ARB, usage hint is GL_STATIC_DRAW) will use VIDEO memory as the source for buffer object operations.
[4024] OpenGL other of severity notification from the API id 131185: Buffer detailed info: Buffer object 9 (bound to GL_VERTEX_ARRAY_BUFFER_BINDING_ARB, usage hint is GL_STATIC_DRAW) will use VIDEO memory as the source for buffer object operations.
[4024] OpenGL other of severity notification from the API id 131185: Buffer detailed info: Buffer object 6 (bound to GL_ARRAY_BUFFER_ARB, usage hint is GL_DYNAMIC_DRAW) has been mapped WRITE_ONLY in SYSTEM HEAP memory (fast).
[4024] OpenGL other of severity notification from the API id 131185: Buffer detailed info: Buffer object 5 (bound to GL_ARRAY_BUFFER_ARB, usage hint is GL_DYNAMIC_DRAW) has been mapped WRITE_ONLY in SYSTEM HEAP memory (fast).
[4024] OpenGL error of severity high from the API id 1282: GL_INVALID_OPERATION error generated. Copy is not valid when anti-aliasing is enabled for the current read buffer.
[4024] ..\..\..\source\renderer\Renderer.cpp:1382: OpenGL error(s) occurred: GL_INVALID_OPERATION (0502)

Ran with API Trace

Stanislas Dolcini, [13.06.20 11:09]
002193: message: major api error 1282: GL_INVALID_OPERATION error generated. Copy is not valid when anti-aliasing is enabled for the current read buffer.
4002193 @0 glCopyTexImage2D(target = GL_TEXTURE_2D, level = 0, internalformat = GL_DEPTH_COMPONENT, x = 0, y = 0, width = 1024, height = 768, border = 0)
4002193: warning: glGetError(glCopyTexImage2D) = GL_INVALID_OPERATION
4016092: message: minor api issue 131076: Usage warning: glClear() called with GL_STENCIL_BUFFER_BIT, but there is no stencil buffer. Operation will have no effect.

Hope it helps.

source/renderer/PostprocManager.cpp
511

Because it's the target: GL_DRAW_FRAMEBUFFER_EXT, not the source.

@vladislavbelov My friend, have you finished fixing this patch?

Fixes too late resolving.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3032/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2479/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1380/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2492/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

builderr-debug-macos.txt
../../../source/renderer/PostprocManager.cpp:42:1: error: version control conflict marker in file
<<<<<<< .mine
^
../../../source/renderer/PostprocManager.cpp:691:16: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleColorTex);
                      ^
../../../source/renderer/PostprocManager.cpp:692:27: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        pglTexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleCount, GL_RGBA, m_Width, m_Height, GL_TRUE);
                                 ^
../../../source/renderer/PostprocManager.cpp:696:16: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleDepthTex);
                      ^
../../../source/renderer/PostprocManager.cpp:697:27: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        pglTexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleCount, GL_DEPTH24_STENCIL8_EXT, m_Width, m_Height, GL_TRUE);
                                 ^
../../../source/renderer/PostprocManager.cpp:699:16: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, 0);
                      ^
../../../source/renderer/PostprocManager.cpp:708:3: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
                GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleColorTex, 0);
                ^
../../../source/renderer/PostprocManager.cpp:711:3: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
                GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleDepthTex, 0);
                ^
../../../source/renderer/PostprocManager.cpp:723:16: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, 0);
                      ^
9 errors generated.
make[1]: *** [obj/graphics_Debug/PostprocManager.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [graphics] Error 2

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1393/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

builderr-debug-gcc6.txt
../../../source/renderer/PostprocManager.cpp: In constructor 'CPostprocManager::CPostprocManager()':
../../../source/renderer/PostprocManager.cpp:42:1: error: version control conflict marker in file
 <<<<<<< .mine
 ^~~~~~~
../../../source/renderer/PostprocManager.cpp:42:1: error: version control conflict marker in file
../../../source/renderer/PostprocManager.cpp: At global scope:
../../../source/renderer/PostprocManager.cpp:42:1: error: version control conflict marker in file
make[1]: *** [obj/graphics_Debug/PostprocManager.o] Error 1
make: *** [graphics] Error 2

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3045/display/redirect

Proper rebase.

Build failure - The Moirai have given mortals hearts that can endure.

builderr-debug-macos.txt
../../../source/renderer/PostprocManager.cpp:686:16: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleColorTex);
                      ^
../../../source/renderer/PostprocManager.cpp:687:27: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        pglTexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleCount, GL_RGBA, m_Width, m_Height, GL_TRUE);
                                 ^
../../../source/renderer/PostprocManager.cpp:691:16: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleDepthTex);
                      ^
../../../source/renderer/PostprocManager.cpp:692:27: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        pglTexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleCount, GL_DEPTH24_STENCIL8_EXT, m_Width, m_Height, GL_TRUE);
                                 ^
../../../source/renderer/PostprocManager.cpp:694:16: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, 0);
                      ^
../../../source/renderer/PostprocManager.cpp:703:3: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
                GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleColorTex, 0);
                ^
../../../source/renderer/PostprocManager.cpp:706:3: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
                GL_TEXTURE_2D_MULTISAMPLE, m_MultisampleDepthTex, 0);
                ^
../../../source/renderer/PostprocManager.cpp:718:16: error: use of undeclared identifier 'GL_TEXTURE_2D_MULTISAMPLE'
        glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, 0);
                      ^
8 errors generated.
make[1]: *** [obj/graphics_Debug/PostprocManager.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [graphics] Error 2

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1394/display/redirect

Stan added a comment.Aug 21 2020, 10:40 PM

I just tested the patch, it works fine on my Intel UHD and on my GTX 1070. I noticed it totally ignores trees, but I guess that's intended.

@Stan Then why did he ignore the trees? It's strange? Is this a flight from something?

Just now, I tested this patch and it doesn't seem to fix the problem. Why aren't trees getting anti-aliasing optimisation? What exactly does it change? Please tell me, thank you!

@Stan Then why did he ignore the trees? It's strange? Is this a flight from something?

Using multisampling for transparent objects might be too expensive at the moment. Also transparent objects might have smooth edges that helps to reduce aliasing.

Stan added a comment.Aug 22 2020, 10:45 AM

The trees are ignored for performance.

Freagarach added a subscriber: Freagarach.EditedAug 22 2020, 11:20 AM

I tried this on a Nvidia geforce 9700m GT and obviously ×16 lags uncontrollably (~2 FPS). However I also noticed something else (look at the image above door on the CC):

binaries/data/mods/public/gui/options/options.json
129

You could use × instead of the x?

Stan added a comment.Aug 22 2020, 11:21 AM

Looks like Z Fighting

@Stan Has the problem of water disappearing been solved?

Stan added a comment.Aug 22 2020, 12:12 PM

Yes, just try the patcg my friend :)

@Freagarach The Vla seems to have noticed this, right?

gameboy added a comment.EditedAug 22 2020, 1:07 PM

@Stan My friend Stan, what do you think of his (Freagarach) question?

Patch works for me (Windows 10, RX 5700).
@vladislavbelov your former patch had AA for trees too, without a big performance hit. Does the new performance impact came from the water bugfix?
Also I don't think, we should care about performance hits by trees, as we have FXAA for lower hardware. I see no need for the implementation in the current state, as FXAA + sharpening looks better than MSAA, as long as trees have aliasing. Especially as FXAA and sharpening has no measurable performance impact compared to MSAA.

@vladislavbelov MSAA is a mainstream anti-aliasing tool that should get better play. Many graphics CARDS support it now. I recommend implementing it in its entirety, which will improve the overall picture quality of the game as well as the performance.

@Stan @vladislavbelov I'm very happy that MSAA can be implemented, which has further improved the image quality of the game. As for this patch, when will it be officially released?

Just found this nice post about MSAA that might come in handy here @vladislavbelov https://mynameismjp.wordpress.com/2012/10/24/msaa-overview/