MuseScore icon indicating copy to clipboard operation
MuseScore copied to clipboard

[MU3] Fix GH#7449: Almost everything suitable for 3.6.3 or 3.7.0

Open Jojo-Schmitz opened this issue 4 years ago • 63 comments

Mostly resolves #7449 (i.e.: it includes the vast majority of the PRs listed there, but not all. Contains far more than listed there too)

Submitted mainly to get the development builds from GitHub CI (AKA artifacts, see the corresponding checks), for Windows (64-bit and 32-bit, no PortableApp unfortunately), Mac and Linux (64-bit AppImage) and also to get the vtests to run (which are expected to fail, but I'm curious in what way. Edit: As far as I can tell the vtests failures don't show any real failures, only changes for the better, but see for yourself, from the artifact of the corresponding check), and also for the mtests of course (which I expect it to pass with flying colors. Edit: indeed they do).

Yes, it really is more than 350 commits (and still counting)... All this on top of the 81 commits that the 3.x branch is ahead of 3.6.2 already. So this indeed feels more like a 3.7(.0) rather than a 3.6.3, but as long as such a thing is not planned for, nor another PR for such a much smaller 3.6.3 exists... Edit: I made up my mind and made it a 3.7.0 now ;-) (Might also get named 3.9.0, to make clear that it really is the last minor release before/below 4.0)

From PRs against the 3.x branch (not merged into it yet):

  • #4853
  • #5096 (Despite the title this really is/was for 3.x, needs #7704)
  • #5752 (Despite the title this really is/was for 3.x)
  • #5805 (Despite the title this really is/was for 3.x, needs #7704)
  • #5840 (Despite the title this really is/was for 3.x, marked as WIP)
  • #5902 (Needs #7704. Doesn't seem to work properly, @SKefalidis?)
  • #6095
  • #6312 (Might needs some compatibility switch. May cause 'denser' layouts, and so differences at where automatic system breaks are placed)
  • #6489
  • #6693
  • #7003
  • #7388 (Might be usefull to maintainers of the builds for Linux distributions)
  • #7443 (Both parts)
  • #7453 / #7597
  • #7455 / #8014
  • #7463 / #7584 (Partly)
  • #7464 / #7498
  • #7500 (Needs #7704)
  • #7520 / #7521 (Both parts)
  • #7523
  • ~#7870, part 1 / #7617~ (But reverted again later, due to to many mtest failures)
  • #7870, part 2 / #7621
  • #7704 (Needed by several other PRs/commits, non-consequential unless merged into 3.x for real)
  • #7705
  • #7728
  • #7739
  • #7870, part 3 / #7863
  • #7891 / #7891 (Needs #7704 too)
  • #7902 / #7900
  • #7935 / #8019
  • #7947 / #7793 (Both parts)
  • #7952
  • #7979 / #7978 (Needs #7704 too)
  • #7987 / #7988 (Needs #7704 too)
  • #8000 / #8001
  • #8008
  • #8011 / #8012 + #8967
  • #8034 / #8033 _(Both parts, see also #6021)
  • #8036 / #8037
  • #8059
  • #8071 / #8072
  • #8074 / #8075
  • #8083 / #8082
  • #8102 / #8103
  • #8111 / #8112
  • #8187 / #8188
  • #8282 / #8429
  • #8488 / #8489
  • #8515 / #8516
  • #8578 / #8853
  • #8721
  • #9019
  • #9081
  • #9236
  • #9238
  • #9593
  • #9851
  • #10158
  • Adding QmlModels, needed for Qt 5.14 and later (no PR)
  • Fix #321767: Delete/rename/move/clarify the "Open" column of "String Data" (no PR)
  • Fix #324413: Allow loading (compressed) scores with uppercase extensions (no PR)
  • Fix #325316: Help using (almost) any font in chord symbols (no PR)
  • Fix #327933: Privacy Policy link is not working after Reset to factory defaults. (no PR, not applicable to master)
  • Fix #328337: Crash on deleting time signatures in front of multimeasure rests (no PR, apparently not applicable to master)
  • Add solo channel to violins, violas, cellos and contrabasses (no PR)
  • Remove some entries from the Advanced Preferences (as they are available via the UI)
  • Fix #10407
  • Switching to Qt 5.15.2, mainly for macOS and Fix #317323: macOS 10.5 accessibility setting “Enable Hover Text” causes crash, but updating the builds for Linux and Windows too. Also enables plugins to use ECMA 6 features. (needed to revert that for Windows 32-bit builds, as there's no Qt5.15.2 package available for CI, and for Linux, as otherwise the vtests won't work)
  • Fix Image paste crash (Fixing a screw-up of mine from #7699, a backport of #7698)
  • Fix #22842: First ABC import overwritten by the second (no PR, not (anymore/yet again) applicable to master)
  • Fix #304539: ABC import fails (no PR, not (anymore/yet again) applicable to master)
  • #12018
  • Fix #304539: Allow "reverse" (no PR, not (anymore/yet again) applicable to master)
  • Update translations (partly for testing, but should stay in, and gets updated from time to time with the latest translations from Transifex, plus having the translations for German completed, for the new/changed strings)
  • Force building 32-bit Windows on GitHub CI (but still using Qt 5.9) and allow "Save online" even for an unstable build in GUI and on the commandline. (just for testing),

Not included so far:

  • #4833 (Despite its title it is rather based on 3.x, pretty big, from GSoC 2019)
  • #5003 (Despite its title it is rather based on 3.x, needs more work, mtests fail, @mattmcclinch?)
  • #5050 (Despite its title it is rather based on 3.x)
  • #5414 (Despite its title it is rather based on 3.x, rebase needed, @mgavioli?)
  • #5505 (Despite its title it is rather based on 3.x)
  • ~#5637 (Despite its title it is rather based on 3.x)~
  • #5720 (Despite its title it is rather based on 3.x)
  • ~#5735 (Despite its title it is rather based on 3.x, causes a regression: ties are not shown anymore at all, @sidewayss?)~
  • #5762 (Despite its title it is rather based on 3.x, needs some work)
  • #6032 (Despite its title it is rather based on 3.x, mtests need fixing)
  • ~#6080~
  • #6145 (Despite its title it is rather based on 3.x, pretty huge)
  • #6494 (Needs more work, I'd need help with that)
  • #6550 (Not sure, it is quite big)
  • #6917 (Needs #6550 and is quite big too)
  • #7127 (But see also #5816

PRs backported from master (merged and not yet merged ones):

  • #7617
  • #7744 (Partly)
  • #7957 (Part 2 only)
  • #8106 / #8137 (Got closed meanwhile, but still seems worthwhile IMHO)
  • #8141
  • #8142 (Both parts)
  • #8146 (But see also #8175)
  • #8170
  • #8244
  • #8254
  • #8279
  • #8289
  • #8299 (at least the macOS part)
  • #8424
  • #8429 / #8282 (Both parts)
  • #8510
  • #8762 (Partly)
  • #8765 (Partly, the parts not related to Unity builds, although getting 3.x to use Unity builds would be quite nice)
  • #8772
  • #8775 (Partly, the part fixing Fix #284064: "Respell pitches" should only work on the selection, if there is any)_
  • #8793
  • #8808
  • #8818 (Fixing a regression from #8510)
  • #8825 (Better/different way to fix the issue from #8816)
  • #8833 (Both parts)
  • #8835 (Needs #7704)
  • #8877 (Both parts) / #7870, part 4 and 5 (needs #7704). Need to check vs. #12340!
  • #8894 (All 4 parts, needs #7704)
  • #8903
  • #8919
  • #8953 (All 3 parts)
  • #8958: https://trello.com/c/86Ks7zZX/58-stemdownnw-stemupse-not-honoured-for-x-notehead
  • #8978
  • #8979
  • #8985
  • #9001
  • #9077
  • #9086
  • #9095
  • #9097
  • #9104
  • #9126
  • #9137 (needs parts of #9068 for computeFirstSegmentXPosition())
  • #9138
  • #9140
  • #9158
  • #9193 (need to check whether/how it interacts with #5902)
  • #9219
  • #9280
  • #9281 (may need to get reverted, as it seems to cause a regression, see https://github.com/musescore/MuseScore/pull/9281#issuecomment-1019658960 and https://github.com/musescore/MuseScore/pull/9000#issuecomment-1019634404)
  • #9308
  • #9309
  • #9405/#9422
  • #9446
  • #9456
  • #9496
  • #9532
  • #9560
  • #9574
  • #9601
  • #9630
  • #9647
  • #9662
  • #9706
  • #9730
  • #9733
  • #9774
  • #9798
  • #9804
  • #9805 (not fully workying yet, so needs more work)
  • #9819
  • #9831
  • #9850
  • #9864
  • #9903
  • #9955
  • #9989
  • #10000 (may need more work)
  • #10007
  • #10060
  • #10094
  • #10159
  • #10282
  • #10283
  • #10346
  • #10374
  • #10412
  • #10442
  • #10454
  • #10471
  • #10488
  • #10516
  • #10519
  • #10534
  • #10549
  • #10577
  • #10586
  • #10646
  • #10654
  • #10663 (backport in 2 different commits and also changing the affected templates)
  • #10675
  • #10709
  • #10725
  • #10761
  • #10793 (2nd part only, 1st doesn't apply to 3.x)
  • #10809
  • #10849
  • #10860
  • #11413
  • #11485 (But there's a conflict with #8556)
  • #11560 (the 2nd commit only, the 1st requires C++17)
  • #11561
  • #11609
  • #11959
  • #12026
  • #12063
  • #12084
  • #12086
  • #12106
  • #12133
  • #12203
  • #12541
  • #12652
  • #12680
  • #12863
  • #13060
  • #13063
  • #13488 (without the mtests, and as that basically a duplicate of #7621, backported quite a while ago)
  • #14618
  • #14797
  • #14803
  • #14924 (plus fixing the templates causing that issue)
  • #15001 (plus fixing a compiler warning and some formatting)
  • #15725
  • #15730
  • #15927

Not included so far:

  • #5762 (Minor tweak needed to address code review)
  • #5816 (But see also #7127)
  • #6233
  • #7997
  • #8273
  • #8425
  • #8431
  • #8976 (Pretty big, worth the effort?)
  • #8867 (Pretty big, worth the effort?)
  • #8956 (Maybe? Why not?)
  • #9068 (UI missing though, at least partially needed for #9137, but otherwise quite large for little benefit and apparently needs C++17)
  • ~#9092 (UI missing though, seems to need more work, but porting it doesn't seem to make sense anyhow)~
  • #9237 (quite big)
  • #9277
  • #9307 (probably too involved)
  • #9359 (needs more work though, like it fails badly on older files (MuseScore 2 and probably 1))
  • #9365 (maybe)
  • #9389 (needs more work)
  • #9395 (needs/follow-up to #9359)
  • ~#9414 (apparently not needed, not even partly)~
  • #9452 (needs #9359)
  • #9461 (needs more work, several mtests fail)
  • #9602
  • #9653
  • #10044
  • #10066
  • #10609 (maybe instead of or in addition to #10543)
  • #10665 (seems not needed, but at least not harmful either)

PRs against the 3.6.2_backend / 3.6.2_backend_musicxml branches (all merged into those). See also #9581 for a forward port of those to master:

  • #8108
  • #8116
  • #8133
  • #8175
  • #8176
  • #8181
  • #8190
  • #8237
  • #8275
  • #8283
  • #8284
  • #8285
  • ~#8296, part 1~ (causes a regression, so it got reverted (much) later)
  • #8296, part 2
  • #8298
  • #8311
  • #8325
  • #8326
  • #8347
  • #8358
  • #8412
  • #8413
  • #8430 (But see https://github.com/musescore/MuseScore/pull/8430#pullrequestreview-698705061)
  • #8433
  • #8461
  • #8478
  • #8480
  • #8481
  • #8492
  • #8506
  • #8509
  • #8527
  • #8530
  • #8554 (Merged as part of the commit that caused these warnings)
  • #8556 (But there's a conflict with #11485)
  • #8581
  • #8583
  • #8613
  • #8622
  • #8623
  • #8636
  • #8637
  • #8649
  • #8651
  • #8678
  • #8763 (Partly, see below)
  • #8766 (Partly, see below)
  • #8866
  • Several commits fixing failing mtests related to the above PRs and their interaction with other PRs in this one here
  • #9629
  • #10861
  • #11121
  • #11125

Not included so far:

  • #8763 (Partly, the 3rd which is of dubious value and fails its own mtest too(?!?), the 4th, which depends on it and the 5th, depends on it too (and also on the middle commit from #8766, see below), the 7th and 8th which both need the 3rd etc., the 10th due to merge issues, seems to need one of the left out commits, the 13th,14th and 15th, these need previously left out one)
  • #8766 (Partly, ENG-85, the middle commit)
  • #10564
  • #10596
  • ~#10740~ (reverted by #10816)
  • Fixed output to stdout for backend (due to the huge formatting changes it is very difficult to spot what this really is about)
  • added video to backend CI build
  • ~#10816~ (reverts #10740)
  • #11103 (causes merge conflicts which I'm currently too lazy to check and fix)
  • #11175 (not wanted, would be disabled for a non-Debug build anyhow)
  • #11511 (doesn't apply to 3.x)
  • qDebug instead of qWarning
  • Disabled logging for backend (again?)

There are still some PRs not (yet) migrated into this one here (see above) and also some open issues (i.e. without a PR or otherwise known fix), see #7449.

Jojo-Schmitz avatar Sep 01 '21 16:09 Jojo-Schmitz

I'd appreciate if the original authors of the PRs/commits I've included in this PR would double check whether I did it correctly. In (more ot less) chronological order: @AntonioBL, @SKefalidis, @MarcSabatella, @NozBead, @infojunkie, @worldwideweary, @shoogle, @sidharth-anand, @OmarEmaraDev, @mirabilos, @apricot2012, @lvinken , @tobik, @mattmcclinch, @rettinghaus, @laturetab, @iveshenry18 (although his were all done with git cherry-pick rather than manual like most of the rest, so should be OK, but that does sound like "famos last words", doesn't it ;-)), @wizofaus, @Mystic-Slice, @njvdberg, @dmitrio95, @oktophonie (he gave his OK already), @cbjeukendrup, @lyrra, @PatrickNorton, @RomanPudashkin, @asattely, @Nick-Mazuk (I hope I haven't fogotten anyone), all of which I'd also like to thank for their fixes. Once checked, and if based on 3.x, you may close your PR...

Jojo-Schmitz avatar Sep 02 '21 12:09 Jojo-Schmitz

looks fine!

rettinghaus avatar Sep 02 '21 12:09 rettinghaus

300dababf0b38f8d971b973a1c011133977ef083 looks good

Mystic-Slice avatar Sep 02 '21 16:09 Mystic-Slice

#311792 looks good to me, thanks

infojunkie avatar Sep 02 '21 16:09 infojunkie

#311792 looks good to me, thanks

Thanks, I guess with that you can close your PR #6693

Jojo-Schmitz avatar Sep 02 '21 16:09 Jojo-Schmitz

#6095 looking good thank you

NozBead avatar Sep 02 '21 16:09 NozBead

I have not done a good job keeping up on Discord. Is a 3.6.3 being seriously considered? If so, to me this list is too long by a factor of 10 or so. I see far too much risk that this breaks something and then requires 3.6.4. If we're OK with that, fine. But otherwise, I would prefer to see 3.6.3 just include fixes for the top 10 most serious regressions that have low-risk fixes.

Anyhow, I'd like to understand the context a bit more.

MarcSabatella avatar Sep 02 '21 18:09 MarcSabatella

There is no context and you didn't miss anything on Discord reg. plans for a 3.6.3. This is basically my private pet peeve. Whether or not it gets actually used is not my decision anyway. I will use it myself though, and in exactly this form, shape and size.

Jojo-Schmitz avatar Sep 02 '21 19:09 Jojo-Schmitz

Thanks @Jojo-Schmitz for attending to the PRs that the community has put effort into creating :pray: I hope this work goes into a new public version.

infojunkie avatar Sep 02 '21 19:09 infojunkie

Makes sense. I am personally also interested in the other non-existent 3.6.3 I described - yours feels more like 3.7! - but the nice thing is that work done on "your" build will also be directly relevant should "mine" ever become a thing as well.

Anyhow, near as I can tell the only PR of mine in this list is https://github.com/musescore/MuseScore/pull/7453. Your version of that commit looks as good as mine :-).

MarcSabatella avatar Sep 02 '21 20:09 MarcSabatella

#8721 retested on tip 6b514fd, works fine! And #8818, also tested copying stuff into MMRest, works fine, no regression.

lyrra avatar Sep 02 '21 21:09 lyrra

#8721 retested on tip 6b514fd, works fine! And #8818, also tested copying stuff into MMRest, works fine, no regression.

Thanks, so guess you could close #8721?

Jojo-Schmitz avatar Sep 02 '21 22:09 Jojo-Schmitz

Makes sense. I am personally also interested in the other non-existent 3.6.3 I described - yours feels more like 3.7! - but the nice thing is that work done on "your" build will also be directly relevant should "mine" ever become a thing as well.

Hmmm calling it 3.7 is also a nice option ;-)

Anyhow, near as I can tell the only PR of mine in this list is #7453. Your version of that commit looks as good as mine :-).

Thanks for checking!

Jojo-Schmitz avatar Sep 02 '21 22:09 Jojo-Schmitz

My #8493 is fine in this pull request (I hope I didn't miss something else). And congratulations with the PR #9000 ;-)

dmitrio95 avatar Sep 03 '21 06:09 dmitrio95

Seems OK to me (checked #319776/#7935, #270988/#8059, #321751/#8282, #321753/#8282 and #321960/#8793). The sum of all these PRs indeed feels like 3.7 to me (I wouldn't mind if 3.7 would be created as there are many known bugs with solutions in 3.x and it does not seem to me that 4.0 will be production ready on short notice).

lvinken avatar Sep 05 '21 07:09 lvinken

Thanks for checking and confirming.

Jojo-Schmitz avatar Sep 05 '21 13:09 Jojo-Schmitz

Nice work, @Jojo-Schmitz.

Would love to have 3.7 (some previous releases were rushed and flawed) to round off the series (3.7.1 might happen, if necessary); this also means getting it on Microsoft Store and to better-bridge the eventual transition to Series 4, the latter of which won’t be suitable for everyone upon immediate release.

chenlung avatar Sep 06 '21 10:09 chenlung

Fix #298260: humanize(sloppy) timing when renderChunk This code isn't 3.6.3 worthy, perhaps a preview for 3.7 if cli-parameters are added (to opt-in). As it stands it will perform randomization unconditionally.

lyrra avatar Sep 06 '21 11:09 lyrra

Fix #298260: humanize(sloppy) timing when renderChunk

This code isn't 3.6.3 worthy, perhaps a preview for 3.7 if cli-parameters are added (to opt-in). As it stands it will perform randomization unconditionally.

I know (or ather just found out for sure, but guessed it before too). Just added it here for the artifacts, will remove it again

Jojo-Schmitz avatar Sep 06 '21 11:09 Jojo-Schmitz

Fix #298260: humanize(sloppy) timing when renderChunk

This code isn't 3.6.3 worthy, perhaps a preview for 3.7 if cli-parameters are added (to opt-in). As it stands it will perform randomization unconditionally.

I know (or ather just found out for sure, but guessed it before too). Just added it here for the artifacts, will remove it again

Great! I should've named the branch proof-of-concept :D

lyrra avatar Sep 06 '21 12:09 lyrra

Why the heck does an mtest fail all of a sudden?

46/62 Test #47: tst_palette ......................***Failed    1.49 sec
This plugin does not support createPlatformOpenGLContext!`

Jojo-Schmitz avatar Sep 06 '21 19:09 Jojo-Schmitz

Just to be sure, should this release trigger an import-dialog, when opening an 3.6.2 score?

lyrra avatar Sep 08 '21 18:09 lyrra

No, why?

Jojo-Schmitz avatar Sep 08 '21 19:09 Jojo-Schmitz

No, why? I get an score-migration-dialog popup. It causes a crash also. Both may be due to I'm running from build rather than install. Here's the patch to get through that crash:

diff --git a/mscore/migration/scoremigrationdialog.cpp b/mscore/migration/scoremigrationdialog.cpp
index e5e728dd4..b7173306a 100644
--- a/mscore/migration/scoremigrationdialog.cpp
+++ b/mscore/migration/scoremigrationdialog.cpp
@@ -23,5 +23,5 @@ ScoreMigrationDialog::ScoreMigrationDialog(QQmlEngine* engine, Ms::Score* score)
 void ScoreMigrationDialog::focusInEvent(QFocusEvent* event)
       {
       QQuickView::focusInEvent(event);
-      rootObject()->forceActiveFocus();
+      //rootObject()->forceActiveFocus();
       }

lyrra avatar Sep 08 '21 19:09 lyrra

A score migration dialog should shown only for pre 3.6 scores (and that's what it does for me). And running from build rather than install is just wrong and ever was.

Jojo-Schmitz avatar Sep 08 '21 19:09 Jojo-Schmitz

A score migration dialog should shown only for pre 3.6 scores (Some quick test on scores that perhaps are too primitive to trigger import-dialog.)

  • 3.6.2 can open a 3.01-score / pgmversion 3.1.0, without import-dialog popup.
  • 3.6.3 open a score saved from 3.6.3 does not trigger import-dialog popup.

lyrra avatar Sep 08 '21 19:09 lyrra

It doesn't do that for me (and I just tested this again a minute ago): try running from install rather than build! There's no such thing as too primitive to trigger import-dialog, it just happens based on the file version.

But maybe you can help me finding out with that one mtest, tst_palette, fails? Got to be one of the recent ~10 commits.

Jojo-Schmitz avatar Sep 08 '21 19:09 Jojo-Schmitz

There's no such thing as too primitive to trigger import-dialog, it just happens based on the file version. That's good to know! I'll try to find out what happens from an installed musescore. If nothing else, hooking gdb on that import-dialog function could give som clues.

lyrra avatar Sep 08 '21 19:09 lyrra

Needed this fix to compile on mingw: https://github.com/lyrra/MuseScore/commit/804d9bfaf8698cff6c34a63a80bbb397e0bcf0fd

lyrra avatar Sep 09 '21 06:09 lyrra

Ah, I see, it is not (no longer) tst_palette (which I didn't understand at all), but tst_mxml_io. Should be an easy fix.

Edit: done!

Jojo-Schmitz avatar Sep 09 '21 07:09 Jojo-Schmitz