Skip to content
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

Turn dithering into a global post-processing effect #3335

Merged
merged 2 commits into from
Jul 24, 2023

Conversation

10110111
Copy link
Contributor

Description

This is the first step towards future HDR rendering. The whole rendering pipeline is split into two stages:

  1. Rendering of the scene in deeper color (RGBA16 for now)
  2. Post-processing

Currently post-processing comprises only dithering and resolution of multisampling samples. This in particular makes multisampling a one-place configuration, so the filters like e.g. spherical mirror will get antialiased image automagically without special setup (before this commit spherical mirror image was aliased). Also, application of dithering only once reduces interference of the dithering noise (e.g. when two images are blended together), and generally makes the result more correct.

Low-graphics modes such as ANGLE, OpenGL<3.3, OpenGL Compatibility Profile, etc. will use the old way of rendering, i.e. without the intermediate deep-color FBO. Thus they will not have dithering nor multisampling.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update

How Has This Been Tested?

Test Configuration:

  • Operating system: Ubuntu 20.04
  • Graphics Card: Intel UHD Graphics 620

Checklist:

  • My code follows the code style of this project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (header file)
  • I have updated the respective chapter in the Stellarium User Guide
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@github-actions
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files.

Copy link
Member

@alex-w alex-w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabling DSO markers cause the change in the rendering of grids and lines in OpenGL
compatible mode.

stellarium-001
stellarium-002

@10110111
Copy link
Contributor Author

Very strange. Do the lines change width?

Also, please post a PNG version of these screenshots, I can't even understand whether the lines are aliased due to JPEG artifacts.

@alex-w
Copy link
Member

alex-w commented Jul 22, 2023

Very strange. Do the lines change width?

Not exactly. Please see the screencast.

Also, please post a PNG version of these screenshots, I can't even understand whether the lines are aliased due to JPEG artifacts.

stellarium-001
stellarium-002

v23.2:
stellarium-003

@10110111
Copy link
Contributor Author

Not exactly. Please see the screencast.

This screencast doesn't toggle DSO markers at all. Yes, I see the ripples on the moving antialiased lines, but it's something that's also present in the released versions, and will exist until we switch to linear-light rendering.

From the new screenshots I see exactly that the width increased when the markers are turned on. Is this the expected width? It seems that your single-pixel lines are the wrong ones.

@alex-w
Copy link
Member

alex-w commented Jul 22, 2023

The second screencast

@10110111
Copy link
Contributor Author

10110111 commented Jul 22, 2023

OK, I can reproduce with a scale factor of 200%, the widening of the lines is not related to this PR. I'll try to address it separately.

@10110111 10110111 force-pushed the dithering-as-post-processing branch 2 times, most recently from f94f57d to 60b9139 Compare July 22, 2023 15:46
@10110111
Copy link
Contributor Author

Line width is fixed in master by b837ea3.

@alex-w
Copy link
Member

alex-w commented Jul 22, 2023

This is not related to this PR, but I see in the log:

Initializing planets GL shaders... 
StelPainter: Warnings while linking transformShaderProgram shader program:
WARNING: Could not find vertex shader attribute 'texCoord' to match BindAttributeLocation request.
WARNING: Could not find vertex shader attribute 'normalIn' to match BindAttributeLocation request.

These warnings looks not very good

@10110111
Copy link
Contributor Author

10110111 commented Jul 22, 2023

This is not related to this PR, but I see in the log

Something like the following should fix them. I don't have these warnings, so cannot test.

diff --git a/src/core/modules/Planet.cpp b/src/core/modules/Planet.cpp
index 4358121c3f..1a478c0884 100644
--- a/src/core/modules/Planet.cpp
+++ b/src/core/modules/Planet.cpp
@@ -3431,7 +3431,13 @@ bool Planet::initShader()
         transformFShader = "void main()\n{ }\n";
     }
 #endif
-    GL(transformShaderProgram = createShader("transformShaderProgram", transformShaderVars, transformVShader, transformFShader,QByteArray(),attrLoc));
+    const QMap<QByteArray,int> transformAttrLoc{
+                                                {"unprojectedVertex", StelOpenGLArray::ATTLOC_VERTEX},
+#ifdef DEBUG_SHADOWMAP
+                                                {"texCoord", StelOpenGLArray::ATTLOC_TEXCOORD},
+#endif
+                                               };
+    GL(transformShaderProgram = createShader("transformShaderProgram", transformShaderVars, transformVShader, transformFShader,QByteArray(),transformAttrLoc));
 
     //check if ALL shaders have been created correctly
     shaderError = !(planetShaderProgram&&

@gzotti
Copy link
Member

gzotti commented Jul 23, 2023

Seems to have problems compiling in Windows/MSVC2017/Qt5.12.12.

external:IC:\Qt\5.12.12\msvc2017_64\include\QtMultimediaWidgets -external:IC:\Qt\5.12.12\msvc2017_64\include\QtScript -external:IC:\Qt\5.12.12\msvc2017_64\include\QtSerialPort -external:W0 /DWIN32 /D_WINDOWS /GR /EHsc /wd4244 /wd4305 /wd4351 /wd4996 /wd5105 /MP /D_WIN32_WINNT=0x0601 /O2 /Ob2 /DNDEBUG -MD -std:c++17 /showIncludes /Fosrc\CMakeFiles\stelMain.dir\core\modules\AtmosphereShowMySky.cpp.obj /Fdsrc\CMakeFiles\stelMain.dir\stelMain.pdb /FS -c D:\StelDev\GIT\stellarium\src\core\modules\AtmosphereShowMySky.cpp
D:\StelDev\GIT\stellarium\src\core\modules\Atmosphere.hpp(45): error C2039: "runtime_error" ist kein Member von "std".
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\random(29): note: Siehe Deklaration von "std"
D:\StelDev\GIT\stellarium\src\core\modules\Atmosphere.hpp(46): error C2504: "runtime_error": Basisklasse undefiniert
D:\StelDev\GIT\stellarium\src\core\modules\Atmosphere.hpp(47): error C2039: "runtime_error" ist kein Member von "std".
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\random(29): note: Siehe Deklaration von "std"
D:\StelDev\GIT\stellarium\src\core\modules\Atmosphere.hpp(47): error C3083: "runtime_error": Das Symbol links neben "::" muss ein Typ sein.
D:\StelDev\GIT\stellarium\src\core\modules\Atmosphere.hpp(47): error C2039: "runtime_error" ist kein Member von "std".
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\random(29): note: Siehe Deklaration von "std"
D:\StelDev\GIT\stellarium\src\core\modules\Atmosphere.hpp(47): error C2873: "runtime_error": Das Symbol kann nicht in einer using-Deklaration verwendet werden
D:\StelDev\GIT\stellarium\src\core\modules\Atmosphere.hpp(48): error C2039: "runtime_error" ist kein Member von "std".
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\random(29): note: Siehe Deklaration von "std"
D:\StelDev\GIT\stellarium\src\core\modules\Atmosphere.hpp(48): error C2614: "Atmosphere::InitFailure": Unzul„ssige Elementinitialisierung: "runtime_error" ist weder Basis noch Element

Low-graphics modes such as ANGLE, OpenGL<3.3, OpenGL Compatibility
Profile, etc. will not have dithering nor multisampling since this
commit.
@10110111 10110111 force-pushed the dithering-as-post-processing branch from 60b9139 to 955e5bd Compare July 23, 2023 21:53
@10110111
Copy link
Contributor Author

Seems to have problems compiling in Windows/MSVC2017/Qt5.12.12.

Fixed missing include.

Copy link
Member

@gzotti gzotti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel a bit sad to see dithering go away from ANGLE builds. But it's bearable.

@10110111 10110111 merged commit aed7d4d into Stellarium:master Jul 24, 2023
10 checks passed
@10110111 10110111 deleted the dithering-as-post-processing branch July 24, 2023 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants