Code review fixes, remove implicit GLSL casts

apps/openmw/mwrender/renderingmanager.cpp

@@ -1171,8 +1171,8 @@ namespace MWRender
                     lightManager->updateMaxLights();
 
                     auto defines = mResourceSystem->getSceneManager()->getShaderManager().getGlobalDefines();
-                    for (auto& define : lightManager->getLightDefines())
+                    for (const auto& [name, key] : lightManager->getLightDefines())
-                        defines[define.first] = define.second;
+                        defines[name] = key;
                     mResourceSystem->getSceneManager()->getShaderManager().setGlobalDefines(defines);
 
                     mSceneRoot->removeUpdateCallback(mStateUpdater);

components/sceneutil/lightmanager.cpp

@@ -184,8 +184,8 @@ namespace SceneUtil
 
         void configureLayout(int offsetColors, int offsetPosition, int offsetAttenuationRadius, int size, int stride)
         {
-            constexpr auto sizeofVec4 = sizeof(GL_FLOAT) * osg::Vec4::num_components;
             constexpr auto sizeofFloat = sizeof(GL_FLOAT);
+            constexpr auto sizeofVec4 = sizeofFloat * osg::Vec4::num_components;
 
             mOffsets[Diffuse] = offsetColors / sizeofFloat;
             mOffsets[Ambient] = mOffsets[Diffuse] + 1;
@@ -197,11 +197,12 @@ namespace SceneUtil
 
             // Copy over previous buffers light data. Buffers populate before we know the layout.
             LightBuffer oldBuffer = LightBuffer(*this);
+            mData->resize(size / sizeofFloat);
             for (int i = 0; i < oldBuffer.mCount; ++i)
             {
-                std::memcpy(&(*mData)[getOffset(i, Diffuse)], &(*mData)[oldBuffer.getOffset(i, Diffuse)], sizeof(osg::Vec4f));
+                std::memcpy(&(*mData)[getOffset(i, Diffuse)], &(*oldBuffer.mData)[oldBuffer.getOffset(i, Diffuse)], sizeof(osg::Vec4f));
-                std::memcpy(&(*mData)[getOffset(i, Position)], &(*mData)[oldBuffer.getOffset(i, Position)], sizeof(osg::Vec4f));
+                std::memcpy(&(*mData)[getOffset(i, Position)], &(*oldBuffer.mData)[oldBuffer.getOffset(i, Position)], sizeof(osg::Vec4f));
-                std::memcpy(&(*mData)[getOffset(i, AttenuationRadius)], &(*mData)[oldBuffer.getOffset(i, AttenuationRadius)], sizeof(osg::Vec4f));
+                std::memcpy(&(*mData)[getOffset(i, AttenuationRadius)], &(*oldBuffer.mData)[oldBuffer.getOffset(i, AttenuationRadius)], sizeof(osg::Vec4f));
             }
         }
 
@@ -766,17 +767,27 @@ namespace SceneUtil
 
         std::string generateDummyShader(int maxLightsInScene)
         {
-            return "#version 120\n"
+            const std::string define = "@maxLightsInScene";
-            "#extension GL_ARB_uniform_buffer_object : require                                      \n"
+
-            "struct LightData {                                                                     \n"
+            std::string shader = R"GLSL(
-            "   ivec4 packedColors;                                                                 \n"
+                #version 120
-            "   vec4 position;                                                                      \n"
+                #extension GL_ARB_uniform_buffer_object : require
-            "   vec4 attenuation;                                                                   \n"
+                struct LightData {
-            "};                                                                                     \n"
+                   ivec4 packedColors;
-            "uniform LightBufferBinding {                                                           \n"
+                   vec4 position;
-            "   LightData LightBuffer[" + std::to_string(mLightManager->getMaxLightsInScene()) + "];\n"
+                   vec4 attenuation;
-            "};                                                                                     \n"
+                };
-            "void main() { gl_Position = vec4(0.0); }                                               \n";
+                uniform LightBufferBinding {
+                   LightData LightBuffer[@maxLightsInScene];
+                };
+                void main()
+                {
+                    gl_Position = vec4(0.0);
+                }
+            )GLSL";
+
+            shader.replace(shader.find(define), define.length(), std::to_string(maxLightsInScene));
+            return shader;
         }
 
         LightManager* mLightManager;

docs/source/reference/modding/settings/shaders.rst

@@ -174,7 +174,7 @@ this mode when supported and where the GPU is not a bottleneck. On some weaker
 devices, using this mode along with :ref:`force per pixel lighting` can carry
 performance penalties.
 
-Note that when enabled groundcover lighting is forced to be vertex lighting,
+Note that when enabled, groundcover lighting is forced to be vertex lighting,
 unless normal maps are provided. This is due to some groundcover mods using the
 Z-Up normals technique to avoid some common issues with shading. As a
 consequence, per pixel lighting would give undesirable results.

files/settings-default.cfg

@@ -447,7 +447,7 @@ radial fog = false
 # uncaps the light limit, enables groundcover lighting, and uses a modified
 # attenuation formula to reduce popping and light seams. "shaders" comes with
 # all these benefits and is meant for larger light limits, but may not be
-# supported on older hardware and may be less performant on weaker hardware when
+# supported on older hardware and may be slower on weaker hardware when
 # 'force per pixel lighting' is enabled.
 lighting method = shaders compatibility
 

files/shaders/lighting.glsl

@@ -72,7 +72,7 @@ void doLighting(vec3 viewPos, vec3 viewNormal, out vec3 diffuseLight, out vec3 a
     diffuseLight = diffuseOut * shadowing;
 #else
     shadowDiffuse = diffuseOut;
-    diffuseLight = vec3(0);
+    diffuseLight = vec3(0.0);
 #endif
 
     for (int i = @startLight; i < @endLight; ++i)