Hello,
thanks you for your instructions!
I looked around in phabricator and I didn't know where to post it, so
for now I hope it's ok to send it here...
Desired functionality: With the patch it shoud be possible to change
Lift in the range from -1 to +1.
MLT and Kdenlive diffs attached.
Best regards
Dušan Hanuš
Dne 8.9.2017 v 17:08 Jean-Baptiste Mardelle napsal(a):
On Friday, September 8, 2017 3:28:57 PM CEST, Dušan Hanuš wrote:
Hello,
Hello and welcome!
I modified the Lift Gamma Gain effect so that I can select negative
Lift values (which for me as an editor is absolute must so that I
don't need to use another --and slow-- effects like Bezier Curves etc.)
Now if I wanted to share the commit, how do I do that? (I'm not very
experienced developer, so please could you describe it as
specifically as possible?)
If you downloaded the source code using git, you should be able to
easily create a patch using:
git diff > mypatch.diff
You can then send the patch on the mailing list for review, or open an
issue on phabricator.kde.org and attach the patch here (you will need
to create a kde account).
If you did not use git, I guess the easiest is to download the source
code using git in a separate directory, and after that copy the
modified source files to the git version. You can then use the "git
diff" command to create the patch.
I imagine it must be reviewed somehow...
And there's one more thing. This effect is actually two parts - a
QWidget and a MLT effect which unfortunately does not handle negative
values greatly and must be modified too.
So I guess MLT is completely different project and I have to ask
there, right?
Yes that's correct. The patch related to MLT can be sent to the
mlt-devel mailing list: https://sourceforge.net/p/mlt/mailman/
You can also post the patches to Kdenlive and MLT on this list first
so some people can test it and give you feedback.
Let us know if you need more help.
Best regards
Jean-Baptiste Mardelle
Thank you!
diff --git a/src/effectstack/widgets/colorwheel.cpp b/src/effectstack/widgets/colorwheel.cpp
index 7dd34df..d735720 100644
--- a/src/effectstack/widgets/colorwheel.cpp
+++ b/src/effectstack/widgets/colorwheel.cpp
@@ -21,7 +21,48 @@
#include "colorwheel.h"
#include <qmath.h>
-ColorWheel::ColorWheel(const QString &id, const QString &name, const QColor &color, QWidget *parent)
+NegQColor NegQColor::fromHsvF(qreal h, qreal s, qreal l, qreal a)
+{
+ NegQColor color;
+ color.qcolor = QColor::fromHsvF(h,s,l<0?-l:l,a);
+ color.sign_r = l<0?-1:1;
+ color.sign_g = l<0?-1:1;
+ color.sign_b = l<0?-1:1;
+ return color;
+}
+NegQColor NegQColor::fromRgbF(qreal r, qreal g, qreal b, qreal a)
+{
+ NegQColor color;
+ color.qcolor = QColor::fromRgbF(r<0?-r:r,g<0?-g:g,b<0?-b:b,a);
+ color.sign_r = r<0?-1:1;
+ color.sign_g = g<0?-1:1;
+ color.sign_b = b<0?-1:1;
+ return color;
+}
+qreal NegQColor::redF() {
+ return qcolor.redF()*sign_r;
+}
+qreal NegQColor::greenF() {
+ return qcolor.greenF()*sign_g;
+}
+qreal NegQColor::blueF() {
+ return qcolor.blueF()*sign_b;
+}
+qreal NegQColor::valueF() {
+ return qcolor.valueF()*sign_g;
+}
+int NegQColor::hue() {
+ return qcolor.hue();
+}
+qreal NegQColor::hueF() {
+ return qcolor.hueF();
+}
+qreal NegQColor::saturationF() {
+ return qcolor.saturationF();
+}
+
+
+ColorWheel::ColorWheel(const QString &id, const QString &name, const NegQColor &color, QWidget *parent)
: QWidget(parent)
, m_id(id)
, m_isMouseDown(false)
@@ -41,12 +82,18 @@ ColorWheel::ColorWheel(const QString &id, const QString &name, const QColor &col
setCursor(Qt::CrossCursor);
}
-QColor ColorWheel::color()
+void ColorWheel::setFactorDefaultZero(qreal factor, qreal defvalue, qreal zero) {
+ m_sizeFactor = factor;
+ m_defaultValue = defvalue;
+ m_zeroShift = zero;
+}
+
+NegQColor ColorWheel::color()
{
return m_color;
}
-void ColorWheel::setColor(const QColor &color)
+void ColorWheel::setColor(const NegQColor &color)
{
m_color = color;
}
@@ -56,10 +103,10 @@ int ColorWheel::wheelSize() const
return qMin(width() - m_sliderWidth, height() - m_unitSize);
}
-QColor ColorWheel::colorForPoint(const QPoint &point)
+NegQColor ColorWheel::colorForPoint(const QPoint &point)
{
if (! m_image.valid(point)) {
- return QColor();
+ return NegQColor();
}
if (m_isInWheel) {
qreal w = wheelSize();
@@ -74,13 +121,17 @@ QColor ColorWheel::colorForPoint(const QPoint &point)
theta += 2.0 * M_PI;
}
qreal hue = (theta * 180.0 / M_PI) / 360.0;
- return QColor::fromHsvF(hue, rad, m_color.valueF());
+ return NegQColor::fromHsvF(hue, rad, m_color.valueF());
}
if (m_isInSquare) {
qreal value = 1.0 - qreal(point.y() - m_margin) / (wheelSize() - m_margin * 2);
- return QColor::fromHsvF(m_color.hueF(), m_color.saturationF(), value);
+ if (m_zeroShift!=0) {
+ value=value-m_zeroShift;
+ }
+
+ return NegQColor::fromHsvF(m_color.hueF(), m_color.saturationF(), value);
}
- return QColor();
+ return NegQColor();
}
QSize ColorWheel::sizeHint() const
@@ -107,7 +158,7 @@ void ColorWheel::mousePressEvent(QMouseEvent *event)
qreal g = m_color.greenF();
qreal max = qMax(r, b);
max = qMax(max, g);
- changeColor(QColor::fromRgbF(max, max, max));
+ changeColor(NegQColor::fromRgbF(max, max, max));
}
} else if (m_sliderRegion.contains(m_lastPoint)) {
m_isInWheel = false;
@@ -115,14 +166,15 @@ void ColorWheel::mousePressEvent(QMouseEvent *event)
if (event->button() != Qt::MidButton) {
changeColor(colorForPoint(m_lastPoint));
} else {
- QColor c;
- if (m_id == QLatin1String("lift")) {
- c = QColor::fromRgbF(0, 0, 0);
+ NegQColor c;
+/* if (m_id == QLatin1String("lift")) {
+ c = NegQColor::fromRgbF(0.0, 0.0, 0.0);
} else if (m_id == QLatin1String("gamma")) {
- c = QColor::fromRgbF(0.5, 0.5, 0.5);
+ c = NegQColor::fromRgbF(0.5, 0.5, 0.5);
} else {
- c = QColor::fromRgbF(0.25, 0.25, 0.25);
- }
+ c = NegQColor::fromRgbF(0.25, 0.25, 0.25);
+ }*/
+ c = NegQColor::fromRgbF(m_defaultValue/m_sizeFactor, m_defaultValue/m_sizeFactor, m_defaultValue/m_sizeFactor);
changeColor(c);
}
}
@@ -136,10 +188,10 @@ void ColorWheel::mouseMoveEvent(QMouseEvent *event)
return;
}
if (m_wheelRegion.contains(m_lastPoint) && m_isInWheel) {
- const QColor color = colorForPoint(m_lastPoint);
+ const NegQColor color = colorForPoint(m_lastPoint);
changeColor(color);
} else if (m_sliderRegion.contains(m_lastPoint) && m_isInSquare) {
- const QColor color = colorForPoint(m_lastPoint);
+ const NegQColor color = colorForPoint(m_lastPoint);
changeColor(color);
}
}
@@ -152,6 +204,30 @@ void ColorWheel::mouseReleaseEvent(QMouseEvent *event)
m_isInSquare = false;
}
+
+/*void ColorWheel::keyPressEvent(QKeyEvent *event )
+{
+ QPoint point = m_lastPoint;
+ switch(event->key()){
+ case Qt::Key_Up:
+ point.setY(point.y()-1);
+ break;
+ case Qt::Key_Down:
+ point.setY(point.y()+1);
+ break;
+ case Qt::Key_Left:
+ point.setX(point.x()-1);
+ break;
+ case Qt::Key_Right:
+ point.setX(point.x()+1);
+ break;
+ }
+ if (m_wheelRegion.contains(point)) {
+ const NegQColor color = colorForPoint(point);
+ changeColor(color);
+ }
+}*/
+
void ColorWheel::resizeEvent(QResizeEvent *event)
{
m_image = QImage(event->size(), QImage::Format_ARGB32_Premultiplied);
@@ -164,14 +240,15 @@ void ColorWheel::resizeEvent(QResizeEvent *event)
QString ColorWheel::getParamValues()
{
- if (m_id == QLatin1String("gamma")) {
+/* if (m_id == QLatin1String("gamma")) {
return QString::number(m_color.redF() * 2, 'g', 2) + QLatin1Char(',') + QString::number(m_color.greenF() * 2, 'g', 2) + QLatin1Char(',') + QString::number(m_color.blueF() * 2, 'g', 2);
} else if (m_id == QLatin1String("gain")) {
return QString::number(m_color.redF() * 4, 'g', 2) + QLatin1Char(',') + QString::number(m_color.greenF() * 4, 'g', 2) + QLatin1Char(',') + QString::number(m_color.blueF() * 4, 'g', 2);
}
// default (lift)
return QString::number(m_color.redF(), 'g', 2) + QLatin1Char(',') + QString::number(m_color.greenF(), 'g', 2) + QLatin1Char(',') + QString::number(m_color.blueF(), 'g', 2);
-
+*/
+ return QString::number(m_color.redF() * m_sizeFactor, 'g', 2) + QLatin1Char(',') + QString::number(m_color.greenF() * m_sizeFactor, 'g', 2) + QLatin1Char(',') + QString::number(m_color.blueF() * m_sizeFactor, 'g', 2);
}
void ColorWheel::paintEvent(QPaintEvent *event)
@@ -262,6 +339,10 @@ void ColorWheel::drawWheelDot(QPainter &painter)
void ColorWheel::drawSliderBar(QPainter &painter)
{
qreal value = 1.0 - m_color.valueF();
+ if (m_id == QLatin1String("lift")) {
+ value -= m_zeroShift;
+ }
+
int ws = wheelSize();
qreal scale = qreal(ws + m_sliderWidth) / maximumWidth();
int w = m_sliderWidth * scale;
@@ -275,7 +356,7 @@ void ColorWheel::drawSliderBar(QPainter &painter)
painter.resetTransform();
}
-void ColorWheel::changeColor(const QColor &color)
+void ColorWheel::changeColor(const NegQColor &color)
{
m_color = color;
drawWheel();
diff --git a/src/effectstack/widgets/colorwheel.h b/src/effectstack/widgets/colorwheel.h
index bc3b8ab..32937ef 100644
--- a/src/effectstack/widgets/colorwheel.h
+++ b/src/effectstack/widgets/colorwheel.h
@@ -23,22 +23,42 @@
#include <QPainter>
#include <QResizeEvent>
+class NegQColor
+{
+public:
+ int8_t sign_r=1;
+ int8_t sign_g=1;
+ int8_t sign_b=1;
+ QColor qcolor;
+ static NegQColor fromHsvF(qreal h, qreal s, qreal l, qreal a = 1.0);
+ static NegQColor fromRgbF(qreal r, qreal g, qreal b, qreal a = 1.0);
+ qreal redF();
+ qreal greenF();
+ qreal blueF();
+ qreal valueF();
+ int hue();
+ qreal hueF();
+ qreal saturationF();
+};
+
class ColorWheel : public QWidget
{
Q_OBJECT
public:
- explicit ColorWheel(const QString &id, const QString &name, const QColor &color, QWidget *parent = nullptr);
+ explicit ColorWheel(const QString &id, const QString &name, const NegQColor &color, QWidget *parent = nullptr);
QSize sizeHint() const Q_DECL_OVERRIDE;
QSize minimumSizeHint() const Q_DECL_OVERRIDE;
- QColor color();
- void setColor(const QColor &color);
+ NegQColor color();
+ void setColor(const NegQColor &color);
+ void setFactorDefaultZero(qreal factor, qreal defvalue, qreal zero);
+
signals:
- void colorChange(const QColor &color);
+ void colorChange(const NegQColor &color);
public slots:
- void changeColor(const QColor &color);
+ void changeColor(const NegQColor &color);
protected:
void mousePressEvent(QMouseEvent *event) Q_DECL_OVERRIDE;
@@ -47,6 +67,8 @@ protected:
void resizeEvent(QResizeEvent *event) Q_DECL_OVERRIDE;
void paintEvent(QPaintEvent *event) Q_DECL_OVERRIDE;
+// void keyPressEvent(QKeyEvent *keyEvent ) Q_DECL_OVERRIDE;
+
private:
QString m_id;
QSize m_initialSize;
@@ -57,14 +79,18 @@ private:
int m_sliderWidth;
QRegion m_wheelRegion;
QRegion m_sliderRegion;
- QColor m_color;
+ NegQColor m_color;
bool m_isInWheel;
bool m_isInSquare;
int m_unitSize;
QString m_name;
+ qreal m_sizeFactor = 1;
+ qreal m_defaultValue = 1;
+ qreal m_zeroShift = 0;
+
int wheelSize() const;
- QColor colorForPoint(const QPoint &point);
+ NegQColor colorForPoint(const QPoint &point);
void drawWheel();
void drawWheelDot(QPainter &painter);
void drawSliderBar(QPainter &painter);
diff --git a/src/effectstack/widgets/lumaliftgain.cpp b/src/effectstack/widgets/lumaliftgain.cpp
index ecea0f2..1ac93b7 100644
--- a/src/effectstack/widgets/lumaliftgain.cpp
+++ b/src/effectstack/widgets/lumaliftgain.cpp
@@ -24,6 +24,7 @@
#include <KLocalizedString>
+static const double LIFT_FACTOR = 2.0;
static const double GAMMA_FACTOR = 2.0;
static const double GAIN_FACTOR = 4.0;
@@ -45,21 +46,24 @@ LumaLiftGain::LumaLiftGain(const QDomNodeList &nodes, QWidget *parent) :
values.insert(pa.attribute(QStringLiteral("name")), val);
}
- QColor lift = QColor::fromRgbF(values.value(QStringLiteral("lift_r")),
- values.value(QStringLiteral("lift_g")),
- values.value(QStringLiteral("lift_b")));
- QColor gamma = QColor::fromRgbF(values.value(QStringLiteral("gamma_r")) / GAMMA_FACTOR,
+ NegQColor lift = NegQColor::fromRgbF(values.value(QStringLiteral("lift_r")) / LIFT_FACTOR,
+ values.value(QStringLiteral("lift_g")) / LIFT_FACTOR,
+ values.value(QStringLiteral("lift_b")) / LIFT_FACTOR);
+ NegQColor gamma = NegQColor::fromRgbF(values.value(QStringLiteral("gamma_r")) / GAMMA_FACTOR,
values.value(QStringLiteral("gamma_g")) / GAMMA_FACTOR,
values.value(QStringLiteral("gamma_b")) / GAMMA_FACTOR);
- QColor gain = QColor::fromRgbF(values.value(QStringLiteral("gain_r")) / GAIN_FACTOR,
+ NegQColor gain = NegQColor::fromRgbF(values.value(QStringLiteral("gain_r")) / GAIN_FACTOR,
values.value(QStringLiteral("gain_g")) / GAIN_FACTOR,
values.value(QStringLiteral("gain_b")) / GAIN_FACTOR);
m_lift = new ColorWheel(QStringLiteral("lift"), i18n("Lift"), lift, this);
+ m_lift->setFactorDefaultZero(LIFT_FACTOR, 0, 0.5);
connect(m_lift, &ColorWheel::colorChange, this, &LumaLiftGain::valueChanged);
m_gamma = new ColorWheel(QStringLiteral("gamma"), i18n("Gamma"), gamma, this);
+ m_gamma->setFactorDefaultZero(GAMMA_FACTOR, 1, 0);
connect(m_gamma, &ColorWheel::colorChange, this, &LumaLiftGain::valueChanged);
m_gain = new ColorWheel(QStringLiteral("gain"), i18n("Gain"), gain, this);
+ m_gain->setFactorDefaultZero(GAIN_FACTOR, 1, 0);
connect(m_gain, &ColorWheel::colorChange, this, &LumaLiftGain::valueChanged);
flowLayout->addWidget(m_lift);
@@ -78,13 +82,13 @@ LumaLiftGain::LumaLiftGain(const QDomNodeList &nodes, QWidget *parent) :
void LumaLiftGain::updateEffect(QDomElement &effect)
{
- QColor lift = m_lift->color();
- QColor gamma = m_gamma->color();
- QColor gain = m_gain->color();
+ NegQColor lift = m_lift->color();
+ NegQColor gamma = m_gamma->color();
+ NegQColor gain = m_gain->color();
QMap<QString, double> values;
- values.insert(QStringLiteral("lift_r"), lift.redF());
- values.insert(QStringLiteral("lift_g"), lift.greenF());
- values.insert(QStringLiteral("lift_b"), lift.blueF());
+ values.insert(QStringLiteral("lift_r"), lift.redF() * LIFT_FACTOR);
+ values.insert(QStringLiteral("lift_g"), lift.greenF() * LIFT_FACTOR);
+ values.insert(QStringLiteral("lift_b"), lift.blueF() * LIFT_FACTOR);
values.insert(QStringLiteral("gamma_r"), gamma.redF() * GAMMA_FACTOR);
values.insert(QStringLiteral("gamma_g"), gamma.greenF() * GAMMA_FACTOR);
diff --git a/src/modules/plus/filter_lift_gamma_gain.c b/src/modules/plus/filter_lift_gamma_gain.c
index 11c377b..140cd5b 100644
--- a/src/modules/plus/filter_lift_gamma_gain.c
+++ b/src/modules/plus/filter_lift_gamma_gain.c
@@ -67,6 +67,11 @@ static void refresh_lut( mlt_filter filter, mlt_frame frame )
g += glift * ( 1.0 - g );
b += blift * ( 1.0 - b );
+ // Clamp negative values
+ r = r < 0.0 ? 0.0 : r;
+ g = g < 0.0 ? 0.0 : g;
+ b = b < 0.0 ? 0.0 : b;
+
// Apply gamma
r = pow( r, 2.2 / rgamma );
g = pow( g, 2.2 / ggamma );
