Selected clip length updates

[michaelgregorius]

Do not update the clip length when dragging a node in AutomationClip::setDragValue.

This is achieved by removing the call to updateLength from AutomationClip::removeNode and explicitly calling the update in all other places that call removeNode except in setDragValue.

This commit therefore also improves the separation of concerns. Removing a node now only update the data structures. Updating the clip length is a completely different thing and rather a "policy" and therefore both calls should be done independently.

Fixes #7466.

[JohannesLorenz]

@michaelgregorius Please don't invite me to review just randomly. I am no expert at all about the topic (for Lv2, e.g., it would make sense), and I want to choose for myself which PRs and how many I review (note I am only a free-time developer). Also, I would rather recommend not to ask "at all" or "at lmms-developers" (or whatever the tag was to alert everyone), unless it it really relevant to all or requires feedback of everyone.

[michaelgregorius]

@JohannesLorenz, sorry, I have invited you because you were proposed by the GitHub UI which stated that you would have already done some development/reviews for the changed code area. Unfortunately, I don't really have an overview of who is responsible for what. So oftentimes I search for developers that I already had some interactions with here.

By the way, I am also not working professionally on LMMS. 😉

[sakertooth]

Just tested this PR.

Shrinking the automation clip to be less than one bar and then adding a node within the bounds of the clip does not resize it to one bar, which is good. However, removing that node causes the clip to be resized to one bar still, which I don't think is intended. The same occurs when you drag the node to the beginning of the clip.

Also, it might be better to update the clip length in response to changes of the last node, so that if the user drags the last node too far and then drags it back to where they want it, they don't have to update the clip length manually if they don't want the clip to be that long. However, that might be out of scope/unwanted, not sure.

2024-10-12.01-15-48.mp4

[michaelgregorius]

Shrinking the automation clip to be less than one bar and then adding a node within the bounds of the clip does not resize it to one bar, which is good. However, removing that node causes the clip to be resized to one bar still, which I don't think is intended. The same occurs when you drag the node to the beginning of the clip.

Both problems are caused by AutomationClip::timeMapLength which returns a length of one bar if the map is empty or if the last node is at position 0:

lmms/src/core/AutomationClip.cpp

Line 206 in 44837db

TimePos AutomationClip::timeMapLength() const

Also, it might be better to update the clip length in response to changes of the last node, so that if the user drags the last node too far and then drags it back to where they want it, they don't have to update the clip length manually if they don't want the clip to be that long. However, that might be out of scope/unwanted, not sure.

I think it's a question in general if these automated length changes help the users. The "only change if the last node is changed" mode might also create extra work for the users. Let's say you have a set of four consecutive clips and each is one bar long. If you edit the second one in such a way that it's last node gets moved towards the start then the clip might be shortened and the clips are not consecutive blocks anymore.