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

Moving pianoroll changes into new PR. #9109

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

Conversation

blackears
Copy link
Contributor

Resolves: (direct link to the issue)

(short description of the changes and the motivation to make the changes)

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • I created the test (mtest, vtest, script test) to verify the changes I made

@blackears
Copy link
Contributor Author

This replaces #9033 which has gotten unmanageable.

@blackears blackears mentioned this pull request Sep 13, 2021
8 tasks
class AutomationVertex : public EngravingItem
{
double _value;
double _ticks; //Number of whole notes offset from start of score
Copy link
Contributor

Choose a reason for hiding this comment

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

how will this work if the user removes or inserts a time range (measure etc.) into the music - will all existing vertices be adjusted accordingly?
All ticks can just be integers no? (even _value might be better as a float or qreal, I haven't seen a lot of doubles being used so far)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was planning for the vertices to be stored in a list sorted by _ticks. _ticks could be an integer or something else, but if it is an integer then we will need to determine what unit one tick is. We could use the 1/480 of a whole note MS3 used - is that unit still relevant?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I assumed that's what it was meant to be!
But what I meant is that if you had, say, vertices defined for ticks 480 to 4800 then the user removed the first (4/4) measure entirely, would that modify all extract vertices in all staffs to subtract 480 from the "tick" values?
What if the user removed measures at the end of the piece that meant the vertex tick value was
now beyond the end of the piece? (I would think you couldn't just delete the vertex if it was defining a slope...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a measure is deleted, I would presume any coresponding vertices would be deleted too and it would be up to the user to fix things if it caused a problem. Probably something to ask Martin about, though.

@RobFog
Copy link

RobFog commented Mar 17, 2022

A rebase is needed.

@blackears
Copy link
Contributor Author

blackears commented Mar 17, 2022

Looks like Github just upgraded its security. I need to change all my keys first.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Mar 17, 2022

@scorster
Copy link

scorster commented Nov 29, 2023

Just adding a link to Piano Roll Editor bug reports and requests located in the MuseScore discussion area.

https://github.com/orgs/musescore/discussions/16925

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.

None yet

5 participants