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

Apply filters when processing pedals #3881

Merged
merged 7 commits into from
Dec 10, 2024
Merged

Apply filters when processing pedals #3881

merged 7 commits into from
Dec 10, 2024

Conversation

lpugin
Copy link
Contributor

@lpugin lpugin commented Dec 9, 2024

Fixes #3880

  • const_cast should be avoided, but needs refactoring of GetTstampStaves

Copy link
Contributor

@rettinghaus rettinghaus left a comment

Choose a reason for hiding this comment

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

What about the case if @staff is missing and only @startid is being used?

@lpugin
Copy link
Contributor Author

lpugin commented Dec 9, 2024

What about the case if @staff is missing and only @startid is being used?

See

* Return a vector of staves looking at the @staff attribute or at the parent staff or the @startid

Copy link
Contributor

@brdvd brdvd left a comment

Choose a reason for hiding this comment

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

Another glitch is that the pedal event is duplicated since the functor is repeated for each layer.

Pedal-duplicated

Not sure how to approach this. Probably it's not harmful and we can keep it for now. I'll have a look at the const-cast-issue.

src/midifunctor.cpp Outdated Show resolved Hide resolved
@lpugin
Copy link
Contributor Author

lpugin commented Dec 9, 2024

Another glitch is that the pedal event is duplicated since the functor is repeated for each layer.
Pedal-duplicated

Not sure how to approach this. Probably it's not harmful and we can keep it for now.

The easiest is to add a flag to the functor to indicate when Pedal should be processed or not. Alternatively we could check if there is a Layer filter - which is the case only once when the functor is processed, but that would probably be just more complicated.

I'll have a look at the const-cast-issue.

Thanks!

@brdvd
Copy link
Contributor

brdvd commented Dec 9, 2024

The easiest is to add a flag to the functor to indicate when Pedal should be processed or not. Alternatively we could check if there is a Layer filter - which is the case only once when the functor is processed, but that would probably be just more complicated.

Yes, sounds good. This flag should actually effect the processing of any control elements.

* const_cast should be avoided, but needs refactoring of GetTstampStaves
@lpugin lpugin force-pushed the develop-pedal-fix branch from 1f31d7f to 226fa15 Compare December 9, 2024 11:59
@lpugin lpugin requested a review from brdvd December 9, 2024 12:09
src/doc.cpp Outdated Show resolved Hide resolved
src/midifunctor.cpp Show resolved Hide resolved
Copy link
Contributor

@brdvd brdvd left a comment

Choose a reason for hiding this comment

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

Looks great now, thank you!

src/midifunctor.cpp Outdated Show resolved Hide resolved
Co-authored-by: Andrew Hankinson <[email protected]>
@ahankinson ahankinson merged commit fd418ec into develop Dec 10, 2024
9 checks passed
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.

Pedal affects all MIDI instruments
4 participants