Skip to content

PianoRoll: fix getKey and remove dead code #8008

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

allejok96
Copy link
Contributor

  • Fixes: when right clicking on a piano key close to the lower edge of the screen, "Mark semitone" will mark the wrong key because it uses the upper edge of the context menu as reference.
  • Fixes: getKey() is off by 2 pixels
  • Introduces: yCoordOfKey() which is the reverse of getKey()
  • Removes: section of code that can never be reached

@allejok96 allejok96 changed the title PianoRoll: fix getKey calculations and remove unrachable code PianoRoll: fix getKey calculations and remove unreachable code Jul 13, 2025
@allejok96 allejok96 changed the title PianoRoll: fix getKey calculations and remove unreachable code PianoRoll: fix getKey and remove dead code Jul 13, 2025
@JohannesLorenz JohannesLorenz self-requested a review July 14, 2025 18:23
Copy link
Contributor

@JohannesLorenz JohannesLorenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only 2 editorials.

Checked:

  • Code logic
  • Code style
  • Partial testing: I checked that the dead code cannot be reached

Suggesting to test the other points of the OP.

@@ -343,6 +342,9 @@ protected slots:
int noteEditRight() const;
int noteEditLeft() const;

int getKey(const int y) const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use const in the definition. However, in the declaration, it does not make sense, and thus often causes compiler warnings (having const only in the definition does not cause them).

return key_num;
// If y == keyAreaBottom() the cursor is on the first pixel BELOW the editor
// and thus its distanceFromBottom should be -1. We calculate the distance from
// ABSOLUTE bottom before dividing, because integer division with negative numbers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because integer division with negative numbers causes rounding issues

What does this mean? Can you explain, maybe with a simple example?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants