[PATCH] Disable Fog option

[elexis]

As proposed by cc, there are three reasons why someone would want to disable fog (the graphics effect, not the fog of war):

• For playing, the fog is often obstructing the view, even if it's visually appealing.
• For promotional screenshots we want to keep shaders enabled (especially for shadows) and zoom out very far without getting everything fogged.
• On many maps there is too much fog (they should be fixed some day).

This example shows that we can change the setting at runtime:

Index: source/renderer/RenderModifiers.cpp
===================================================================
--- source/renderer/RenderModifiers.cpp	(revision 19759)
+++ source/renderer/RenderModifiers.cpp	(working copy)
@@ -92,7 +92,6 @@
 		shader->Uniform(str_sunColor, GetLightEnv()->m_SunColor);
 
 		shader->Uniform(str_fogColor, GetLightEnv()->m_FogColor);
-		shader->Uniform(str_fogParams, GetLightEnv()->m_FogFactor, GetLightEnv()->m_FogMax, 0.f, 0.f);
 	}
 
 	if (shader->GetTextureBinding(str_losTex).Active())
Index: source/renderer/TerrainRenderer.cpp
===================================================================
--- source/renderer/TerrainRenderer.cpp	(revision 19759)
+++ source/renderer/TerrainRenderer.cpp	(working copy)
@@ -473,7 +473,6 @@
 	shader->Uniform(str_sunDir, lightEnv.GetSunDir());
 
 	shader->Uniform(str_fogColor, lightEnv.m_FogColor);
-	shader->Uniform(str_fogParams, lightEnv.m_FogFactor, lightEnv.m_FogMax, 0.f, 0.f);
 }
 
 void TerrainRenderer::RenderTerrainShader(const CShaderDefines& context, int cullGroup, ShadowMap* shadow)
@@ -788,7 +787,6 @@
 	m->fancyWaterShader->Uniform(str_losMatrix, losTexture.GetTextureMatrix());
 	m->fancyWaterShader->Uniform(str_cameraPos, camPos);
 	m->fancyWaterShader->Uniform(str_fogColor, lightEnv.m_FogColor);
-	m->fancyWaterShader->Uniform(str_fogParams, lightEnv.m_FogFactor, lightEnv.m_FogMax, 0.f, 0.f);
 	m->fancyWaterShader->Uniform(str_time, (float)time);
 	m->fancyWaterShader->Uniform(str_screenSize, (float)g_Renderer.GetWidth(), (float)g_Renderer.GetHeight(), 0.0f, 0.0f);
 

The setting should depend on GLSL, just like the water settings depend on the non-ugly water setting in gui/options/options.json.

[dp304]

Please review the above patch. It does not improve performance though. For that, probably some patch in the shader is needed as well.

[elexis]

Patch looks good. The only thing which could be questioned in the patch is whether setting the fog factor to 0 is sufficient or if more than that should be disabled (probably not an issue).

Since we're a bit overloaded with reviews, you might want to upload to ​http://code.wildfiregames.com/ in case we didn't get to this in time (there is a bit more activity over there).

Furthermore your nickname will be added to the credits file programming.json if the patch is accepted. If you want your full name added there too, please post it or include the diff to the credit file in the patch.

[elexis]

(actually noone looks at the wip milestone)

[dp304]

Thank you! I've attempted to make an alternative patch. The options integration is the same, but fog generation is now disabled in the shaders, based on how it is done for e.g. shadows. This should improve performance when fog is off. However, it doesn't seem to work well for water (and maybe neither for particles etc.?). When I turn on fog, water is not affected. When I restart the game with fog on, it is fine, but when I turn off fog, water remains foggy. (Apparently CRenderer::ReloadShaders() doesn't reload that particular shader)

[dp304]

This new patch fixes the issue with water not responding to the fog setting. It also seems to fix another unrelated bug where water does not update when turning off shadows (but not "shadows on water"). In addition, I made the fog setting dependent on "prefer GLSL" as requested in the ticket, and added a default value (true).

[elexis]

That is nice to do it right in the shader and get the performance benefit.

Not a dealbreaker, but at least the water shader might be more maintainable and extensible if the formula using the fog isn't duplicated.

Could be achieved by adding the directive inside of the get_fog functions instead of the calls or by storing the (optionally fogged) color in a helper variable before using it. (One should expect that the compiler is smart enough to remove the helper variable or empty functions)

Also binaries/data/mods/public/gui/credits/texts/programming.json is the path to said credits file if you want your realname added besides your nick.

The MakeShadersDirty change will be committed separately for auditability I guess.

Also ​http://code.wildfiregames.com/ is our review platform where one can see what has changed in different versions of patches, do formal approval or request of changes, inline comments, in case you want to upload more patches in the future or help reviewing other peoples patches :-)

[elexis]

In 20583:

Reload water shaders too when changing shader settings.

Refs #4626
Patch By: dp304

[elexis]

In 20584:

Option to disable fog effects.

Fixes #4626
Patch By: dp304

[elexis]

Thanks for the patch, that will make a lot of people enable GLSL!

I have added you to the credits file and removed the duplication in the water_high.fs shader as mentioned.

[Itms]

Hello and thanks for the patches dp304!

I really urge you to propose your patches on Phabricator next time. On top of giving you a higher chance of seeing your patch reviewed (elexis put some extra effort here because this is a great functionality), patches uploaded there go through automated testing, which ensures we spend as little time as possible fixing some mistakes that could have been avoided.

Looking forward to your future contributions! :)

[dp304]

My pleasure, and thank you for the credits!

Next time if I make another patch, I'll of course use Phabricator, sorry for the extra work caused.