Set AutomationPattern length to 1 bar if the length is 0

[Veratil]

Fixes #5254

AutomationPattern's length is now always updated (not only if it's a HiddenAutomationTrack which we're removing eventually). Just changing that though resulted in the length of the block in the SongEditor to be 0 (or a single pixel), so this also makes the minimum length of the block 1 bar.

Tested playing around with adding and removing blocks from BB tracks, Automation tracks, and Piano tracks, all seemed to still behave properly.

EDIT: I totally forgot that there were actually tests for the automation track length. I'm not sure how we want to change these.

If the last bar and last tick is 0, then set the length to 1 bar.

[LmmsBot]

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

• Linux (AppImage): lmms-1.2.1.637-linux-x86_64.AppImage (build link)

Windows

• Windows 32-bit: lmms-1.2.1.637-mingw-win32.exe (build link)
• Windows 64-bit: lmms-1.2.1.637-mingw-win64.exe (build link)
• Windows 32-bit: lmms-1.2.1-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.2.1-msvc2017-win64.exe (build link)

macOS

• macOS : lmms-1.2.1.637-mac10.13.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://6047-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.637-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6047?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://6050-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.637-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6050?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://6046-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.637-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6046?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/gbe50uaocvyvd07k/artifacts/build/lmms-1.2.1-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/32455342"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/c1oxoy96igh3mihr/artifacts/build/lmms-1.2.1-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/32455342"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://6048-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.637-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6048?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "a4f677362d3c37e5436884c70c8869f0cc9174cd"}

[Spekular]

this also makes the minimum length of the block 1 bar.

This breaks some of the functionality #4973 introduced. Patterns shorter than a bar are valid. They're especially useful for sample tracks, but I think they should be available in all track types.

Proposed alternatives:

• Only update when the pattern would grow
• Only update when adding a node to the right of all other nodes
• Only update if at least one node with position > 0 exists
• Apply snapping to the automatic length change? I know I had to add some checks to avoid zero length clips, so perhaps the snapping code would handle this case.

[Veratil]

I didn't realize it wasn't restricted!

I'll rework and figure out a better solution. 👍

[Veratil]

Update OP.

This should fix the issue, while still allowing any length TCO as @Spekular pointed out.

This breaks some of the functionality #4973 introduced. Patterns shorter than a bar are valid. They're especially useful for sample tracks, but I think they should be available in all track types.

I do notice you can't resize PianoRoll TCOs, but all the others can resize the same (even Automation). 🙂

[Spekular]

I was hoping that leaving a "comment" review would get rid of the merge-blocking "changes requested" state without explicitly approving the changes, but that didn't work. I've approved them now because I think you should be able to merge this if you want to, but I do have another round of proposed changes :)

[Veratil]

Actually, I do have a question for the class now: this changes the behavior of Automation tracks.

Let's say I do this:

1. Resize the track to 15 bars
2. Put a (last) tick on bar 6

This resizes the length down to 6 bars. Do we want this behavior? Should we never resize down?

[Spekular]

Should we never resize down?

I think this is a pretty sane solution. Does it work if you change line 201 in AutomationPattern.cpp as below?

-changeLength( timeMapLength() );
+changeLength( qMax(timeMapLength(), length()) );

[Veratil]

Should we never resize down?

I think this is a pretty sane solution. Does it work if you change line 201 in AutomationPattern.cpp as below?

-changeLength( timeMapLength() );
+changeLength( qMax(timeMapLength(), length()) );

It doesn't seem to work. It looks like if you extend the visual track, the internal length is still only the default 192 (or whatever the latest change through updateLength is).

[Spekular]

That's odd, resizing should call changeLength...

lmms/src/core/Track.cpp

Line 948 in 2419433

else if( m_action == Resize || m_action == ResizeLeft )

[Veratil]

Disregard previous comment, apparently had old compiled version running. That does indeed work!

[Spekular]

Tested and works perfectly for me:

• Doesn't create zero length clips
• Never shrinks clips
• Edit: Oh and I guess I should mention that it fixes the original issue.