Skip to content
GitLab
Commit 01949163 authored by Jean-Baptiste Mardelle's avatar Jean-Baptiste Mardelle
Browse files

Fix Slide mix not correctly updated when creating a new mix on the previous clip, add tests. 

CCBUG: 453770
parent 8142abe5
Hide whitespace changes
Inline Side-by-side
src/timeline2/model/trackmodel.cpp
View file @ 01949163
......@@ -1808,11 +1808,14 @@ bool TrackModel::requestClipMix(const QString &mixId, std::pair<int, int> clipId
if (remixPlaylists && source_track != dest_track) {
// A list of clip ids x playlists
QMap<int, int> rearrangedPlaylists;
QMap<int, QVector<QPair<QString, QVariant>>> mixParameters;
int ix = 0;
int moveId = m_mixList.value(clipIds.second, -1);
while (moveId > -1) {
int current = m_allClips[moveId]->getSubPlaylistIndex();
rearrangedPlaylists.insert(moveId, current);
QVector<QPair<QString, QVariant>> params = m_sameCompositions.at(moveId)->getAllParameters();
mixParameters.insert(moveId, params);
if (hasEndMix(moveId)) {
moveId = m_mixList.value(moveId, -1);
} else {
......@@ -1820,7 +1823,6 @@ bool TrackModel::requestClipMix(const QString &mixId, std::pair<int, int> clipId
}
ix++;
}
rearrange_playlists = [this, rearrangedPlaylists]() {
// First, remove all clips on playlist 0
QMapIterator<int, int> i(rearrangedPlaylists);
......@@ -1876,7 +1878,8 @@ bool TrackModel::requestClipMix(const QString &mixId, std::pair<int, int> clipId
if (m_sameCompositions.count(i.key()) > 0) {
// There is a mix at clip start, adjust direction
Mlt::Transition &transition = *static_cast<Mlt::Transition*>(m_sameCompositions[i.key()]->getAsset());
transition.set("reverse", i.value());
bool reverse = i.value() == 1;
updateCompositionDirection(transition, reverse);
}
}
return true;
......@@ -1884,7 +1887,7 @@ bool TrackModel::requestClipMix(const QString &mixId, std::pair<int, int> clipId
return false;
}
};
rearrange_playlists_undo = [this, rearrangedPlaylists]() {
rearrange_playlists_undo = [this, rearrangedPlaylists, mixParameters]() {
// First, remove all clips on playlist 1
QMapIterator<int, int> i(rearrangedPlaylists);
while (i.hasNext()) {
......@@ -1939,7 +1942,13 @@ bool TrackModel::requestClipMix(const QString &mixId, std::pair<int, int> clipId
if (m_sameCompositions.count(i.key()) > 0) {
// There is a mix at clip start, adjust direction
Mlt::Transition &transition = *static_cast<Mlt::Transition*>(m_sameCompositions[i.key()]->getAsset());
transition.set("reverse", 1 - i.value());
if (mixParameters.contains(i.key())) {
// Restore all original params
QVector<QPair<QString, QVariant>> params = mixParameters.value(i.key());
for (const auto &p : qAsConst(params)) {
transition.set(p.first.toUtf8().constData(), p.second.toString().toUtf8().constData());
}
}
}
}
return true;
......@@ -1976,15 +1985,7 @@ bool TrackModel::requestClipMix(const QString &mixId, std::pair<int, int> clipId
}
if (dest_track == 0) {
// Mix should be reversed
if (mixId == QLatin1String("luma") || mixId == QLatin1String("dissolve") || mixId == QLatin1String("mix")) {
Xml::setXmlParameter(xml, QStringLiteral("reverse"), QStringLiteral("1"));
} else if (mixId == QLatin1String("composite")) {
Xml::setXmlParameter(xml, QStringLiteral("invert"), QStringLiteral("1"));
} else if (mixId == QLatin1String("wipe")) {
Xml::setXmlParameter(xml, QStringLiteral("geometry"), QStringLiteral("0=0% 0% 100% 100% 100%;-1=0% 0% 100% 100% 0%"));
} else if (mixId == QLatin1String("slide")) {
Xml::setXmlParameter(xml, QStringLiteral("rect"), QStringLiteral("0=0% 0% 100% 100% 100%;-1=100% 0% 100% 100% 100%"));
}
reverseCompositionXml(mixId, xml);
}
std::shared_ptr<AssetParameterModel> asset(new AssetParameterModel(std::move(t), xml, assetName, {ObjectType::TimelineMix, clipIds.second}, QString()));
m_sameCompositions[clipIds.second] = asset;
......@@ -2575,15 +2576,7 @@ void TrackModel::switchMix(int cid, const QString &composition, Fun &undo, Fun &
QDomElement xml = TransitionsRepository::get()->getXml(composition);
if (reverse) {
// Mix should be reversed
if (composition == QLatin1String("luma") || composition == QLatin1String("dissolve") || composition == QLatin1String("mix")) {
Xml::setXmlParameter(xml, QStringLiteral("reverse"), QStringLiteral("1"));
} else if (composition == QLatin1String("composite")) {
Xml::setXmlParameter(xml, QStringLiteral("invert"), QStringLiteral("1"));
} else if (composition == QLatin1String("wipe")) {
Xml::setXmlParameter(xml, QStringLiteral("geometry"), QStringLiteral("0=0% 0% 100% 100% 100%;-1=0% 0% 100% 100% 0%"));
} else if (composition == QLatin1String("slide")) {
Xml::setXmlParameter(xml, QStringLiteral("rect"), QStringLiteral("0=0% 0% 100% 100% 100%;-1=100% 0% 100% 100% 100%"));
}
reverseCompositionXml(composition, xml);
}
std::shared_ptr<AssetParameterModel> asset(new AssetParameterModel(std::move(t), xml, composition, {ObjectType::TimelineMix, cid}, QString()));
m_sameCompositions[cid] = asset;
......@@ -2629,6 +2622,116 @@ void TrackModel::switchMix(int cid, const QString &composition, Fun &undo, Fun &
UPDATE_UNDO_REDO(local_redo, local_undo, undo, redo);
}
void TrackModel::reverseCompositionXml(const QString &composition, QDomElement xml)
{
if (composition == QLatin1String("luma") || composition == QLatin1String("dissolve") || composition == QLatin1String("mix")) {
Xml::setXmlParameter(xml, QStringLiteral("reverse"), QStringLiteral("1"));
} else if (composition == QLatin1String("composite")) {
Xml::setXmlParameter(xml, QStringLiteral("invert"), QStringLiteral("1"));
} else if (composition == QLatin1String("wipe")) {
Xml::setXmlParameter(xml, QStringLiteral("geometry"), QStringLiteral("0=0% 0% 100% 100% 100%;-1=0% 0% 100% 100% 0%"));
} else if (composition == QLatin1String("slide")) {
Xml::setXmlParameter(xml, QStringLiteral("rect"), QStringLiteral("0=0% 0% 100% 100% 100%;-1=100% 0% 100% 100% 100%"));
}
}
void TrackModel::updateCompositionDirection(Mlt::Transition &transition, bool reverse)
{
QString composition(transition.get("kdenlive_id"));
if (composition.isEmpty()) {
composition = transition.get("mlt_service");
}
if (composition == QLatin1String("luma") || composition == QLatin1String("dissolve") || composition == QLatin1String("mix")) {
transition.set("reverse", reverse ? 1 : 0);
} else if (composition == QLatin1String("composite")) {
transition.set("invert", reverse ? 1 : 0);
} else if (composition == QLatin1String("wipe")) {
if (reverse) {
transition.set("geometry", "0=0% 0% 100% 100% 100%;-1=0% 0% 100% 100% 0%");
} else {
transition.set("geometry", "0=0% 0% 100% 100% 0%;-1=0% 0% 100% 100% 100%");
}
} else if (composition == QLatin1String("slide")) {
QString currentSlide(transition.get("rect"));
currentSlide.replace(QLatin1Char('%'), QString());
currentSlide = currentSlide.section(QLatin1Char('='), 1);
// Check if we start centered
if (!reverse && currentSlide.startsWith(QLatin1String("0 0 "))) {
currentSlide = currentSlide.section(QLatin1Char('='), 1);
QStringList sizes = currentSlide.split(QLatin1Char(' '));
double x = sizes.at(0).toDouble();
double y = sizes.at(1).toDouble();
QString result = QStringLiteral("0=");
if (x > 0) {
result.append(QStringLiteral("-100% "));
} else if (x < 0) {
result.append(QStringLiteral("100% "));
} else {
result.append(QStringLiteral("0% "));
}
if (y > 0) {
result.append(QStringLiteral("-100% "));
} else if (y < 0) {
result.append(QStringLiteral("100% "));
} else {
result.append(QStringLiteral("0% "));
}
result.append(QStringLiteral("100% 100% 100%;-1=0% 0% 100% 100% 100%"));
transition.set("rect", result.toUtf8().constData());
} else if (reverse) {
QString secondPart = currentSlide.section(QLatin1Char('='), 1);
if (secondPart.startsWith(QLatin1String("0 0 "))) {
QStringList sizes = currentSlide.split(QLatin1Char(' '));
double x = sizes.at(0).toDouble();
double y = sizes.at(1).toDouble();
QString result = QStringLiteral("0=0% 0% 100% 100% 100%;-1=");
if (x > 0) {
result.append(QStringLiteral("-100% "));
} else if (x < 0) {
result.append(QStringLiteral("100% "));
} else {
result.append(QStringLiteral("0% "));
}
if (y > 0) {
result.append(QStringLiteral("-100% "));
} else if (y < 0) {
result.append(QStringLiteral("100% "));
} else {
result.append(QStringLiteral("0% "));
}
result.append(QStringLiteral("100% 100% 100%"));
transition.set("rect", result.toUtf8().constData());
}
}
}
}
bool TrackModel::mixIsReversed(int cid) const
{
if (m_sameCompositions.count(cid) > 0) {
// There is a mix at clip start, adjust direction
Mlt::Transition &transition = *static_cast<Mlt::Transition*>(m_sameCompositions.at(cid)->getAsset());
QString composition(transition.get("kdenlive_id"));
if (composition.isEmpty()) {
composition = transition.get("mlt_service");
}
if (composition == QLatin1String("luma") || composition == QLatin1String("dissolve") || composition == QLatin1String("mix")) {
return transition.get_int("reverse") == 1;
} else if (composition == QLatin1String("composite")) {
return transition.get_int("invert") == 1;
} else if (composition == QLatin1String("wipe")) {
QString geom = transition.get("geometry");
geom.replace(QLatin1Char('%'), QString());
return geom.contains(QStringLiteral(" 100;"));
} else if (composition == QLatin1String("slide")) {
QString geom(transition.get("rect"));
geom.replace(QLatin1Char('%'), QString());
return geom.startsWith(QStringLiteral("0=0 0 "));
}
}
return false;
}
QVariantList TrackModel::stackZones() const
{
return m_effectStack->getEffectZones();
......
src/timeline2/model/trackmodel.hpp
View file @ 01949163
......@@ -330,6 +330,8 @@ protected:
int isOnCut(int cid);
/** @brief Returns all mix info as xml */
QDomElement mixXml(QDomDocument &document, int cid) const;
/** @brief Check if a mix is reversed (moslty used in tests) */
bool mixIsReversed(int cid) const;
public slots:
/** Delete the current track and all its associated clips */
......@@ -359,6 +361,8 @@ private:
/// This is a lock that ensures safety in case of concurrent access
mutable QReadWriteLock m_lock;
void reverseCompositionXml(const QString &composition, QDomElement xml);
void updateCompositionDirection(Mlt::Transition &transition, bool reverse);
protected:
std::shared_ptr<EffectStackModel> m_effectStack;
......
tests/mixtest.cpp
View file @ 01949163
......@@ -567,6 +567,157 @@ TEST_CASE("Simple Mix", "[SameTrackMix]")
state0();
}
SECTION("Test chained mixes and check mix direction")
{
// Add 2 more color clips
int cid5;
int cid6;
int cid7;
state0();
REQUIRE(timeline->requestClipInsertion(binId2, tid2, 540, cid5));
REQUIRE(timeline->requestItemResize(cid5, 20, true, true));
REQUIRE(timeline->requestClipInsertion(binId2, tid2, 560, cid6));
REQUIRE(timeline->requestItemResize(cid6, 40, true, true));
REQUIRE(timeline->requestClipInsertion(binId2, tid2, 600, cid7));
REQUIRE(timeline->requestItemResize(cid7, 20, true, true));
// Cid3 pos=500, duration=20
// Cid4 pos=520, duration=20
// Cid5 pos=540, duration=20
// Cid6 pos=560, duration=40
// Cid7 pos=600, duration=20
auto mix0 = [&]() {
REQUIRE(timeline->getClipsCount() == 9);
REQUIRE(timeline->m_allClips[cid3]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid4]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid5]->getSubPlaylistIndex() == 1);
REQUIRE(timeline->m_allClips[cid6]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid7]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->getTrackById_const(tid2)->mixCount() == 1);
REQUIRE(timeline->getTrackById_const(tid2)->mixIsReversed(cid5) == false);
};
auto mix1 = [&]() {
REQUIRE(timeline->m_allClips[cid3]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid4]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid5]->getSubPlaylistIndex() == 1);
REQUIRE(timeline->m_allClips[cid6]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid7]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->getTrackById_const(tid2)->mixCount() == 2);
REQUIRE(timeline->getTrackById_const(tid2)->mixIsReversed(cid5) == false);
REQUIRE(timeline->getTrackById_const(tid2)->mixIsReversed(cid6) == true);
};
auto mix2 = [&]() {
REQUIRE(timeline->m_allClips[cid3]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid4]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid5]->getSubPlaylistIndex() == 1);
REQUIRE(timeline->m_allClips[cid6]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid7]->getSubPlaylistIndex() == 1);
REQUIRE(timeline->getTrackById_const(tid2)->mixCount() == 3);
REQUIRE(timeline->getTrackById_const(tid2)->mixIsReversed(cid5) == false);
REQUIRE(timeline->getTrackById_const(tid2)->mixIsReversed(cid6) == true);
REQUIRE(timeline->getTrackById_const(tid2)->mixIsReversed(cid7) == false);
};
auto mix3 = [&]() {
REQUIRE(timeline->m_allClips[cid3]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid4]->getSubPlaylistIndex() == 1);
REQUIRE(timeline->m_allClips[cid5]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->m_allClips[cid6]->getSubPlaylistIndex() == 1);
REQUIRE(timeline->m_allClips[cid7]->getSubPlaylistIndex() == 0);
REQUIRE(timeline->getTrackById_const(tid2)->mixCount() == 4);
REQUIRE(timeline->getTrackById_const(tid2)->mixIsReversed(cid4) == false);
REQUIRE(timeline->getTrackById_const(tid2)->mixIsReversed(cid5) == true);
REQUIRE(timeline->getTrackById_const(tid2)->mixIsReversed(cid6) == false);
REQUIRE(timeline->getTrackById_const(tid2)->mixIsReversed(cid7) == true);
};
// Mix 4 and 5
REQUIRE(timeline->mixClip(cid5));
mix0();
// Mix 5 and 6
REQUIRE(timeline->mixClip(cid6));
mix1();
// Mix 6 and 7
REQUIRE(timeline->mixClip(cid7));
mix2();
// Mix 3 and 4, this will revert all subsequent mixes
REQUIRE(timeline->mixClip(cid4));
mix3();
// Undo mix 3 and 4
undoStack->undo();
mix2();
// Now switch mixes to Slide type
timeline->switchComposition(cid7, QString("slide"));
timeline->switchComposition(cid6, QString("slide"));
timeline->switchComposition(cid5, QString("slide"));
mix2();
// Mix 3 and 4, this will revert all subsequent mixes
REQUIRE(timeline->mixClip(cid4));
mix3();
// Undo mix 3 and 4
undoStack->undo();
mix2();
// Now switch mixes to Wipe type
timeline->switchComposition(cid7, QString("wipe"));
timeline->switchComposition(cid6, QString("wipe"));
timeline->switchComposition(cid5, QString("wipe"));
mix2();
// Mix 3 and 4, this will revert all subsequent mixes
REQUIRE(timeline->mixClip(cid4));
mix3();
// Undo mix 3 and 4
undoStack->undo();
mix2();
// Undo Wipe mix switch on cid5
undoStack->undo();
// Undo mix switch on cid6
undoStack->undo();
// Undo mix switch on cid7
undoStack->undo();
mix2();
// Undo Slide mix switch on cid5
undoStack->undo();
// Undo mix switch on cid6
undoStack->undo();
// Undo mix switch on cid7
undoStack->undo();
mix2();
// Undo mix 6 and 7
undoStack->undo();
mix1();
// Undo mix 5 and 6
undoStack->undo();
mix0();
// Undo mix 4 and 5
undoStack->undo();
// Undo insert/resize ops
undoStack->undo();
undoStack->undo();
undoStack->undo();
undoStack->undo();
undoStack->undo();
undoStack->undo();
state0();
}
binModel->clean();
pCore->m_projectManager = nullptr;
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment