diff --git a/modules/tracktion_engine/model/clips/tracktion_AudioClipBase.cpp b/modules/tracktion_engine/model/clips/tracktion_AudioClipBase.cpp index b13f36e7ed3..f70dd440012 100644 --- a/modules/tracktion_engine/model/clips/tracktion_AudioClipBase.cpp +++ b/modules/tracktion_engine/model/clips/tracktion_AudioClipBase.cpp @@ -205,10 +205,16 @@ class ProxyGeneratorJob : public AudioProxyGenerator::GeneratorJob //============================================================================== AudioClipBase::AudioClipBase (const juce::ValueTree& v, EditItemID id, Type t, ClipOwner& targetParent) : Clip (v, targetParent, id, t), - loopInfo (edit.engine, state.getOrCreateChildWithName (IDs::LOOPINFO, getUndoManager()), getUndoManager()), + loopInfo (edit.engine, state.getOrCreateChildWithName (IDs::LOOPINFO, nullptr), getUndoManager()), pluginList (edit), lastProxy (edit.engine) { + // N.B. Nothing in this constructor may write to the state via the UndoManager. + // A clip can be constructed whilst an undo is in progress (undoing a record or a + // clip deletion re-adds the clip's state), and UndoManager::perform refuses to + // re-enter, so any undoable write made here is silently discarded. + // Corrections to derived geometry and old-Edit migrations are made with a null + // UndoManager instead. auto um = getUndoManager(); level->dbGain.referTo (state, IDs::gain, um); @@ -245,9 +251,9 @@ AudioClipBase::AudioClipBase (const juce::ValueTree& v, EditItemID id, Type t, C // Keep this in to handle old edits.. if (state.getProperty (IDs::timeStretch)) - timeStretchMode = juce::VariantConverter::fromVar (state.getProperty (IDs::stretchMode)); + timeStretchMode.setValue (juce::VariantConverter::fromVar (state.getProperty (IDs::stretchMode)), nullptr); - timeStretchMode = TimeStretcher::checkModeIsAvailable (timeStretchMode); + timeStretchMode.setValue (TimeStretcher::checkModeIsAvailable (timeStretchMode), nullptr); autoPitch.referTo (state, IDs::autoPitch, um); autoPitchMode.referTo (state, IDs::autoPitchMode, um); @@ -256,7 +262,7 @@ AudioClipBase::AudioClipBase (const juce::ValueTree& v, EditItemID id, Type t, C isReversed.referTo (state, IDs::isReversed, um); autoDetectBeats.referTo (state, IDs::autoDetectBeats, um); - level->pan = juce::jlimit (-1.0f, 1.0f, static_cast (level->pan.get())); + level->pan.setValue (juce::jlimit (-1.0f, 1.0f, static_cast (level->pan.get())), nullptr); checkFadeLengthsForOverrun(); useClipLaunchQuantisation.referTo (state, IDs::useClipLaunchQuantisation, um); @@ -750,8 +756,12 @@ void AudioClipBase::checkFadeLengthsForOverrun() if (fadeIn + fadeOut > len) { const double scale = len / (fadeIn + fadeOut); - fadeIn = fadeIn * scale; - fadeOut = fadeOut * scale; + + // This is derived geometry, not user data, and it can be corrected whilst a clip is + // being constructed. If that happens during an undo, an undoable write would re-enter + // UndoManager::perform, which refuses it and silently discards the correction. + fadeIn.setValue (fadeIn * scale, nullptr); + fadeOut.setValue (fadeOut * scale, nullptr); } // also check the auto fades diff --git a/modules/tracktion_engine/model/clips/tracktion_Clip.cpp b/modules/tracktion_engine/model/clips/tracktion_Clip.cpp index e7f1e729e78..8fa7c6bf07d 100644 --- a/modules/tracktion_engine/model/clips/tracktion_Clip.cpp +++ b/modules/tracktion_engine/model/clips/tracktion_Clip.cpp @@ -879,6 +879,67 @@ TEST_SUITE("tracktion_engine") CHECK (clip->getPosition().time == positionBeforeTrim.time); } } + + TEST_CASE("Fade length correction isn't undoable") + { + auto& engine = *tracktion::engine::Engine::getEngines()[0]; + auto edit = Edit::createSingleTrackEdit (engine); + auto track = getAudioTracks (*edit)[0]; + auto& undoManager = edit->getUndoManager(); + + auto clip = dynamic_cast (insertNewClip (*track, TrackItem::Type::wave, { 2_tp, 3_tp })); + REQUIRE (clip != nullptr); + + // A comp remnant can be left with fades that are longer than the clip itself + const auto length = TimeDuration::fromSeconds (0.00999999999999979); + auto clipState = clip->state; + clipState.setProperty (IDs::length, length.inSeconds(), nullptr); + clipState.setProperty (IDs::fadeIn, 0.0, nullptr); + clipState.setProperty (IDs::fadeOut, 0.02, nullptr); + + SUBCASE ("Correcting the fades during construction isn't undoable") + { + track->state.removeChild (clipState, nullptr); + undoManager.clearUndoHistory(); + undoManager.beginNewTransaction(); + + track->state.addChild (clipState, -1, nullptr); + const double correctedFadeOut = clipState[IDs::fadeOut]; + CHECK (correctedFadeOut == doctest::Approx (length.inSeconds())); + + // Undoing back past the clip's construction mustn't revert the correction + while (undoManager.canUndo()) + undoManager.undo(); + + CHECK (double (clipState[IDs::fadeOut]) == doctest::Approx (correctedFadeOut)); + } + + SUBCASE ("Fades are corrected when a clip is rebuilt during an undo") + { + undoManager.clearUndoHistory(); + undoManager.beginNewTransaction(); + clip->removeFromParent(); + REQUIRE (track->getClips().isEmpty()); + + undoManager.beginNewTransaction(); + undoManager.undo(); + + REQUIRE (track->getClips().size() == 1); + auto rebuiltClip = dynamic_cast (track->getClips()[0]); + REQUIRE (rebuiltClip != nullptr); + CHECK (rebuiltClip->getFadeIn() + rebuiltClip->getFadeOut() <= length); + + // The correction has to reach the state, not just the CachedValue, or it's + // lost the next time the Edit is loaded + const double fadeInInState = rebuiltClip->state[IDs::fadeIn]; + const double fadeOutInState = rebuiltClip->state[IDs::fadeOut]; + CHECK (fadeInInState + fadeOutInState <= length.inSeconds()); + CHECK (fadeOutInState == doctest::Approx (length.inSeconds())); + + // The undo history must still be intact + CHECK (undoManager.canRedo()); + } + } } } // namespace tracktion::inline engine