Skip to content
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

Don't update m_positionLine in paintEvent #5967

Merged
merged 1 commit into from
Apr 15, 2021

Conversation

Veratil
Copy link
Contributor

@Veratil Veratil commented Mar 26, 2021

Fixes #5863 (for real this time)

This extra update in paintEvent was causing the misalignment in the piano keys (and actually anywhere the position line was in the window). There is still a very small glitch of the line height when resizing, but this is a small issue I can fix in another PR later.

@Veratil Veratil added the needs testing This pull request needs more testing label Mar 26, 2021
@LmmsBot
Copy link

LmmsBot commented Mar 26, 2021

🤖 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://13235-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.85%2Bgf0e04cd97-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13235?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://13234-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.85%2Bgf0e04cd97-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13234?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/1a75o55en9ropsj9/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38426809"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/reun7q77fojjs99s/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38426809"}], "Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://13233-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.85%2Bgf0e04cd-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13233?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://13231-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.85%2Bgf0e04cd97-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13231?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "2d1200acb69734c45bc4089c3a38dd3349d4a6b0"}

@superpaik
Copy link
Contributor

superpaik commented Mar 29, 2021

While the resizing seem to work almost ok regarding the keys of the pianoroll, there are some issues with the notes (see attached animated gif) that are misaligned with the pianoroll keys. The misaligned is corrected when the cursors enters the pianoroll screen.
Also, maybe it's not related to this PR, when in 150% vertical zooming, some notes names (G, D and C) seem to not have enough space to be written and are "cut" by the left. Only in 150%, not with any other vertical zoom. Tested on windows 10.
imatge

test-pianoRoll-resize

@Veratil
Copy link
Contributor Author

Veratil commented Mar 29, 2021

While the resizing seem to work almost ok regarding the keys of the pianoroll, there are some issues with the notes (see attached animated gif) that are misaligned with the pianoroll keys. The misaligned is corrected when the cursors enters the pianoroll screen.

I'll look into that one next. Thanks!

Also, maybe it's not related to this PR, when in 150% vertical zooming, some notes names (G, D and C) seem to not have enough space to be written and are "cut" by the left. Only in 150%, not with any other vertical zoom.

Not related, but I believe another PR is touching this code and gives it more draw space.


So other than those two separate issues, this PR looks good then I believe. 👍

@Veratil
Copy link
Contributor Author

Veratil commented Apr 14, 2021

Merging tomorrow unless anyone objects.

@Veratil Veratil merged commit 3c4e67c into LMMS:master Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs testing This pull request needs more testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pianoroll resizing is graphically broken in LMMS 1.3
4 participants