Shaders: alpha doesn't make sense for light colors.

I basically deprecated half of the Phong shader API. Sigh. Enjoy
updating your code.

doc/changelog.dox

@@ -189,6 +189,13 @@ See also:
 -   @cpp Shaders::Phong::setLightPosition(const Vector3&) @ce is deprecated in
     favor of @ref Shaders::Phong::setLightPositions() with a single item ---
     it's short enough to not warrant the existence of a dedicated overload
+-   @ref Shaders::Phong::setLightColors() and
+    @ref Shaders::Phong::setLightColor() taking four-component colors are
+    deprecated in favor of variants taking just three-component colors, as the
+    alpha had no meaningful use anyway.
+-   @cpp Shaders::Phong::setLightColor(const Magnum::Color4&) @ce is deprecated
+    in favor of @ref Shaders::Phong::setLightColors() with a single item ---
+    it's short enough to not warrant the existence of a dedicated overload
 -   @cpp Trade::AbstractMaterialData @ce as well as its containing header
     `Magnum/Trade/AbstractMaterialData.h` is now a deprecated alias to the new
     and more flexible @ref Trade::MaterialData.

doc/snippets/MagnumSceneGraph-gl.cpp

@@ -184,7 +184,7 @@ struct MyApplication: Platform::Application {
     SceneGraph::Object<SceneGraph::MatrixTransformation2D>* _cameraObject;
     SceneGraph::Camera3D* _camera;
     Vector4 _lightPositionRelativeToCamera;
-    Vector3 _lightColor, _ambientColor;
+    Color3 _lightColor, _ambientColor;
 /* [Drawable-multiple-groups] */
     // ...
 

src/Magnum/Shaders/Phong.cpp

@@ -87,7 +87,7 @@ Phong::Phong(const Flags flags, const UnsignedInt lightCount): _flags{flags}, _l
         constexpr Containers::StringView lightColorInitializerPreamble = "#define LIGHT_COLOR_INITIALIZER "_s;
         constexpr Containers::StringView lightRangeInitializerPreamble = "#define LIGHT_RANGE_INITIALIZER "_s;
         constexpr Containers::StringView lightPositionInitializerItem = "vec4(0.0, 0.0, 1.0, 0.0), "_s;
-        constexpr Containers::StringView lightColorInitializerItem = "vec4(1.0), "_s;
+        constexpr Containers::StringView lightColorInitializerItem = "vec3(1.0), "_s;
         constexpr Containers::StringView lightRangeInitializerItem = "1.0/0.0, "_s;
 
         lightInitializerVertex.reserve(
@@ -258,7 +258,7 @@ Phong::Phong(const Flags flags, const UnsignedInt lightCount): _flags{flags}, _l
         if(flags & Flag::NormalTexture)
             setNormalTextureScale(1.0f);
         setLightPositions(Containers::Array<Vector4>{Containers::DirectInit, lightCount, Vector4{0.0f, 0.0f, 1.0f, 0.0f}});
-        setLightColors(Containers::Array<Magnum::Color4>{Containers::DirectInit, lightCount, Magnum::Color4{1.0f}});
+        setLightColors(Containers::Array<Magnum::Color4>{Containers::DirectInit, lightCount, Magnum::Color3{1.0f}});
         setLightRanges(Containers::Array<Float>{Containers::DirectInit, lightCount, Constants::inf()});
         /* Light position is zero by default */
         setNormalMatrix({});
@@ -416,26 +416,51 @@ Phong& Phong::setLightPosition(const Vector3& position) {
 }
 #endif
 
-Phong& Phong::setLightColors(const Containers::ArrayView<const Magnum::Color4> colors) {
+Phong& Phong::setLightColors(const Containers::ArrayView<const Magnum::Color3> colors) {
     CORRADE_ASSERT(_lightCount == colors.size(),
         "Shaders::Phong::setLightColors(): expected" << _lightCount << "items but got" << colors.size(), *this);
     if(_lightCount) setUniform(_lightColorsUniform, colors);
     return *this;
 }
 
-/* It's light, but can't be in the header because MSVC needs to know the size
-   of Color for the initializer list use */
+#ifdef MAGNUM_BUILD_DEPRECATED
+Phong& Phong::setLightColors(const Containers::ArrayView<const Magnum::Color4> colors) {
+    Containers::Array<Magnum::Color3> threeComponent{Containers::NoInit, colors.size()};
+    for(std::size_t i = 0; i != colors.size(); ++i)
+        threeComponent[i] = colors[i].rgb();
+    setLightColors(threeComponent);
+    return *this;
+}
+
 Phong& Phong::setLightColors(const std::initializer_list<Magnum::Color4> colors) {
+    CORRADE_IGNORE_DEPRECATED_PUSH
+    return setLightColors(Containers::arrayView(colors));
+    CORRADE_IGNORE_DEPRECATED_POP
+}
+#endif
+
+Phong& Phong::setLightColors(const std::initializer_list<Magnum::Color3> colors) {
     return setLightColors(Containers::arrayView(colors));
 }
 
-Phong& Phong::setLightColor(const UnsignedInt id, const Magnum::Color4& color) {
+Phong& Phong::setLightColor(const UnsignedInt id, const Magnum::Color3& color) {
     CORRADE_ASSERT(id < _lightCount,
         "Shaders::Phong::setLightColor(): light ID" << id << "is out of bounds for" << _lightCount << "lights", *this);
     setUniform(_lightColorsUniform + id, color);
     return *this;
 }
 
+#ifdef MAGNUM_BUILD_DEPRECATED
+Phong& Phong::setLightColor(UnsignedInt id, const Magnum::Color4& color) {
+    return setLightColor(id, color.rgb());
+}
+
+Phong& Phong::setLightColor(const Magnum::Color4& color) {
+    /* Use the list variant to check the shader really has just one light */
+    return setLightColors({color.rgb()});
+}
+#endif
+
 Phong& Phong::setLightRanges(const Containers::ArrayView<const Float> ranges) {
     CORRADE_ASSERT(_lightCount == ranges.size(),
         "Shaders::Phong::setLightRanges(): expected" << _lightCount << "items but got" << ranges.size(), *this);

src/Magnum/Shaders/Phong.frag

@@ -144,9 +144,9 @@ uniform highp uint objectId; /* defaults to zero */
 #ifdef EXPLICIT_UNIFORM_LOCATION
 layout(location = LIGHT_COLORS_LOCATION) /* I fear this will blow up some drivers */
 #endif
-uniform lowp vec4 lightColors[LIGHT_COUNT]
+uniform lowp vec3 lightColors[LIGHT_COUNT]
     #ifndef GL_ES
-    = vec4[](LIGHT_COLOR_INITIALIZER)
+    = vec3[](LIGHT_COLOR_INITIALIZER)
     #endif
     ;
 
@@ -265,7 +265,7 @@ void main() {
 
         highp vec3 normalizedLightDirection = normalize(lightDirections[i].xyz);
         lowp float intensity = max(0.0, dot(normalizedTransformedNormal, normalizedLightDirection))*attenuation;
-        fragmentColor += vec4(finalDiffuseColor.rgb*lightColors[i].rgb*intensity, lightColors[i].a*finalDiffuseColor.a/float(LIGHT_COUNT));
+        fragmentColor += vec4(finalDiffuseColor.rgb*lightColors[i]*intensity, finalDiffuseColor.a/float(LIGHT_COUNT));
 
         /* Add specular color, if needed */
         if(intensity > 0.001) {

src/Magnum/Shaders/Phong.h

@@ -880,36 +880,65 @@ class MAGNUM_SHADERS_EXPORT Phong: public GL::AbstractShaderProgram {
         /**
          * @brief Set light colors
          * @return Reference to self (for method chaining)
+         * @m_since_latest
          *
-         * Initial values are @cpp 0xffffffff_rgbaf @ce. Expects that the size
+         * Initial values are @cpp 0xffffff_rgbf @ce. Expects that the size
          * of the @p colors array is the same as @ref lightCount().
+         * @see @ref Shaders-Phong-lights, @ref setLightColor()
          */
-        Phong& setLightColors(Containers::ArrayView<const Magnum::Color4> colors);
+        Phong& setLightColors(Containers::ArrayView<const Magnum::Color3> colors);
 
-        /** @overload */
-        Phong& setLightColors(std::initializer_list<Magnum::Color4> colors);
+        /**
+         * @overload
+         * @m_since_latest
+         */
+        Phong& setLightColors(std::initializer_list<Magnum::Color3> colors);
+
+        #ifdef MAGNUM_BUILD_DEPRECATED
+        /**
+         * @brief @copybrief setLightColors(Containers::ArrayView<const Magnum::Color3>)
+         * @m_deprecated_since_latest Use @ref setLightColors(Containers::ArrayView<const Magnum::Color3>)
+         *      instead. The alpha channel isn't used in any way.
+         */
+        CORRADE_DEPRECATED("use setLightColors(Containers::ArrayView<const Magnum::Color3>) instead") Phong& setLightColors(Containers::ArrayView<const Magnum::Color4> colors);
+
+        /**
+         * @brief @copybrief setLightColors(std::initializer_list<Magnum::Color3>)
+         * @m_deprecated_since_latest Use @ref setLightColors(std::initializer_list<Magnum::Color3>)
+         *      instead. The alpha channel isn't used in any way.
+         */
+        CORRADE_DEPRECATED("use setLightColors(std::initializer_list<Magnum::Color3>) instead") Phong& setLightColors(std::initializer_list<Magnum::Color4> colors);
+        #endif
 
         /**
          * @brief Set position for given light
          * @return Reference to self (for method chaining)
+         * @m_since_latest
          *
-         * Unlike @ref setLightColors() updates just a single light color.
-         * Expects that @p id is less than @ref lightCount().
-         * @see @ref setLightColor(const Magnum::Color4&)
+         * Unlike @ref setLightColors() updates just a single light color. If
+         * updating more than one light, prefer the batch function instead to
+         * reduce the count of GL API calls. Expects that @p id is less than
+         * @ref lightCount().
+         */
+        Phong& setLightColor(UnsignedInt id, const Magnum::Color3& color);
+
+        #ifdef MAGNUM_BUILD_DEPRECATED
+        /**
+         * @brief @copybrief setLightColor(UnsignedInt, const Magnum::Color3&)
+         * @m_deprecated_since_latest Use @ref setLightColor(UnsignedInt, const Magnum::Color3&)
+         *      instead. The alpha channel isn't used in any way.
          */
-        Phong& setLightColor(UnsignedInt id, const Magnum::Color4& color);
+        CORRADE_DEPRECATED("use setLightColor(UnsignedInt, const Magnum::Color3&) instead") Phong& setLightColor(UnsignedInt id, const Magnum::Color4& color);
 
         /**
          * @brief Set light color
-         * @return Reference to self (for method chaining)
-         *
-         * Convenience alternative to @ref setLightColors() when there is just
-         * one light.
-         * @see @ref setLightColor(UnsignedInt, const Magnum::Color4&)
+         * @m_deprecated_since_latest Use @ref setLightColors(std::initializer_list<Magnum::Color3>)
+         *      with a single item instead --- it's short enough to not warrant
+         *      the existence of a dedicated overload. The alpha channel isn't
+         *      used in any way.
          */
-        Phong& setLightColor(const Magnum::Color4& color) {
-            return setLightColors({&color, 1});
-        }
+        CORRADE_DEPRECATED("use setLightColor(std::initializer_list<Color3>) instead") Phong& setLightColor(const Magnum::Color4& color);
+        #endif
 
         /**
          * @brief Set light attenuation ranges

src/Magnum/Shaders/Test/PhongGLTest.cpp

@@ -176,7 +176,7 @@ using namespace Math::Literals;
 const struct {
     const char* name;
     Deg rotation;
-    Color4 lightColor1, lightColor2;
+    Color3 lightColor1, lightColor2;
     Float lightPosition1, lightPosition2;
 } RenderColoredData[]{
     {"", {}, 0x993366_rgbf, 0x669933_rgbf, -3.0f, 3.0f},
@@ -718,7 +718,7 @@ void PhongGLTest::setWrongLightId() {
     std::ostringstream out;
     Error redirectError{&out};
     Phong{{}, 3}
-        .setLightColor(3, {})
+        .setLightColor(3, Color3{})
         .setLightPosition(3, Vector4{})
         .setLightRange(3, 0.0f);
     CORRADE_COMPARE(out.str(),
@@ -1648,7 +1648,7 @@ void PhongGLTest::renderZeroLights() {
         /* Keep alpha mask at the default 0.5 to test the default */
         /* Passing a zero-sized light position / color array, shouldn't assert */
         .setLightPositions(Containers::ArrayView<const Vector4>{})
-        .setLightColors({})
+        .setLightColors(Containers::ArrayView<const Color3>{})
         /* Using a bogus normal matrix -- it's not used so it should be okay.
            Same for all other unused values, they should get ignored. */
         .setNormalMatrix(Matrix3x3{Math::ZeroInit})