-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improves the PianoRoll detuning drawing #5928
base: master
Are you sure you want to change the base?
Conversation
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.
🤖 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
Linux
macOS🤖{"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"} |
I've tried to compile it but it failed (Qt version 5.15.2):
After adding Looks great! :-D |
Moves lambda functions up, uses keyAreaTop() instead of PR_TOP_MARGIN, and include missing header (<QPainterPath>).
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 |
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 |
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? |
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. |
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 |
Fixes a new conflict on PianoRoll.cpp
Maybe extend the detuning to the end of the note release and let the drawing fade too? |
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 theAutomationNode
class which doesn't emit thedataChanged()
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.