Edit::markAsChanged() not called on midi note changes

Edit::markAsChanged() is called for all other modifications like the addition of new tracks or audio clip movements. However, that is not the case for midi note modifications inside a midi clip. Any idea why?

It should do as changing the note properties are undoable actions so the UndoManger should inform the UndoTransactionTimer to call edit.markAsChanged(). Are you sure that’s not happening?
How are you changing the note properties?

Hello, thanks for the help.

Are you sure that’s not happening?
Pretty sure. If I put a breakpoint in Edit::markAsChanged(), it never gets called when I change the size of the note.

How are you changing the note properties?
I’ve tried 2 ways:

  1. Using setProperty on te::IDs::b and te::IDs::l directly (works great).
  2. midiNote->setStartAndLength where midiNote is a te::MidiNote* (works great also).

One thing to note (no pun intended), is that for the setProperty, I pass nullptr for the undo manager. So I can see 1. not working but 2. should, no?

I’ve spent a bit of time digging into the tracktion code but no luck yet.

P.S. Edit::markAsChanged() is called for other modifications like track creation and movement and resize of audio and midi clips.

I’m trying to come up with a unit test case for this but I’m a bit unsure what your use case is?
Is it just that you’d expect to be able to undo changing a note’s length?

Hello

I want to know if there are pending changes in the Edit. At the moment, Edit::hasChangedSinceSaved() returns false after modifying midi notes (resizing, moving, etc…). From what you’re telling me, it should return true.

Ok, I can add a test for that.
Just one more thing, this operation is async, so you need to leave the message thread unblocked for about half a second before it will update. You’re not trying to check it straight away are you?

No
I modify a midi note. I get up to get a coffee. And then close my app which calls Edit::hasChangedSinceSaved(). It returns false.

Well I just added this test and verified it works as expected:

I think maybe you need to step through the UndoManager::perform function where it calls sendChangeMessage to see if it actually sends the message? And if so, does the Edit::UndoTransactionTimer pick it up?

Interesting.
Thanks for looking into this.
I’ll look into your suggestions.

Juggling tasks at the moment but I’ve found some time to bring your test code into mine. I’m still getting the same problem.


Anyway, it must be the way I’m initializing my Edit.
Will keep looking.

In that code snipped, where are you creating m_Edit?
The change manager is only created asynchronously so a synchronous test (if that’s what it is) won’t work.

OK, I’ve moved your code into a test button action (so, plenty of time for things to get created). I can confirm that your code now works. However, if I pass nullptr for the UndoManager, the problem is back. I’m not using the UndoManager at the moment in my code but will be in the near future.

I find it strange that Edit::hasChangedSinceSaved() returns true for clip and track modifications even without the UndoManager, but that note modifications needs the UndoManaer in order for Edit::hasChangedSinceSaved() to return true.

Thanks again for your help

Hmm, I’ve not looked in to it but I image changing clip properties is actually doing something with the UndoManger internally which is causing the change?

We’ve been through many systems of trying to figure out when an Edit has changed over the years but have found this works the best. The reasoning being that for things like view changes etc. that aren’t undoable, you don’t want the Edit to marked as “dirty” and needing saving.

For anything significant, it will usually be undoable and so it’s handled by the undo manager.
For everything else, you can always call Edit::markAsChanged() manually.

I guess bring up the question of why you’re not passing in the undo manager in this instance?

I didn’t pass the UndoManager because I was new to juce and tracktion and wanted to get things working quickly I guess. One less thing to think about? Maybe a bad decision … I don’t know.

The important thing is that things are more clear now and I’ll put the UndoManager at the top of my list.

Thanks

Yeah, that can be a bit strange. For most things, it’s just done internally. But for a MidiList, you can construct them without an Edit so there’s explicit passing of the UndoManager.

I have a feeling this might be legacy though as there’s a constructor that takes an UndoManager. Maybe it would make more sense to always use this internally and get rid of that argument.