diff --git a/doc/changelog.dox b/doc/changelog.dox index 1d01cf92c..92d6c19ac 100644 --- a/doc/changelog.dox +++ b/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. diff --git a/doc/snippets/MagnumSceneGraph-gl.cpp b/doc/snippets/MagnumSceneGraph-gl.cpp index 6aaf43ef2..f34571855 100644 --- a/doc/snippets/MagnumSceneGraph-gl.cpp +++ b/doc/snippets/MagnumSceneGraph-gl.cpp @@ -184,7 +184,7 @@ struct MyApplication: Platform::Application { SceneGraph::Object* _cameraObject; SceneGraph::Camera3D* _camera; Vector4 _lightPositionRelativeToCamera; - Vector3 _lightColor, _ambientColor; + Color3 _lightColor, _ambientColor; /* [Drawable-multiple-groups] */ // ... diff --git a/src/Magnum/Shaders/Phong.cpp b/src/Magnum/Shaders/Phong.cpp index 66b78a3db..4fb2eced0 100644 --- a/src/Magnum/Shaders/Phong.cpp +++ b/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{Containers::DirectInit, lightCount, Vector4{0.0f, 0.0f, 1.0f, 0.0f}}); - setLightColors(Containers::Array{Containers::DirectInit, lightCount, Magnum::Color4{1.0f}}); + setLightColors(Containers::Array{Containers::DirectInit, lightCount, Magnum::Color3{1.0f}}); setLightRanges(Containers::Array{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 colors) { +Phong& Phong::setLightColors(const Containers::ArrayView 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 colors) { + Containers::Array 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 colors) { + CORRADE_IGNORE_DEPRECATED_PUSH + return setLightColors(Containers::arrayView(colors)); + CORRADE_IGNORE_DEPRECATED_POP +} +#endif + +Phong& Phong::setLightColors(const std::initializer_list 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 ranges) { CORRADE_ASSERT(_lightCount == ranges.size(), "Shaders::Phong::setLightRanges(): expected" << _lightCount << "items but got" << ranges.size(), *this); diff --git a/src/Magnum/Shaders/Phong.frag b/src/Magnum/Shaders/Phong.frag index 09587b3c7..c8fa162a1 100644 --- a/src/Magnum/Shaders/Phong.frag +++ b/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) { diff --git a/src/Magnum/Shaders/Phong.h b/src/Magnum/Shaders/Phong.h index b7182d628..af3eea238 100644 --- a/src/Magnum/Shaders/Phong.h +++ b/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 colors); + Phong& setLightColors(Containers::ArrayView colors); - /** @overload */ - Phong& setLightColors(std::initializer_list colors); + /** + * @overload + * @m_since_latest + */ + Phong& setLightColors(std::initializer_list colors); + + #ifdef MAGNUM_BUILD_DEPRECATED + /** + * @brief @copybrief setLightColors(Containers::ArrayView) + * @m_deprecated_since_latest Use @ref setLightColors(Containers::ArrayView) + * instead. The alpha channel isn't used in any way. + */ + CORRADE_DEPRECATED("use setLightColors(Containers::ArrayView) instead") Phong& setLightColors(Containers::ArrayView colors); + + /** + * @brief @copybrief setLightColors(std::initializer_list) + * @m_deprecated_since_latest Use @ref setLightColors(std::initializer_list) + * instead. The alpha channel isn't used in any way. + */ + CORRADE_DEPRECATED("use setLightColors(std::initializer_list) instead") Phong& setLightColors(std::initializer_list 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) + * 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) instead") Phong& setLightColor(const Magnum::Color4& color); + #endif /** * @brief Set light attenuation ranges diff --git a/src/Magnum/Shaders/Test/PhongGLTest.cpp b/src/Magnum/Shaders/Test/PhongGLTest.cpp index ef07f0778..6472745cf 100644 --- a/src/Magnum/Shaders/Test/PhongGLTest.cpp +++ b/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{}) - .setLightColors({}) + .setLightColors(Containers::ArrayView{}) /* 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})