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

Same Beat and Measure (Select All Similar Filter) #20137

Merged
merged 2 commits into from
Nov 24, 2023

Conversation

Jojo-Schmitz
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz commented Nov 21, 2023

Continuing on #7421, resp. porting (the rest of) #5637

Resolves: https://musescore.org/en/node/295235

Images of the dialog, the English items are the additions:
image
image

The 1st commit brings Mu4 on par with Mu3
The 2nd commit fixes a bug with text lines that exists in Mu3 too

@Jojo-Schmitz Jojo-Schmitz force-pushed the same-beat-measure branch 2 times, most recently from 7df14d5 to 3f36308 Compare November 22, 2023 09:33
@worldwideweary
Copy link
Contributor

Hey Jojo, something I realized since when I implemented that back in late 2019: apparently text-lines don't carry a valid measure property, so that the action fails to give desirable results when using for example a pedal line or crescendo hairpin. The code I used in 3.x locally goes something like this (There's probably a cleaner way than with the compounded if-statements, but safety first and it worked):

  1. Store the measure of element in collectMatch:
 void Score::collectMatch(void* data, Element* e)
       {
       ElementPattern* p = static_cast<ElementPattern*>(data);
+      auto eMeasure = e->findMeasure();
  1. If eMeasure was null but is a spanner segment, double check its "start element" and get its measure via findMeasure() instead of just pulling out if e->findMeasure is invalid:
-      if (p->measure && (p->measure != e->findMeasure()))
-            return;
- 
+      if (p->measure) {
+            if (!eMeasure && e->isSpannerSegment()) {
+                  if (auto ss  = toSpannerSegment(e)){
+                  if (auto s   = ss->spanner())      {
+                  if (auto se  = s->startElement())  {
+                  if (auto mse = se->findMeasure())  {
+                        eMeasure = mse;
+                        }}}}
+                  }
+            if (p->measure != eMeasure)
+                  return;
+            }
  1. Same thing with setting the pattern:
-      if (sameMeasure->isChecked())
-            p->measure = e->findMeasure();
- 
+      if (sameMeasure->isChecked()) {
+            auto m = e->findMeasure();
+            if (!m && e->isSpannerSegment()) {
+                  if (auto ss  = toSpannerSegment(e)) {
+                  if (auto s   = ss->spanner())       {
+                  if (auto se  = s->startElement())   {
+                  if (auto mse = se->findMeasure())   {
+                        m = mse;
+                        }}}}
+                  }
+            if (m)
+                  p->measure = m;
+            }

Works over here...

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Nov 23, 2023

But that'd be an additional issue and fix, needed for Mu4 and Mu3, right?

Also I can't reproduce the problem you describe, neither in 3.6.2 nor in my PR for master

Edit: actually I can now reproduce it

@Jojo-Schmitz Jojo-Schmitz force-pushed the same-beat-measure branch 2 times, most recently from 66a6e68 to f4f9783 Compare November 23, 2023 16:15
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 24, 2023
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 24, 2023
@zacjansheski
Copy link
Contributor

Tested on MacOS 14, Windows 11, Ubuntu 22.04.3. Approved
@cbjeukendrup please advise on 4.2/4.3 protocol

@Jojo-Schmitz
Copy link
Contributor Author

It does add stings to translate. Not sure whether that's an issue, but rather mention it 😉

@cbjeukendrup
Copy link
Contributor

@Jojo-Schmitz If you would like to make a 4.2 version too, I think that's fine since this change is low-risk enough (it only touches a relatively self-contained bit of functionality).
@zacjansheski Since 4.2 and master have hardly diverged yet, I don't think a 4.2 version would require separate testing.

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Nov 24, 2023

So you'd like a 4.2.0 PR?

@cbjeukendrup
Copy link
Contributor

Well, if you're willing to create one, then yes please :)

@cbjeukendrup cbjeukendrup merged commit 2ea4345 into musescore:master Nov 24, 2023
11 checks passed
@Jojo-Schmitz
Copy link
Contributor Author

Done, see #20187

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 25, 2023
@Jojo-Schmitz Jojo-Schmitz deleted the same-beat-measure branch November 25, 2023 10:21
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

4 participants