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

Color banding artifacts make atmosphere ugly in low light #131

Closed
10110111 opened this issue Apr 6, 2018 · 70 comments · Fixed by #132
Closed

Color banding artifacts make atmosphere ugly in low light #131

10110111 opened this issue Apr 6, 2018 · 70 comments · Fixed by #132
Assignees
Labels
enhancement Improve existing functionality
Milestone

Comments

@10110111
Copy link
Contributor

10110111 commented Apr 6, 2018

See the following screenshot in 1:1 scale to understand the problem:
screenshot of the problem

This banding can be smoothed by application of dithering. I've done a quick-and-dirty implementation of it just to see which places would need it, and below is a proof-of-concept patch I've come up with. I'm sure it can be improved, including a switch to toggle it on/off as well as enable/disable 6 BPP monitor support (which is currently an #ifdef). Also it might be better to simply do all the rendering to a higher-precision FBO texture (e.g. with GL_RGBA32F format) and then simply dither this render target when blitting to screen. But at least this patch makes the image much nicer (see the screenshot after the diff).

diff --git a/src/core/StelPainter.cpp b/src/core/StelPainter.cpp
index 529407e826..f3d66e5cc3 100644
--- a/src/core/StelPainter.cpp
+++ b/src/core/StelPainter.cpp
@@ -2027,12 +2027,36 @@ void StelPainter::initGLShaders()
 
 	QOpenGLShader fshader2(QOpenGLShader::Fragment);
 	const char *fsrc2 =
+"#version 120\n"
+"vec3 dither(vec3 c)\n"
+"{\n"
+"    const float bayerPattern[] = float[](\n"
+"        0,  32,  8, 40,  2, 34, 10, 42,  /* 8x8 Bayer ordered dithering  */\n"
+"        48, 16, 56, 24, 50, 18, 58, 26,  /* pattern.  Each input pixel   */\n"
+"        12, 44,  4, 36, 14, 46,  6, 38,  /* is scaled to the 0..63 range */\n"
+"        60, 28, 52, 20, 62, 30, 54, 22,  /* before looking in this table */\n"
+"        3,  35, 11, 43,  1, 33,  9, 41,  /* to determine the action.     */\n"
+"        51, 19, 59, 27, 49, 17, 57, 25,\n"
+"        15, 47,  7, 39, 13, 45,  5, 37,\n"
+"        63, 31, 55, 23, 61, 29, 53, 21);\n"
+"    float bayer=bayerPattern[int(mod(int(gl_FragCoord.x),8)+8*mod(int(gl_FragCoord.y),8))]/64;\n"
+#ifdef MONITOR_6_BPP
+        "    const float rgbByteMax=63.;\n"
+#else
+        "    const float rgbByteMax=255.;\n"
+#endif
+"    vec3 rgb=c*rgbByteMax;\n"
+"    vec3 head=floor(rgb);\n"
+"    vec3 tail=fract(rgb);\n"
+"    return (head+step(bayer,tail))/rgbByteMax;\n"
+"}\n"
+"vec4 dither(vec4 c) { return vec4(dither(c.xyz),c.w); }\n"
 		"varying mediump vec2 texc;\n"
 		"uniform sampler2D tex;\n"
 		"uniform mediump vec4 texColor;\n"
 		"void main(void)\n"
 		"{\n"
-		"    gl_FragColor = texture2D(tex, texc)*texColor;\n"
+		"    gl_FragColor = dither(texture2D(tex, texc)*texColor);\n"
 		"}\n";
 	fshader2.compileSourceCode(fsrc2);
 	if (!fshader2.log().isEmpty()) { qWarning() << "StelPainter: Warnings while compiling fshader2: " << fshader2.log(); }
@@ -2067,12 +2091,36 @@ void StelPainter::initGLShaders()
 
 	QOpenGLShader fshader4(QOpenGLShader::Fragment);
 	const char *fsrc4 =
+"#version 120\n"
+"vec3 dither(vec3 c)\n"
+"{\n"
+"    const float bayerPattern[] = float[](\n"
+"        0,  32,  8, 40,  2, 34, 10, 42,  /* 8x8 Bayer ordered dithering  */\n"
+"        48, 16, 56, 24, 50, 18, 58, 26,  /* pattern.  Each input pixel   */\n"
+"        12, 44,  4, 36, 14, 46,  6, 38,  /* is scaled to the 0..63 range */\n"
+"        60, 28, 52, 20, 62, 30, 54, 22,  /* before looking in this table */\n"
+"        3,  35, 11, 43,  1, 33,  9, 41,  /* to determine the action.     */\n"
+"        51, 19, 59, 27, 49, 17, 57, 25,\n"
+"        15, 47,  7, 39, 13, 45,  5, 37,\n"
+"        63, 31, 55, 23, 61, 29, 53, 21);\n"
+"    float bayer=bayerPattern[int(mod(int(gl_FragCoord.x),8)+8*mod(int(gl_FragCoord.y),8))]/64;\n"
+#ifdef MONITOR_6_BPP
+        "    const float rgbByteMax=63.;\n"
+#else
+        "    const float rgbByteMax=255.;\n"
+#endif
+"    vec3 rgb=c*rgbByteMax;\n"
+"    vec3 head=floor(rgb);\n"
+"    vec3 tail=fract(rgb);\n"
+"    return (head+step(bayer,tail))/rgbByteMax;\n"
+"}\n"
+"vec4 dither(vec4 c) { return vec4(dither(c.xyz),c.w); }\n"
 		"varying mediump vec2 texc;\n"
 		"varying mediump vec4 outColor;\n"
 		"uniform sampler2D tex;\n"
 		"void main(void)\n"
 		"{\n"
-		"    gl_FragColor = texture2D(tex, texc)*outColor;\n"
+		"    gl_FragColor = dither(texture2D(tex, texc)*outColor);\n"
 		"}\n";
 	fshader4.compileSourceCode(fsrc4);
 	if (!fshader4.log().isEmpty()) { qWarning() << "StelPainter: Warnings while compiling fshader4: " << fshader4.log(); }
diff --git a/src/core/modules/Atmosphere.cpp b/src/core/modules/Atmosphere.cpp
index 12953ed9ed..20798f554f 100644
--- a/src/core/modules/Atmosphere.cpp
+++ b/src/core/modules/Atmosphere.cpp
@@ -58,10 +58,34 @@ Atmosphere::Atmosphere(void)
 	}
 	QOpenGLShader fShader(QOpenGLShader::Fragment);
 	if (!fShader.compileSourceCode(
+"#version 120\n"
+"vec3 dither(vec3 c)\n"
+"{\n"
+"    const float bayerPattern[] = float[](\n"
+"        0,  32,  8, 40,  2, 34, 10, 42,  /* 8x8 Bayer ordered dithering  */\n"
+"        48, 16, 56, 24, 50, 18, 58, 26,  /* pattern.  Each input pixel   */\n"
+"        12, 44,  4, 36, 14, 46,  6, 38,  /* is scaled to the 0..63 range */\n"
+"        60, 28, 52, 20, 62, 30, 54, 22,  /* before looking in this table */\n"
+"        3,  35, 11, 43,  1, 33,  9, 41,  /* to determine the action.     */\n"
+"        51, 19, 59, 27, 49, 17, 57, 25,\n"
+"        15, 47,  7, 39, 13, 45,  5, 37,\n"
+"        63, 31, 55, 23, 61, 29, 53, 21);\n"
+"    float bayer=bayerPattern[int(mod(int(gl_FragCoord.x),8)+8*mod(int(gl_FragCoord.y),8))]/64;\n"
+#ifdef MONITOR_6_BPP
+        "    const float rgbByteMax=63.;\n"
+#else
+        "    const float rgbByteMax=255.;\n"
+#endif
+"    vec3 rgb=c*rgbByteMax;\n"
+"    vec3 head=floor(rgb);\n"
+"    vec3 tail=fract(rgb);\n"
+"    return (head+step(bayer,tail))/rgbByteMax;\n"
+"}\n"
+"vec4 dither(vec4 c) { return vec4(dither(c.xyz),c.w); }\n"
 					"varying mediump vec3 resultSkyColor;\n"
 					"void main()\n"
 					"{\n"
-					 "   gl_FragColor = vec4(resultSkyColor, 1.);\n"
+					 "   gl_FragColor = vec4(dither(resultSkyColor), 1.);\n"
 					 "}"))
 	{
 		qFatal("Error while compiling atmosphere fragment shader: %s", fShader.log().toLatin1().constData());

Screenshot of the same scene with the above patch:
screenshot with patch

@gzotti
Copy link
Member

gzotti commented Apr 6, 2018

This looks excellent, thank you!
However, I see a #version 120 in the GLSL code. The code must be compatible to GLSL ES 1.0, or there must be a way to run dithered on "true" OpenGL and (if not possible) the old undithered version for OpenGL ES2 (e.g. ANGLE on Windows, or the small ARM SBCs systems which would even profit more when running 16bit colors.

Another element where dithering would really help would be the Zodiacal light.

Unfortunately even some ANGLE version that came with later Qt editions (5.9) is worse than earlier (5.6). See https://bugs.launchpad.net/stellarium/+bug/1731788. If you are familiar with GLSL, maybe you can find what's wrong in our shaders (if it's not really an ANGLE bug?)?

@gzotti gzotti added the enhancement Improve existing functionality label Apr 6, 2018
@10110111
Copy link
Contributor Author

10110111 commented Apr 6, 2018

I think we can make it GLES-compatible if we replace the bayerPattern array with an additional texture supplied to the shader.

@gzotti
Copy link
Member

gzotti commented Apr 7, 2018

We will possibly take any working solution which you can give! It just has to work on all platforms.
Is this MONITOR_6_BPP a compile-time switch, and could this be configured at runtime?

@10110111
Copy link
Contributor Author

10110111 commented Apr 7, 2018

Is this MONITOR_6_BPP a compile-time switch, and could this be configured at runtime?

As the patch is simply a proof of concept, this is a compile-time switch. But in final implementation I think it can be a uniform.

@gzotti
Copy link
Member

gzotti commented Apr 7, 2018

Are you able to implement the required things:

  • code to find out current color mode (is this an OpenGL (ES) query working everywhere or operating system dependent (Windows/X11/MacOS?)), add a uniform switch to the shader
  • develop this required Bayer texture
  • put such a dithering shader into the classes where applicable
  • Maybe even introduce a config option for 10bit colors? Modern GPUs should work with that. But I am not sure if some data are only thinking in 8bit colors.

It really looks good, and you appear to have the necessary knowledge for this. I can test on Windows (OpenGL and ANGLE/GL ES2) and on 2 ARM boards (GL ES2). (Not daily, but weekends should be ok).

@10110111
Copy link
Contributor Author

10110111 commented Apr 7, 2018

code to find out current color mode (is this an OpenGL (ES) query working everywhere or operating system dependent (Windows/X11/MacOS?)), add a uniform switch to the shader

Do you mean the monitor color depth — 6 vs 8 BPP? Not sure. Maybe it's doable on mobile devices, but there are some desktop monitors with 6 BPP TN matrices (e.g. old but still used by me SyncMaster 193p Plus). For a beginning I'd make the whole thing configurable like

  • No dithering (safe default)
  • 8 BPP dithering
  • 6 BPP dithering

After implementing this we might try to choose a custom default on devices which can provide color depth of their monitor.

develop this required Bayer texture

Yes, I guess this shouldn't be hard.

Maybe even introduce a config option for 10bit colors?

Do you mean native 10-BPP monitors? I've not even seen such, not even speaking of being able to test, but I guess simply adding an rgbByteMax=1023/10 BPP option to the list of dithering modes should be enough for this to work.

or the small ARM SBCs systems which would even profit more when running 16bit colors

What is the RGBX layout of 16-bit there? 5-6-5 or maybe 5-5-5-1? For the former rgbByteMax would need to be a bit trickier, like vec2(31,63,31).

@gzotti
Copy link
Member

gzotti commented Apr 7, 2018

Hm, I had actually assumed 6bit means 5-6-5 ("64k color mode"), did not think of mobile devices. I have not used anything but 8-8-8 for many years now, but some people seem to still use 64k colors on lesser systems.

On 10bit colors, I also have no such device, but some new monitors seem to have introduced it. We should be prepared for those if possible, then. (Or maybe 10bit colors can just work undithered already?)

RGBX layout: I know I have last seen 16bit colors on some X11 system, also years ago, and you can set it in some xorg.conf files. I googled around a bit, people seem to agree that all are using (at least) 24 bit now. Apparently there are no more 16bit colors available in Win/Mac (?) So I am not sure if it is indeed relevant, OTOH dithering is all the more relevant on low-color systems. Is there any speed gain in 16bit mode? Or was that a framebuffer memory question? One discussion I found was about remote desktops which may be shown in 16bit colors. Now, I am not an expert in X11 settings, and use remote desktops only in rare cases. Would that even matter here (@alex-w @guillaumechereau any opinions?)? A remote desktop system packs the final frame whatever its mode and may color-compress it to 16 bits, right? So in this case likely 16bit-aware dithering is not required.

On Ubuntu Mate 16.04 on a Raspberry Pi 3 (with Mesa17 and VC4 driver) I ran GLXInfo. It lists 216 GLX Visuals with colorbuffer values all 8/8/8/8 or 8/8/8/0. Then there is a list of 270 GLXFBConfigs with colorbuffers 8/8/8/8, 8/8/8/0 and 5/6/5/0. I have not yet reconfigured to 16bit mode to see how this looks, or how Stellarium would look.

@10110111
Copy link
Contributor Author

10110111 commented Apr 7, 2018

Here's a WIP next version of the patch. Now it:

  • Uses texture sampling instead of array indexing
  • Gets rid of #version 120 requirement
  • Passes rgbMaxValue as a uniform, although its value is currently hard-coded, and even repeated in two places.

Could you give me some pointers how (where) to better implement the config option for dithering mode (which will affect rgbMaxValue)?

diff --git a/src/bayer-pattern.hpp b/src/bayer-pattern.hpp
new file mode 100644
index 0000000000..dcd62fec21
--- /dev/null
+++ b/src/bayer-pattern.hpp
@@ -0,0 +1,12 @@
+static const float bayerPattern[8*8] =
+{
+	// 8x8 Bayer ordered dithering pattern.
+	 0/64.f, 32/64.f,  8/64.f, 40/64.f,  2/64.f, 34/64.f, 10/64.f, 42/64.f,
+	48/64.f, 16/64.f, 56/64.f, 24/64.f, 50/64.f, 18/64.f, 58/64.f, 26/64.f,
+	12/64.f, 44/64.f,  4/64.f, 36/64.f, 14/64.f, 46/64.f,  6/64.f, 38/64.f,
+	60/64.f, 28/64.f, 52/64.f, 20/64.f, 62/64.f, 30/64.f, 54/64.f, 22/64.f,
+	 3/64.f, 35/64.f, 11/64.f, 43/64.f,  1/64.f, 33/64.f,  9/64.f, 41/64.f,
+	51/64.f, 19/64.f, 59/64.f, 27/64.f, 49/64.f, 17/64.f, 57/64.f, 25/64.f,
+	15/64.f, 47/64.f,  7/64.f, 39/64.f, 13/64.f, 45/64.f,  5/64.f, 37/64.f,
+	63/64.f, 31/64.f, 55/64.f, 23/64.f, 61/64.f, 29/64.f, 53/64.f, 21/64.f
+};
diff --git a/src/core/StelPainter.cpp b/src/core/StelPainter.cpp
index 529407e826..420211bc71 100644
--- a/src/core/StelPainter.cpp
+++ b/src/core/StelPainter.cpp
@@ -2027,12 +2027,13 @@ void StelPainter::initGLShaders()
 
 	QOpenGLShader fshader2(QOpenGLShader::Fragment);
 	const char *fsrc2 =
+#include "dither.glsl"
 		"varying mediump vec2 texc;\n"
 		"uniform sampler2D tex;\n"
 		"uniform mediump vec4 texColor;\n"
 		"void main(void)\n"
 		"{\n"
-		"    gl_FragColor = texture2D(tex, texc)*texColor;\n"
+		"    gl_FragColor = dither(texture2D(tex, texc)*texColor);\n"
 		"}\n";
 	fshader2.compileSourceCode(fsrc2);
 	if (!fshader2.log().isEmpty()) { qWarning() << "StelPainter: Warnings while compiling fshader2: " << fshader2.log(); }
@@ -2046,6 +2047,8 @@ void StelPainter::initGLShaders()
 	texturesShaderVars.vertex = texturesShaderProgram->attributeLocation("vertex");
 	texturesShaderVars.texColor = texturesShaderProgram->uniformLocation("texColor");
 	texturesShaderVars.texture = texturesShaderProgram->uniformLocation("tex");
+	texturesShaderVars.bayerPattern = texturesShaderProgram->uniformLocation("bayerPattern");
+	texturesShaderVars.rgbMaxValue = texturesShaderProgram->uniformLocation("rgbMaxValue");
 
 	// Texture shader program + interpolated color per vertex
 	QOpenGLShader vshader4(QOpenGLShader::Vertex);
@@ -2067,12 +2070,13 @@ void StelPainter::initGLShaders()
 
 	QOpenGLShader fshader4(QOpenGLShader::Fragment);
 	const char *fsrc4 =
+#include "dither.glsl"
 		"varying mediump vec2 texc;\n"
 		"varying mediump vec4 outColor;\n"
 		"uniform sampler2D tex;\n"
 		"void main(void)\n"
 		"{\n"
-		"    gl_FragColor = texture2D(tex, texc)*outColor;\n"
+		"    gl_FragColor = dither(texture2D(tex, texc)*outColor);\n"
 		"}\n";
 	fshader4.compileSourceCode(fsrc4);
 	if (!fshader4.log().isEmpty()) { qWarning() << "StelPainter: Warnings while compiling fshader4: " << fshader4.log(); }
@@ -2086,6 +2090,8 @@ void StelPainter::initGLShaders()
 	texturesColorShaderVars.vertex = texturesColorShaderProgram->attributeLocation("vertex");
 	texturesColorShaderVars.color = texturesColorShaderProgram->attributeLocation("color");
 	texturesColorShaderVars.texture = texturesColorShaderProgram->uniformLocation("tex");
+	texturesColorShaderVars.bayerPattern = texturesColorShaderProgram->uniformLocation("bayerPattern");
+	texturesColorShaderVars.rgbMaxValue = texturesColorShaderProgram->uniformLocation("rgbMaxValue");
 }
 
 
@@ -2146,6 +2152,19 @@ void StelPainter::drawFromArray(DrawingMode mode, int count, int offset, bool do
 	const Mat4f& m = getProjector()->getProjectionMatrix();
 	const QMatrix4x4 qMat(m[0], m[4], m[8], m[12], m[1], m[5], m[9], m[13], m[2], m[6], m[10], m[14], m[3], m[7], m[11], m[15]);
 
+	const auto makeBayerPatternTexture=[this]{
+		GLuint tex;
+		glGenTextures(1, &tex);
+		glBindTexture(GL_TEXTURE_2D, tex);
+		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);
+		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_REPEAT);
+#include "bayer-pattern.hpp"
+		glTexImage2D(GL_TEXTURE_2D, 0, GL_RED, 8, 8, 0, GL_RED, GL_FLOAT, bayerPattern);
+		return tex;
+	};
+	const auto rgbMaxValue=Vec3f(255,255,255); // TODO: to the options
 	if (!texCoordArray.enabled && !colorArray.enabled && !normalArray.enabled)
 	{
 		pr = basicShaderProgram;
@@ -2166,6 +2185,12 @@ void StelPainter::drawFromArray(DrawingMode mode, int count, int offset, bool do
 		pr->setAttributeArray(texturesShaderVars.texCoord, texCoordArray.type, texCoordArray.pointer, texCoordArray.size);
 		pr->enableAttributeArray(texturesShaderVars.texCoord);
 		//pr->setUniformValue(texturesShaderVars.texture, 0);    // use texture unit 0
+		glActiveTexture(GL_TEXTURE1);
+		if(!bayerPatternTex)
+			bayerPatternTex=makeBayerPatternTexture();
+		glBindTexture(GL_TEXTURE_2D, bayerPatternTex);
+		pr->setUniformValue(texturesShaderVars.bayerPattern, 1);
+		pr->setUniformValue(texturesShaderVars.rgbMaxValue, rgbMaxValue[0], rgbMaxValue[1], rgbMaxValue[2]);
 	}
 	else if (texCoordArray.enabled && colorArray.enabled && !normalArray.enabled)
 	{
@@ -2179,6 +2204,12 @@ void StelPainter::drawFromArray(DrawingMode mode, int count, int offset, bool do
 		pr->setAttributeArray(texturesColorShaderVars.color, colorArray.type, colorArray.pointer, colorArray.size);
 		pr->enableAttributeArray(texturesColorShaderVars.color);
 		//pr->setUniformValue(texturesShaderVars.texture, 0);    // use texture unit 0
+		glActiveTexture(GL_TEXTURE1);
+		if(!bayerPatternTex)
+			bayerPatternTex=makeBayerPatternTexture();
+		glBindTexture(GL_TEXTURE_2D, bayerPatternTex);
+		pr->setUniformValue(texturesColorShaderVars.bayerPattern, 1);
+		pr->setUniformValue(texturesColorShaderVars.rgbMaxValue, rgbMaxValue[0], rgbMaxValue[1], rgbMaxValue[2]);
 	}
 	else if (!texCoordArray.enabled && colorArray.enabled && !normalArray.enabled)
 	{
diff --git a/src/core/StelPainter.hpp b/src/core/StelPainter.hpp
index e08f8e2724..3889963ac5 100644
--- a/src/core/StelPainter.hpp
+++ b/src/core/StelPainter.hpp
@@ -402,6 +402,8 @@ private:
 		int vertex;
 		int texColor;
 		int texture;
+		int bayerPattern;
+		int rgbMaxValue;
 	};
 	static TexturesShaderVars texturesShaderVars;
 	static QOpenGLShaderProgram* texturesColorShaderProgram;
@@ -411,9 +413,12 @@ private:
 		int vertex;
 		int color;
 		int texture;
+		int bayerPattern;
+		int rgbMaxValue;
 	};
 	static TexturesColorShaderVars texturesColorShaderVars;
 
+	GLuint bayerPatternTex=0;
 
 	//! The descriptor for the current opengl vertex array
 	ArrayDesc vertexArray;
diff --git a/src/core/modules/Atmosphere.cpp b/src/core/modules/Atmosphere.cpp
index 12953ed9ed..46cfd21af7 100644
--- a/src/core/modules/Atmosphere.cpp
+++ b/src/core/modules/Atmosphere.cpp
@@ -58,10 +58,11 @@ Atmosphere::Atmosphere(void)
 	}
 	QOpenGLShader fShader(QOpenGLShader::Fragment);
 	if (!fShader.compileSourceCode(
+#include "dither.glsl"
 					"varying mediump vec3 resultSkyColor;\n"
 					"void main()\n"
 					"{\n"
-					 "   gl_FragColor = vec4(resultSkyColor, 1.);\n"
+					 "   gl_FragColor = vec4(dither(resultSkyColor), 1.);\n"
 					 "}"))
 	{
 		qFatal("Error while compiling atmosphere fragment shader: %s", fShader.log().toLatin1().constData());
@@ -76,6 +77,8 @@ Atmosphere::Atmosphere(void)
 	StelPainter::linkProg(atmoShaderProgram, "atmosphere");
 
 	atmoShaderProgram->bind();
+	shaderAttribLocations.bayerPattern = atmoShaderProgram->uniformLocation("bayerPattern");
+	shaderAttribLocations.rgbMaxValue = atmoShaderProgram->uniformLocation("rgbMaxValue");
 	shaderAttribLocations.alphaWaOverAlphaDa = atmoShaderProgram->uniformLocation("alphaWaOverAlphaDa");
 	shaderAttribLocations.oneOverGamma = atmoShaderProgram->uniformLocation("oneOverGamma");
 	shaderAttribLocations.term2TimesOneOverMaxdLpOneOverGamma = atmoShaderProgram->uniformLocation("term2TimesOneOverMaxdLpOneOverGamma");
@@ -368,6 +371,28 @@ void Atmosphere::draw(StelCore* core)
 	const Mat4f& m = sPainter.getProjector()->getProjectionMatrix();
 	atmoShaderProgram->setUniformValue(shaderAttribLocations.projectionMatrix,
 		QMatrix4x4(m[0], m[4], m[8], m[12], m[1], m[5], m[9], m[13], m[2], m[6], m[10], m[14], m[3], m[7], m[11], m[15]));
+
+	const auto rgbMaxValue=Vec3f(255,255,255); // TODO: to the options
+	atmoShaderProgram->setUniformValue(shaderAttribLocations.rgbMaxValue, rgbMaxValue[0], rgbMaxValue[1], rgbMaxValue[2]);
+	const auto makeBayerPatternTexture=[&sPainter]{
+		GLuint tex;
+		const auto& gl=sPainter.glFuncs();
+		gl->glGenTextures(1, &tex);
+		gl->glBindTexture(GL_TEXTURE_2D, tex);
+		gl->glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+		gl->glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+		gl->glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);
+		gl->glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_REPEAT);
+#include "bayer-pattern.hpp"
+		gl->glTexImage2D(GL_TEXTURE_2D, 0, GL_RED, 8, 8, 0, GL_RED, GL_FLOAT, bayerPattern);
+		return tex;
+	};
+	const auto& gl=sPainter.glFuncs();
+	gl->glActiveTexture(GL_TEXTURE1);
+	if(!bayerPatternTex)
+		bayerPatternTex=makeBayerPatternTexture();
+	gl->glBindTexture(GL_TEXTURE_2D, bayerPatternTex);
+	atmoShaderProgram->setUniformValue(shaderAttribLocations.bayerPattern, 1);
 	
 	colorGridBuffer.bind();
 	atmoShaderProgram->setAttributeBuffer(shaderAttribLocations.skyColor, GL_FLOAT, 0, 4, 0);
diff --git a/src/core/modules/Atmosphere.hpp b/src/core/modules/Atmosphere.hpp
index 9553008929..18e697cbc5 100644
--- a/src/core/modules/Atmosphere.hpp
+++ b/src/core/modules/Atmosphere.hpp
@@ -103,6 +103,8 @@ private:
 	//! Vertex shader used for xyYToRGB computation
 	class QOpenGLShaderProgram* atmoShaderProgram;
 	struct {
+		int bayerPattern;
+		int rgbMaxValue;
 		int alphaWaOverAlphaDa;
 		int oneOverGamma;
 		int term2TimesOneOverMaxdLpOneOverGamma;
@@ -114,6 +116,8 @@ private:
 		int skyVertex;
 		int skyColor;
 	} shaderAttribLocations;
+
+	GLuint bayerPatternTex=0;
 };
 
 #endif // _ATMOSTPHERE_HPP_
diff --git a/src/dither.glsl b/src/dither.glsl
new file mode 100644
index 0000000000..b86fc52979
--- /dev/null
+++ b/src/dither.glsl
@@ -0,0 +1,13 @@
+R"(uniform vec3 rgbMaxValue;
+uniform sampler2D bayerPattern;
+vec3 dither(vec3 c)
+{
+    float bayer=texture2D(bayerPattern,gl_FragCoord.xy/8.).r;
+
+    vec3 rgb=c*rgbMaxValue;
+    vec3 head=floor(rgb);
+    vec3 tail=fract(rgb);
+    return (head+step(bayer,tail))/rgbMaxValue;
+}
+vec4 dither(vec4 c) { return vec4(dither(c.xyz),c.w); }
+)"

@gzotti
Copy link
Member

gzotti commented Apr 8, 2018

For configurations: Look e.g. into StelCore.cpp. In its constructor you see config handling.
A similar construct should then go into StelPainter's constructor. The setting could be called "video/dithering" with default=true. Either a Boolean with number of bits detected from OpenGL environment (Format?) or a number of bits? I think it should best be auto-detected, with a fallback to "false" if something goes wrong.
The setting does not have to become a full "Q_PROPERTY" of StelPainter, just a regular private variable, as it will likely not be changed by user action at runtime. (Unless you want to show the effect?)

Please move any .glsl files into data/shaders.

I have not tested your code so far, but avoiding 1.20 should help in ES2 modes, thanks. I will try to configure a Raspberry to 16bit mode to see whether this even works, and whether this gives any performance bonus. (But probably the whole OpenGL setup must be changed then? It is possible we don't need 5/6/5. Else if there is a significant fps increase, dithering is a big plus to hide its ugliness.)

@gzotti
Copy link
Member

gzotti commented Apr 8, 2018

Seems I cannot find the proper xorg.conf lines for a Screen section that defines 16bit colors. Ubuntu Mate 16.04 does not boot, neither with the default nor the VC4 driver overlay.
Who is willing to test some 16bit layout? Or else we skip that. It seems nobody ever bothered about 5/6/5 on an SBC (?) But 10bit colors may be come a topic of relevance within 0-5 years.

@10110111
Copy link
Contributor Author

10110111 commented Apr 8, 2018

Just tried it with my Intel(R) HD Graphics 4600 by simply running a new Xorg instance like

startx `which xterm` -- :1 -depth 16

After changing rgbMaxValue=Vec3f(255,255,255); to rgbMaxValue=Vec3f(31,63,31); in the two files containing this, I got quite a nice picture (as compared to non-dithered version).

@10110111
Copy link
Contributor Author

10110111 commented Apr 8, 2018

On nvidia, though, I get black window in 16-bit mode, even without dithering...

@gzotti
Copy link
Member

gzotti commented Apr 8, 2018

OK, that means support is hardware dependent and cannot be guaranteed, but your dithering runs when available, good! Now you would just have to find out color depth in the StelPainter constructor (or elsewhere) to set the rgbMaxValue.

Is there any noticeable difference in frame rate between 24bit and 16bit depth on the HD4600? Or is it both 60frames by sync rate? I just want to estimate whether struggling to set some other ARM system (with Mali) into 16bit can make any (positive) difference.

@10110111
Copy link
Contributor Author

10110111 commented Apr 8, 2018

OK, that means support is hardware dependent and cannot be guaranteed

Not quite. Other OpenGL programs, including GLSL-intensive ones like Precomputed Atmospheric Scattering demo work perfectly (and PAS looks great when I add dithering to it). At the same time, Stellarium just draws black frames. But that's another issue.

Is there any noticeable difference in frame rate between 24bit and 16bit depth on the HD4600?

I failed to see any subjectively noticeable difference. And the FPS counter at the bottom panel appears unreliable: when I don't move the camera, it simply decays to 18.2 FPS regardless of the mode, although it does sometimes go higher than 50 FPS when I rotate/zoom etc.. So I don't know how to do a reliable benchmark.

@gzotti
Copy link
Member

gzotti commented Apr 8, 2018

That's a feature: Stellarium falls back to a low rate to conserve energy. You can increase [video]/minimum_fps in config.ini. High framerates are of course best-effort.

Wow, the PAS is what we need as next step over the current atmosphere model! How does it behave at night? (twilight with earth shadow is great!)

@10110111
Copy link
Contributor Author

10110111 commented Apr 8, 2018

How does it behave at night?

Well, unsurprisingly, it becomes dark :) . In the demo you can increase exposure to see more, but it'll simply be the scattering of sunlight. PAS doesn't model airglow nor Aurorae, so it might be not an ideal fit for Stellarium (does Stellarium model them, though?).

@10110111
Copy link
Contributor Author

10110111 commented Apr 8, 2018

Please move any .glsl files into data/shaders.

Are you sure dither.glsl should be moved there? It's actually a C++ source file, containing the raw literal with the GLSL source. Maybe it should be simply renamed to .cpp or .hpp?

@gzotti
Copy link
Member

gzotti commented Apr 8, 2018

PAS: I meant how does it look in deep twilight? I should try this myself, obviously, if I find time. Stellarium has airglow brightness from Schaefer's sky brightness model, but no aurorae. The physics-based values have to be checked and compared/recalibrated/better documented in the process.

Ah right, it's rather a header-type file to be included. @alex-w what is the best place then? Just src/core?

@10110111
Copy link
Contributor Author

10110111 commented Apr 8, 2018

I think it should best be auto-detected, with a fallback to "false" if something goes wrong.
The setting does not have to become a full "Q_PROPERTY" of StelPainter, just a regular private variable, as it will likely not be changed by user action at runtime. (Unless you want to show the effect?)

Actually this will look like garbage on TN+film monitors, which work in 24-bit mode (as far as video card is concerned), thus giving OpenGL app 8/8/8 GLX visuals, but only have 6 BPP matrix in reality. This may work nicely if the video card in use supports temporal dithering by itself and has it enabled, but e.g. Intel video chips don't, and on nvidia it's not the default mode AFAIR.

Are you still sure you don't want to let the user choose dithering mode?

@gzotti
Copy link
Member

gzotti commented Apr 8, 2018

Phew, good question. Regular users don't want to tweak settings, but they also don't like unpleasant surprises. But I was not aware of this TN 6bit issue, I had an IPS following my CRT... If this cannot be detected, dithering should default to off, and modes only edited manually.

Is this a setting that should be set interactively to see the effect immediately? Then we should be able to add a combo none/6bit/8bit/10bit into the configuration panel. It is enough if you work with the flag from config.ini, we will do the GUI stuff.

video/dithering should in this case be an int with usable values 0/6/8/10. I have no TN panel to test.

@alex-w
Copy link
Member

alex-w commented Apr 8, 2018

@gzotti Yes, just src/core will be best place

@10110111
Copy link
Contributor Author

10110111 commented Apr 8, 2018

@gzotti
To see how much of an issue it is, here's a pair of photos of the same scene displayed on SyncMaster 193P Plus:
rgbMaxValue=vec3(255) (which would be based on GLX visual):
maxval255
rgbMaxValue=vec3(63) (actual value for the monitor, despite current 24-bit video mode):
maxval63

@10110111
Copy link
Contributor Author

10110111 commented Apr 8, 2018

Is this a setting that should be set interactively to see the effect immediately?

I suppose yes, the user should be able to see whether current setting is better and easily revert if it's not.

@gzotti
Copy link
Member

gzotti commented Apr 8, 2018

Thanks for the images, very instructive. OK, let's make this interactive, default none, with Q_PROPERTY and GUI stuff. But you can leave this to us, just the 6/8 and if you can envision it, 10bit modes please, settable in config.ini.

@10110111
Copy link
Contributor Author

10110111 commented Apr 9, 2018

Current incarnation of the patch:

  • Implements reading dithering mode from the settings
  • Reorganizes dithering code a bit

Is this work with the settings what you intended it to be at this stage?

diff --git a/src/core/Dithering.hpp b/src/core/Dithering.hpp
new file mode 100644
index 0000000..3ccfc6f
--- /dev/null
+++ b/src/core/Dithering.hpp
@@ -0,0 +1,70 @@
+#ifndef DITHERING_HPP
+#define DITHERING_HPP
+
+#include "StelOpenGL.hpp"
+#include "StelPainter.hpp"
+#include "VecMath.hpp"
+
+inline GLuint makeBayerPatternTexture(QOpenGLFunctions& gl)
+{
+    GLuint tex;
+    gl.glGenTextures(1, &tex);
+    gl.glBindTexture(GL_TEXTURE_2D, tex);
+    gl.glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    gl.glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    gl.glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);
+    gl.glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_REPEAT);
+    static constexpr float bayerPattern[8*8] =
+    {
+        // 8x8 Bayer ordered dithering pattern.
+         0/64.f, 32/64.f,  8/64.f, 40/64.f,  2/64.f, 34/64.f, 10/64.f, 42/64.f,
+        48/64.f, 16/64.f, 56/64.f, 24/64.f, 50/64.f, 18/64.f, 58/64.f, 26/64.f,
+        12/64.f, 44/64.f,  4/64.f, 36/64.f, 14/64.f, 46/64.f,  6/64.f, 38/64.f,
+        60/64.f, 28/64.f, 52/64.f, 20/64.f, 62/64.f, 30/64.f, 54/64.f, 22/64.f,
+         3/64.f, 35/64.f, 11/64.f, 43/64.f,  1/64.f, 33/64.f,  9/64.f, 41/64.f,
+        51/64.f, 19/64.f, 59/64.f, 27/64.f, 49/64.f, 17/64.f, 57/64.f, 25/64.f,
+        15/64.f, 47/64.f,  7/64.f, 39/64.f, 13/64.f, 45/64.f,  5/64.f, 37/64.f,
+        63/64.f, 31/64.f, 55/64.f, 23/64.f, 61/64.f, 29/64.f, 53/64.f, 21/64.f
+    };
+    gl.glTexImage2D(GL_TEXTURE_2D, 0, GL_RED, 8, 8, 0, GL_RED, GL_FLOAT, bayerPattern);
+    return tex;
+};
+
+inline Vec3f calcRGBMaxValue(StelPainter::DitheringMode mode)
+{
+    switch(mode)
+    {
+    default:
+    case StelPainter::DitheringMode::Disabled:
+        return Vec3f(0.);
+    case StelPainter::DitheringMode::Color666:
+        return Vec3f(63);
+    case StelPainter::DitheringMode::Color565:
+        return Vec3f(31,63,31);
+    case StelPainter::DitheringMode::Color888:
+        return Vec3f(255);
+    case StelPainter::DitheringMode::Color101010:
+        return Vec3f(1024);
+    }
+};
+
+inline QString makeDitheringShader()
+{
+    return
+R"(uniform vec3 rgbMaxValue;
+uniform sampler2D bayerPattern;
+vec3 dither(vec3 c)
+{
+    if(rgbMaxValue.r==0.) return c;
+    float bayer=texture2D(bayerPattern,gl_FragCoord.xy/8.).r;
+
+    vec3 rgb=c*rgbMaxValue;
+    vec3 head=floor(rgb);
+    vec3 tail=fract(rgb);
+    return (head+step(bayer,tail))/rgbMaxValue;
+}
+vec4 dither(vec4 c) { return vec4(dither(c.xyz),c.w); }
+)";
+}
+
+#endif
diff --git a/src/core/StelPainter.cpp b/src/core/StelPainter.cpp
index 529407e..535551c 100644
--- a/src/core/StelPainter.cpp
+++ b/src/core/StelPainter.cpp
@@ -24,6 +24,7 @@
 #include "StelProjector.hpp"
 #include "StelProjectorClasses.hpp"
 #include "StelUtils.hpp"
+#include "Dithering.hpp"
 
 #include <QDebug>
 #include <QString>
@@ -110,6 +111,17 @@ bool StelPainter::linkProg(QOpenGLShaderProgram* prog, const QString& name)
 	return ret;
 }
 
+StelPainter::DitheringMode StelPainter::parseDitheringMode(QString const& str)
+{
+    const auto s=str.trimmed().toLower();
+    if(s=="disabled"   ) return DitheringMode::Disabled;
+    if(s=="color565"   ) return DitheringMode::Color565;
+    if(s=="color666"   ) return DitheringMode::Color666;
+    if(s=="color888"   ) return DitheringMode::Color888;
+    if(s=="color101010") return DitheringMode::Color101010;
+    return DitheringMode::Disabled;
+}
+
 StelPainter::StelPainter(const StelProjectorP& proj) : QOpenGLFunctions(QOpenGLContext::currentContext()), glState(this)
 {
 	Q_ASSERT(proj);
@@ -136,6 +148,9 @@ StelPainter::StelPainter(const StelProjectorP& proj) : QOpenGLFunctions(QOpenGLC
 	glStencilMask(0x11111111);
 	glState.apply(); //apply default OpenGL state
 	setProjector(proj);
+
+    QSettings*const conf = StelApp::getInstance().getSettings();
+	ditheringMode = parseDitheringMode(conf->value("video/dithering_mode").toString());
 }
 
 void StelPainter::setProjector(const StelProjectorP& p)
@@ -2026,13 +2041,14 @@ void StelPainter::initGLShaders()
 	if (!vshader2.log().isEmpty()) { qWarning() << "StelPainter: Warnings while compiling vshader2: " << vshader2.log(); }
 
 	QOpenGLShader fshader2(QOpenGLShader::Fragment);
-	const char *fsrc2 =
+	const auto fsrc2 =
+        makeDitheringShader()+
 		"varying mediump vec2 texc;\n"
 		"uniform sampler2D tex;\n"
 		"uniform mediump vec4 texColor;\n"
 		"void main(void)\n"
 		"{\n"
-		"    gl_FragColor = texture2D(tex, texc)*texColor;\n"
+		"    gl_FragColor = dither(texture2D(tex, texc)*texColor);\n"
 		"}\n";
 	fshader2.compileSourceCode(fsrc2);
 	if (!fshader2.log().isEmpty()) { qWarning() << "StelPainter: Warnings while compiling fshader2: " << fshader2.log(); }
@@ -2046,6 +2062,8 @@ void StelPainter::initGLShaders()
 	texturesShaderVars.vertex = texturesShaderProgram->attributeLocation("vertex");
 	texturesShaderVars.texColor = texturesShaderProgram->uniformLocation("texColor");
 	texturesShaderVars.texture = texturesShaderProgram->uniformLocation("tex");
+	texturesShaderVars.bayerPattern = texturesShaderProgram->uniformLocation("bayerPattern");
+	texturesShaderVars.rgbMaxValue = texturesShaderProgram->uniformLocation("rgbMaxValue");
 
 	// Texture shader program + interpolated color per vertex
 	QOpenGLShader vshader4(QOpenGLShader::Vertex);
@@ -2066,13 +2084,14 @@ void StelPainter::initGLShaders()
 	if (!vshader4.log().isEmpty()) { qWarning() << "StelPainter: Warnings while compiling vshader4: " << vshader4.log(); }
 
 	QOpenGLShader fshader4(QOpenGLShader::Fragment);
-	const char *fsrc4 =
+	const auto fsrc4 =
+        makeDitheringShader()+
 		"varying mediump vec2 texc;\n"
 		"varying mediump vec4 outColor;\n"
 		"uniform sampler2D tex;\n"
 		"void main(void)\n"
 		"{\n"
-		"    gl_FragColor = texture2D(tex, texc)*outColor;\n"
+		"    gl_FragColor = dither(texture2D(tex, texc)*outColor);\n"
 		"}\n";
 	fshader4.compileSourceCode(fsrc4);
 	if (!fshader4.log().isEmpty()) { qWarning() << "StelPainter: Warnings while compiling fshader4: " << fshader4.log(); }
@@ -2086,6 +2105,8 @@ void StelPainter::initGLShaders()
 	texturesColorShaderVars.vertex = texturesColorShaderProgram->attributeLocation("vertex");
 	texturesColorShaderVars.color = texturesColorShaderProgram->attributeLocation("color");
 	texturesColorShaderVars.texture = texturesColorShaderProgram->uniformLocation("tex");
+	texturesColorShaderVars.bayerPattern = texturesColorShaderProgram->uniformLocation("bayerPattern");
+	texturesColorShaderVars.rgbMaxValue = texturesColorShaderProgram->uniformLocation("rgbMaxValue");
 }
 
 
@@ -2146,6 +2167,7 @@ void StelPainter::drawFromArray(DrawingMode mode, int count, int offset, bool do
 	const Mat4f& m = getProjector()->getProjectionMatrix();
 	const QMatrix4x4 qMat(m[0], m[4], m[8], m[12], m[1], m[5], m[9], m[13], m[2], m[6], m[10], m[14], m[3], m[7], m[11], m[15]);
 
+	const auto rgbMaxValue=calcRGBMaxValue(ditheringMode);
 	if (!texCoordArray.enabled && !colorArray.enabled && !normalArray.enabled)
 	{
 		pr = basicShaderProgram;
@@ -2166,6 +2188,12 @@ void StelPainter::drawFromArray(DrawingMode mode, int count, int offset, bool do
 		pr->setAttributeArray(texturesShaderVars.texCoord, texCoordArray.type, texCoordArray.pointer, texCoordArray.size);
 		pr->enableAttributeArray(texturesShaderVars.texCoord);
 		//pr->setUniformValue(texturesShaderVars.texture, 0);    // use texture unit 0
+		glActiveTexture(GL_TEXTURE1);
+		if(!bayerPatternTex)
+			bayerPatternTex=makeBayerPatternTexture(*this);
+		glBindTexture(GL_TEXTURE_2D, bayerPatternTex);
+		pr->setUniformValue(texturesShaderVars.bayerPattern, 1);
+		pr->setUniformValue(texturesShaderVars.rgbMaxValue, rgbMaxValue[0], rgbMaxValue[1], rgbMaxValue[2]);
 	}
 	else if (texCoordArray.enabled && colorArray.enabled && !normalArray.enabled)
 	{
@@ -2179,6 +2207,12 @@ void StelPainter::drawFromArray(DrawingMode mode, int count, int offset, bool do
 		pr->setAttributeArray(texturesColorShaderVars.color, colorArray.type, colorArray.pointer, colorArray.size);
 		pr->enableAttributeArray(texturesColorShaderVars.color);
 		//pr->setUniformValue(texturesShaderVars.texture, 0);    // use texture unit 0
+		glActiveTexture(GL_TEXTURE1);
+		if(!bayerPatternTex)
+			bayerPatternTex=makeBayerPatternTexture(*this);
+		glBindTexture(GL_TEXTURE_2D, bayerPatternTex);
+		pr->setUniformValue(texturesColorShaderVars.bayerPattern, 1);
+		pr->setUniformValue(texturesColorShaderVars.rgbMaxValue, rgbMaxValue[0], rgbMaxValue[1], rgbMaxValue[2]);
 	}
 	else if (!texCoordArray.enabled && colorArray.enabled && !normalArray.enabled)
 	{
diff --git a/src/core/StelPainter.hpp b/src/core/StelPainter.hpp
index e08f8e2..612ba2c 100644
--- a/src/core/StelPainter.hpp
+++ b/src/core/StelPainter.hpp
@@ -63,6 +63,15 @@ public:
 		TriangleFan                 = 0x0006  //!< GL_TRIANGLE_FAN
 	};
 
+    enum class DitheringMode
+    {
+        Disabled,    //!< Dithering disabled, will leave the infamous color bands
+        Color565,    //!< 16-bit color (AKA High color) with R5_G6_B5 layout
+        Color666,    //!< TN+film typical color depth in TrueColor mode
+        Color888,    //!< 24-bit color (AKA True color)
+        Color101010, //!< 30-bit color (AKA Deep color)
+    };
+
 	explicit StelPainter(const StelProjectorP& prj);
 	~StelPainter();
 
@@ -306,6 +315,8 @@ public:
 	//! @return true if the link was successful.
 	static bool linkProg(class QOpenGLShaderProgram* prog, const QString& name);
 
+    DitheringMode getDitheringMode() const { return ditheringMode; }
+
 private:
 
 	friend class StelTextureMgr;
@@ -402,6 +413,8 @@ private:
 		int vertex;
 		int texColor;
 		int texture;
+		int bayerPattern;
+		int rgbMaxValue;
 	};
 	static TexturesShaderVars texturesShaderVars;
 	static QOpenGLShaderProgram* texturesColorShaderProgram;
@@ -411,9 +424,14 @@ private:
 		int vertex;
 		int color;
 		int texture;
+		int bayerPattern;
+		int rgbMaxValue;
 	};
 	static TexturesColorShaderVars texturesColorShaderVars;
 
+	GLuint bayerPatternTex=0;
+    DitheringMode ditheringMode;
+    static DitheringMode parseDitheringMode(QString const& s);
 
 	//! The descriptor for the current opengl vertex array
 	ArrayDesc vertexArray;
@@ -425,5 +443,7 @@ private:
 	ArrayDesc colorArray;
 };
 
+Q_DECLARE_METATYPE(StelPainter::DitheringMode)
+
 #endif // _STELPAINTER_HPP_
 
diff --git a/src/core/modules/Atmosphere.cpp b/src/core/modules/Atmosphere.cpp
index 12953ed..eaff332 100644
--- a/src/core/modules/Atmosphere.cpp
+++ b/src/core/modules/Atmosphere.cpp
@@ -25,6 +25,7 @@
 #include "StelCore.hpp"
 #include "StelPainter.hpp"
 #include "StelFileMgr.hpp"
+#include "Dithering.hpp"
 
 #include <QDebug>
 #include <QSettings>
@@ -58,10 +59,11 @@ Atmosphere::Atmosphere(void)
 	}
 	QOpenGLShader fShader(QOpenGLShader::Fragment);
 	if (!fShader.compileSourceCode(
+                    makeDitheringShader()+
 					"varying mediump vec3 resultSkyColor;\n"
 					"void main()\n"
 					"{\n"
-					 "   gl_FragColor = vec4(resultSkyColor, 1.);\n"
+					 "   gl_FragColor = vec4(dither(resultSkyColor), 1.);\n"
 					 "}"))
 	{
 		qFatal("Error while compiling atmosphere fragment shader: %s", fShader.log().toLatin1().constData());
@@ -76,6 +78,8 @@ Atmosphere::Atmosphere(void)
 	StelPainter::linkProg(atmoShaderProgram, "atmosphere");
 
 	atmoShaderProgram->bind();
+	shaderAttribLocations.bayerPattern = atmoShaderProgram->uniformLocation("bayerPattern");
+	shaderAttribLocations.rgbMaxValue = atmoShaderProgram->uniformLocation("rgbMaxValue");
 	shaderAttribLocations.alphaWaOverAlphaDa = atmoShaderProgram->uniformLocation("alphaWaOverAlphaDa");
 	shaderAttribLocations.oneOverGamma = atmoShaderProgram->uniformLocation("oneOverGamma");
 	shaderAttribLocations.term2TimesOneOverMaxdLpOneOverGamma = atmoShaderProgram->uniformLocation("term2TimesOneOverMaxdLpOneOverGamma");
@@ -368,6 +372,15 @@ void Atmosphere::draw(StelCore* core)
 	const Mat4f& m = sPainter.getProjector()->getProjectionMatrix();
 	atmoShaderProgram->setUniformValue(shaderAttribLocations.projectionMatrix,
 		QMatrix4x4(m[0], m[4], m[8], m[12], m[1], m[5], m[9], m[13], m[2], m[6], m[10], m[14], m[3], m[7], m[11], m[15]));
+
+	const auto rgbMaxValue=calcRGBMaxValue(sPainter.getDitheringMode());
+	atmoShaderProgram->setUniformValue(shaderAttribLocations.rgbMaxValue, rgbMaxValue[0], rgbMaxValue[1], rgbMaxValue[2]);
+	auto& gl=*sPainter.glFuncs();
+	gl.glActiveTexture(GL_TEXTURE1);
+	if(!bayerPatternTex)
+		bayerPatternTex=makeBayerPatternTexture(*sPainter.glFuncs());
+	gl.glBindTexture(GL_TEXTURE_2D, bayerPatternTex);
+	atmoShaderProgram->setUniformValue(shaderAttribLocations.bayerPattern, 1);
 	
 	colorGridBuffer.bind();
 	atmoShaderProgram->setAttributeBuffer(shaderAttribLocations.skyColor, GL_FLOAT, 0, 4, 0);
diff --git a/src/core/modules/Atmosphere.hpp b/src/core/modules/Atmosphere.hpp
index 9553008..18e697c 100644
--- a/src/core/modules/Atmosphere.hpp
+++ b/src/core/modules/Atmosphere.hpp
@@ -103,6 +103,8 @@ private:
 	//! Vertex shader used for xyYToRGB computation
 	class QOpenGLShaderProgram* atmoShaderProgram;
 	struct {
+		int bayerPattern;
+		int rgbMaxValue;
 		int alphaWaOverAlphaDa;
 		int oneOverGamma;
 		int term2TimesOneOverMaxdLpOneOverGamma;
@@ -114,6 +116,8 @@ private:
 		int skyVertex;
 		int skyColor;
 	} shaderAttribLocations;
+
+	GLuint bayerPatternTex=0;
 };
 
 #endif // _ATMOSTPHERE_HPP_

@gzotti
Copy link
Member

gzotti commented Apr 9, 2018

Looks nice and clean, but for some reason I could not get the QVariant to read a useful settings value from config.ini, neither int nor names. (Maybe a stupid copying error on my side, though.) I turned the StelPainter into a Q_GADGET and added the Q_ENUM(DitheringMode), now it reads/writes the enum names, and I see differences (some small pattern in 565 mode). So, manual configuration is possible!

Is there anything you think is unfinished and you want to polish further, or else I would happily accept&commit this solution, it solves a long-standing wish (see LP bug/wishlist entry, I assigned its solving to you.) Our GUI needs a bit attention, I would do that in the next few days.

@10110111
Copy link
Contributor Author

10110111 commented Apr 9, 2018

for some reason I could not get the QVariant to read a useful settings value from config.ini, neither int nor names

Yeah, that's QTBUG-53384. See the latest edit of my previous comment, it has another approach with QVariant::toString, followed by manual parsing (and then the config values will have to be the same as enumerator names instead of numbers).

Is there anything you think is unfinished and you want to polish further, or else I would happily accept&commit this solution

If the latest version of the diff above is acceptable, I'll put it into a pull request, so that you can merge it.

@alex-w alex-w added this to the 0.18.1 milestone Apr 15, 2018
@gzotti
Copy link
Member

gzotti commented Apr 22, 2018

Alright @10110111 , I can continue today.

Results with dither 5/6/5 from Qt5.9/Windows, Driver version string: "OpenGL ES 2.0 (ANGLE 2.1.0.8613f4946861)"
return of vec3(bayer) is black, nothing. Of course, also toColor(bayer).rgb.
Running with OpenGL looks more meaningful, white (or bluish for toColor()) pattern.

I just ran the vec3(bayer) on a Raspberry Pi3 with VC4 driver (Mesa 17).
Driver version string "OpenGL ES 2.0 Mesa 17.4.0 devel (git-7983adc60f)".
Also here I have a black screen. I will try 2 other PCs with (hopefully) different Qt /ANGLE versions.

To make sure about mediump and highp, my Dithering.hpp is (sorry for the formatting, why is the start part wrong?):

inline QString makeDitheringShader()
{
return
R"(
const int emax=127;
// Input: x>=0
// Output: base 2 exponent of x if (x!=0 && !isnan(x) && !isinf(x))
// -emax if x==0
// emax+1 otherwise
int floorLog2(highp float x)
{
if(x==0.) return -emax;
// NOTE: there exist values of x, for which floor(log2(x)) will give wrong
// (off by one) result as compared to the one calculated with infinite precision.
// Thus we do it in a brute-force way.
for(int e=emax;e>=1-emax;--e)
if(x>=exp2(float(e))) return e;
// If we are here, x must be infinity or NaN
return emax+1;
}
// Input: any x
// Output: IEEE 754 biased exponent with bias=emax
int biasedExp(highp float x) { return emax+floorLog2(abs(x)); }

		// Input: any x such that (!isnan(x) && !isinf(x))
		// Output: significand AKA mantissa of x if !isnan(x) && !isinf(x)
		//         undefined otherwise
		highp float significand(highp float x)
		{
		    // converting int to float so that exp2(genType) gets correctly-typed value
		    highp float expo=float(floorLog2(abs(x)));
		    return abs(x)/exp2(expo);
		}

		// Input: x\in[0,1)
		//        N>=0
		// Output: Nth byte as counted from the highest byte in the fraction
		int part(highp float x,int N)
		{
		    // All comments about exactness here assume that underflow and overflow don't occur
		    const highp float byteShift=256.;
		    // Multiplication is exact since it's just an increase of exponent by 8 == LINE 40
		    //for(int n=0;n<N;++n)
			//x*=byteShift;
		for(int n=0;n<3;++n)
		    if(n<N)
			x*=byteShift;

		    // Cut higher bits away.
		    // $q \in [0,1) \cap \mathbb Q'.$
		    highp float q=fract(x);

		    // Shift and cut lower bits away. Cutting lower bits prevents potentially unexpected
		    // results of rounding by the GPU later in the pipeline when transforming to TrueColor
		    // the resulting subpixel value. 
		    // $c \in [0,255] \cap \mathbb Z.$
		    // Multiplication is exact since it's just and increase of exponent by 8
		    highp float c=floor(byteShift*q);
		    return int(c);
		}

		// Input: any x acceptable to significand()
		// Output: significand of x split to (8,8,8)-bit data vector  
		ivec3 significandAsIVec3(highp float x)
		{
		    ivec3 result;
		    highp float sig=significand(x)/2.; // shift all bits to fractional part
		    result.x=part(sig,0);
		    result.y=part(sig,1);
		    result.z=part(sig,2);
		    return result;
		}

		// Input: any x such that !isnan(x)
		// Output: IEEE 754 defined binary32 number, packed as ivec4(byte3,byte2,byte1,byte0) 
		ivec4 packIEEE754binary32(highp float x)
		{
		    int e = biasedExp(x);
		    // sign to bit 7
		    int s = x<0. ? 128 : 0;

		    ivec4 binary32;
		    binary32.yzw=significandAsIVec3(x);
		    // clear the implicit integer bit of significand
		    if(binary32.y>=128) binary32.y-=128;
		    // put lowest bit of exponent into its position, replacing just cleared integer bit
		    binary32.y+=128*int(mod(float(e),2.));
		    // prepare high bits of exponent for fitting into their positions
		    e/=2;
		    // pack highest byte
		    binary32.x=e+s;

		    return binary32;
		}

		highp vec4 toColor(highp float x)
		{
		    ivec4 binary32=packIEEE754binary32(x);
		    // Transform color components to [0,1] range.
		    // Division is inexact, but works reliably for all integers from 0 to 255 if
		    // the transformation to TrueColor by GPU uses rounding to nearest or upwards.
		    // The result will be multiplied by 255 back when transformed
		    // to TrueColor subpixel value by OpenGL.
		    return vec4(binary32)/255.;
		}

		uniform mediump vec3 rgbMaxValue;
		uniform sampler2D bayerPattern;
		highp  vec3 dither(highp  vec3 c)
		{
		if(rgbMaxValue.r==0.) return c;
		highp float bayer=texture2D(bayerPattern,gl_FragCoord.xy/8.).r;

		//return vec3(bayer);
		return toColor(bayer).rgb;
		//highp vec3 rgb=c*rgbMaxValue;
		//return rgb;
		//highp vec3 head=floor(rgb);
		//return head;
		//highp vec3 tail=rgb-head;
		//return tail;
		//return (head+1.-step(tail,vec3(bayer)))/rgbMaxValue;
		}
		highp  vec4 dither(highp  vec4 c) { return vec4(dither(c.xyz),c.w); }
		)";

}

In the last block of course I return what's needed.

@gzotti
Copy link
Member

gzotti commented Apr 22, 2018

Interestingly, on the RasPi3, the final image seems OK. No brightened texture squares:

rpi_dith565

(Image made with scrot, currently we have some WIP issues around built-in screenshots...)

@10110111
Copy link
Contributor Author

10110111 commented Apr 22, 2018

sorry for the formatting, why is the start part wrong?

The code here is formatted either when each line is prepended with four spaces like

    someCode()

or when the snipped is enclosed in triple backticks ```, like

```
someCode();
```

You can edit your post to fix it.

@10110111
Copy link
Contributor Author

I just ran the vec3(bayer) on a Raspberry Pi3 with VC4 driver (Mesa 17).
Driver version string "OpenGL ES 2.0 Mesa 17.4.0 devel (git-7983adc60f)".
Also here I have a black screen.

What if you set LIBGL_ALWAYS_SOFTWARE=1 environment variable before running the test?

@10110111
Copy link
Contributor Author

10110111 commented Apr 22, 2018

Interestingly, on the RasPi3, the final image seems OK. No brightened texture squares:

But no dithering either: see the flat magenta-ish and greenish bands e.g. near the head of the fish. This is also confirmed by the fact that Bayer texture appears black.

@gzotti
Copy link
Member

gzotti commented Apr 22, 2018

4 spaces, OK. I tried two backticks or up to 3 spaces... Still new here.

The shader does "something" on RasPi3 with the various modes, but no useful pattern. Here are 666, 888 and off:
rpi_dith666
rpi_dith888
rpi_dithnone
888 is pretty good, the others show banding. Testing with that envvar next...

@10110111
Copy link
Contributor Author

One more thing to try: in makeBayerPatternTexture, replace GL_RED with GL_LUMINANCE in both 4th and 7th arguments of glTexImage2D call. According to the ES 2.0 docs, GL_RED may not be acceptable in OpenGL ES 2.0.

@gzotti
Copy link
Member

gzotti commented Apr 22, 2018

Sigh, OpenGL things are still not too flexible:

QXcbIntegration: Cannot create platform OpenGL context, neither GLX nor EGL are enabled

@10110111
Copy link
Contributor Author

QXcbIntegration: Cannot create platform OpenGL context, neither GLX nor EGL are enabled

On which system/config did you get this? I remember getting it when Qt simply wasn't able to link to libEGL.so.

@gzotti
Copy link
Member

gzotti commented Apr 22, 2018

System as described in our Wiki, https://github.com/Stellarium/stellarium/wiki/Raspberry-Pi

Driver version string: "OpenGL ES 2.0 Mesa 17.4.0-devel (git-7983adc60f)"
GL vendor is "Broadcom"
GL renderer is "VC4 V3D 2.1"
GL Shading Language version is "OpenGL ES GLSL ES 1.0.16"
MESA Version Number detected:  17.4

In any case, software OpenGL will have again 0.05 fps, so I don't think it's worth to start fixing that.
Tried with GL_LUMINANCE, again vec3(bayer) is black.

I will now turn back to my other Windows systems with (hopefully) non Qt5.9, i.e. different ANGLE.

@gzotti
Copy link
Member

gzotti commented Apr 22, 2018

Stupid, wih Qt5.10 we have #139.

@10110111
Copy link
Contributor Author

One more thing to try on Mesa: set MESA_DEBUG=1 environment variable and look for any error messages emitted by Mesa in the console.

@gzotti
Copy link
Member

gzotti commented Apr 22, 2018

I now tried with my netbook. AMD A4-1200 Radeon HD 8180, Qt5.9, Win10. This works reasonably well in OpenGL (4.5!), is unusable (GUI panels missing??) with ANGLE/DirectX9, but works well again with ANGLE/DirectX11. No texture squares, and patterned dither 565&666, smooth 888.
Maybe it's the ANGLE D3D9 that is broken for Geforce and AMD GPUs when they can run OpenGL and D3D11? Need to test 2 more...

@gzotti
Copy link
Member

gzotti commented Apr 22, 2018

OK, we have for the "return toColor(bayer).rgb":

Detected: OpenGL ES "2.0"
Driver version string: "OpenGL ES 2.0 (ANGLE 2.1.0.8613f4946861)"
GL vendor is "Google Inc."
GL renderer is "ANGLE (NVIDIA GeForce GTX 960M Direct3D9Ex vs_3_0 ps_3_0)"
GL Shading Language version is "OpenGL ES GLSL ES 1.00 (ANGLE 2.1.0.8613f4946861)"
VS Version Number detected:  3
PS Version Number detected:  3

Black screen.

And

Detected: OpenGL ES "2.0"
Driver version string: "OpenGL ES 2.0 (ANGLE 2.1.0.8613f4946861)"
GL vendor is "Google Inc."
GL renderer is "ANGLE (NVIDIA GeForce GTX 960M Direct3D11 vs_5_0 ps_5_0)"
GL Shading Language version is "OpenGL ES GLSL ES 1.00 (ANGLE 2.1.0.8613f4946861)"
VS Version Number detected:  5
PS Version Number detected:  5

d3d11dith565-tocolor bayer rgb

compare with OpenGL:

Detected: OpenGL "4.5"
Driver version string: "4.5.0 NVIDIA 385.41"
GL vendor is "NVIDIA Corporation"
GL renderer is "GeForce GTX 960M/PCIe/SSE2"
GL Shading Language version is "4.50 NVIDIA"
GLSL Version Number detected:  4.5

ogldith565-tocolor bayer rgb

(Don't bother the high resolution with GUI bar inside, I am playing with #102 as well.)
Changing GL_RED to GL_LUMINANCE does not change the result. It's blue on OpenGL but green with D3D11.

I deactivated all the preliminary returns now, and ... errm.. whatever happened in the past 10 days, the colored texture boxes also have vanished in D3D9 mode. Maybe there was one mediump earlier which I then changed to highp, and now it works? But the black screen still seems wrong. And 888 dither pattern looks worse than undithered, at least for the constellation artwork. With Zodiacal light, on my notebook I see soft colored stripes. 666 is overall better.

This is turning crazy. Now I have an Intel HD4700 to test...

@gzotti
Copy link
Member

gzotti commented Apr 22, 2018

OK, it's an Intel 4600, Qt5.9 like on my Win10 notebooks:
OpenGL (4.3): Works.
ANGLE/DirectX9: Terrible banding when set to 565
ANGLE/DirectX11: Works indistinguishable from OpenGL.

Maybe we should just conclude "Dithering may not work well on Angle/DirectX9" (and again, "update your drivers"...) After all, switching off looks like before.

@gzotti
Copy link
Member

gzotti commented Apr 22, 2018

Thanks for fixing #139. On that Win7 notebook (Geforce580, Qt5.10) I now have similar results now like with the Intel GPU: OpenGL 4.5 and DX11 are fine, DX9 shows ugliest banding. No texture squares, though.

I think we can stop here and better turn to other issues. Objections?

@10110111
Copy link
Contributor Author

If you can provide a Windows binary of current Stellarium git, I might try to find time to see what happens myself. Not sure though how you differentiate Qt versions – are they included with Stellarium? Or do you somehow have them installed system-wide?

In any case, if this works in normal cases (i.e. default mode, when the users don't turn on any workarounds which ANGLE seems to be), I'm fine with closing this issue.

@gzotti
Copy link
Member

gzotti commented Apr 22, 2018

Alex makes weekly betas, available from Launchpad: https://launchpad.net/stellarium
The Qt libraries are packed in the program directory.

The whole ANGLE business is a fallback for unfortunately far too many "average" users who are not aware of the fact that their GPUs need current drivers from other sources than the daily Windows update. It feels like about 40% support requests still relate to that nonsense, and we keep repeating ourselves in the Release FAQs. Some GPUs are meanwhile blacklisted by Qt because their OpenGL drivers are bad (e.g. early Intel HD-2000/3000 always start ANGLE mode, and probably even earlier Intels always would use MESA (software GL), although I don't have access to such old systems now). For others, users should explicitly launch ANGLE mode with the provided link, or ...<prayer_mill> update their drivers</prayer_mill>.

OK, we may close here, and if you are able to detect what's wrong (some rounding/ULP issue, most likely), please just call again!

@gzotti gzotti closed this as completed Apr 22, 2018
@gzotti
Copy link
Member

gzotti commented May 13, 2018

@10110111 unfortunately it seems this feature caused a bad memory leak! @guillaumechereau found:

OK, I did a long run with valgrind massif, and it seems the leak might
come from the call to 'makeBayerPatternTexture' in
StelPainter::drawFromArray.

Also, dithering on Odroid XU3 and C1 SBCs (OpenGL ES2) does not really work, has similar increased banding as DX9 ANGLE.

@gzotti gzotti reopened this May 13, 2018
@10110111
Copy link
Contributor Author

I suppose it's textures which leak. I supposed StelPainter only dies when GL context is destroyed. But it appears to be created and destroyed at every frame. I'll make a pull request to fix this one.

Also, dithering on Odroid XU3 and C1 SBCs (OpenGL ES2) does not really work, has similar increased banding as DX9 ANGLE.

Issue #131 has grown to a hard-to-manage length. Would it be better to track new problems as new issues? (You can always ping me there via the @ syntax.)

@gzotti
Copy link
Member

gzotti commented May 13, 2018

Agreed. Thanks for the fast fix. And I may open a new issue about the GLES dithering stuff.

@gzotti gzotti closed this as completed May 13, 2018
@Stellarium Stellarium deleted a comment from caioaugusto1998 Dec 6, 2019
@Stellarium Stellarium deleted a comment from caioaugusto1998 Dec 6, 2019
@danifernandez5
Copy link

danifernandez5 commented Apr 16, 2020

hi, apologies for reviving this. Im with a 6bpp monitor and the band problem happens to me. Is very annoying. How can I easily solve? Im using the latest beta version and it appears in all options (table mode, angle mode, etc). gpu amd radeon rx vega 10; amd ryzen 7 3700u cpu, drivers 20.4.1; windows 10 64 bit.

@10110111
Copy link
Contributor Author

@danifernandez5
By default dithering is disabled, so it's expected that you see banding. To get rid of banding, knowing that you have a 6bpp monitor, go to Configuration (F2) → ToolsDithering. Choose 6/6/6 bits in this drop-down menu.

@danifernandez5
Copy link

Thank you very much! It worked perfect on 8/8/8. It's weird because my operating system says the color depth is 6 bit. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality
Development

Successfully merging a pull request may close this issue.

5 participants