Improves the PianoRoll detuning drawing

[IanCaio]

This PR rewrites the detuning drawing method of the PianoRoll to behave similar to the AutomationEditor. It now draws Cubic Hermite progressions as well, and the detuning is displayed as a shape instead of a line.

The color of the detuning shape is the same as m_noteColor, except it's lighter and has some alpha.

There's one visual glitch left to fix, which is also present in master: Changes to the automation node's outValues don't immediately trigger a repaint on the PianoRoll, because they are done directly on the AutomationNode class which doesn't emit the dataChanged() signal on the Automation Pattern. This is an easy fix, once it's decided how to approach it. Waiting for some opinions on the Discord channel before working on the fix.

[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! 🎩

Windows

• Windows 32-bit: lmms-1.3.0-alpha.1.72%2Bg0215ce709-mingw-win32.exe (build link)
• Windows 64-bit: lmms-1.3.0-alpha.1.72%2Bg0215ce709-mingw-win64.exe (build link)
• Windows 32-bit: lmms-1.3.0-alpha-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.3.0-alpha-msvc2017-win64.exe (build link)

Linux

• Linux (AppImage): lmms-1.3.0-alpha.1.72%2Bg0215ce7-linux-x86_64.AppImage (build link)

macOS

• macOS : lmms-1.3.0-alpha.1.72%2Bg0215ce709-mac10.14.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://12717-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.72%2Bg0215ce709-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/12717?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://12716-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.72%2Bg0215ce709-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/12716?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/08r51o3s4fump0eo/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38031731"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/gq3bv6g3o7qwnmfr/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38031731"}], "Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://12718-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.72%2Bg0215ce7-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/12718?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://12719-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.72%2Bg0215ce709-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/12719?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "d6f9f9dc41e765704c726449637f094f30b2eaae"}

[cyber-bridge]

I've tried to compile it but it failed (Qt version 5.15.2):

/home/unknown/lmms/detuningDrawing/src/gui/editors/PianoRoll.cpp:1046:16: error: aggregate 'QPainterPath path' has incomplete type and cannot be defined
 1046 |   QPainterPath path;
      |                ^~~~

After adding #include <QPainterPath> to src/gui/editors/PianoRoll.cpp it did compile.

Looks great! :-D

[IanCaio]

I've tried to compile it but it failed (Qt version 5.15.2):

/home/unknown/lmms/detuningDrawing/src/gui/editors/PianoRoll.cpp:1046:16: error: aggregate 'QPainterPath path' has incomplete type and cannot be defined
 1046 |   QPainterPath path;
      |                ^~~~

After adding #include <QPainterPath> to src/gui/editors/PianoRoll.cpp it did compile.

Looks great! :-D

Added the missing header! I've no idea how it compiled without complaints here

[cyber-bridge]

Added the missing header! I've no idea how it compiled without complaints here

I can only guess that your version of Qt is below 5.15. Probably this commit is responsible: https://code.qt.io/cgit/qt/qtbase.git/commit/?id=50d2acdc93b4de2ba56eb67787e2bdcb21dd4bea

[AaronRecord]

I personally like lines better than the shapes but other than that it looks good. I guess the shapes might make it easier to tell which note the detune belongs to

[superpaik]

A part from the little glitch thing you commented, I think that if we use shapes (I like it better than lines, because is in line with the automation detuning editor), we have to modify how it is drawing because it can lead to user confusion. If I'm not wrong, both detuning automations (see attached images) have the same shape in the piano roll editor. One solution would be having in the piano roll the same shape as the automation editor or another, to highlight the shape contour according the automation (second image. Don't take into account the bad quality of the mock-up, it's just an idea ;-)

[IanCaio]

A part from the little glitch thing you commented, I think that if we use shapes (I like it better than lines, because is in line with the automation detuning editor), we have to modify how it is drawing because it can lead to user confusion. If I'm not wrong, both detuning automations (see attached images) have the same shape in the piano roll editor. One solution would be having in the piano roll the same shape as the automation editor or another, to highlight the shape contour according the automation (second image. Don't take into account the bad quality of the mock-up, it's just an idea ;-)

True, well noticed! Highlighting the contour might be a little tricky because even though it looks like one single shape, the drawings are actually many shapes close together, but I'd have to look into it to be sure. Maybe there's a way to add that contour that is not really complicated.

But I thought of an alternative too, which would be extending the shape until the end of the note. Then the first one would continue at C5 until the end and the second one would have that discrete jump and continue on top of the note. What do you think?

[superpaik]

Yes, that'd be great! But have in mind that user can change the note duration once automation has been established, so it would have to change accordingly, and maybe that is not that easy. But if possible, I think it's a good solution.

[IanCaio]

True, once I get some time to work on it I'll check which one will add less complexity: Reworking the path generation so I can have a single path and then use the outline or drawing the detuning all the way to the end of note. The latter might not even require that I worry about the note resizing because the Piano Roll redrawing is connected to the Pattern dataChanged signal, so every time a note is resized the Piano Roll will redraw anyways, and the detuning shape will be redrawn too (now accounting to the resized note)

[zonkmachine]

Bloody brilliant, want this! Glitches that are already in LMMS doesn't have to be fixed here for a merge. That's my opinion anyway.

[zonkmachine]

or drawing the detuning all the way to the end of note.

Maybe extend the detuning to the end of the note release and let the drawing fade too?