-
-
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
Fix PianoRoll m_positionLine misalignment on zoom #5965
Conversation
🤖 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
Windows
macOS🤖{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://13208-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.85%2Bg1eeb855-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13208?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://13206-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.85%2Bg1eeb8555d-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13206?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://13209-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.85%2Bg1eeb8555d-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13209?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/add31i6g7eysnxjk/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38413938"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/dn4vflileilgwm49/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38413938"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://13207-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.85%2Bg1eeb8555d-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13207?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "5d7a4a32f07d754e2cc8620629f134b1cdf75203"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Suggested a small fix on the comment about the width
if (pos >= m_currentPosition && pos <= m_currentPosition + width() - m_whiteKeyWidth) | ||
// ticks relative to m_currentPosition | ||
// < 0 = outside viewport left | ||
// > width = outside viewport right |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// > width = outside viewport right | |
// > (width - piano keys width) = outside viewport right |
It look good to me (windows 10) |
There's no problem in the win64 build now, I am using win10. Thanks. |
Merging tomorrow unless anyone objects. |
Fixes #5962
Adds comments to elaborate on what is being calculated.