New score and parts interactions #18722

mike-spa · 2023-07-20T13:28:07Z

Resolves: #16544

This PR provides a number of important, long-awaited new functionalities on the interaction between score and parts. A short summary:

After a part has been created, changes made in the score also affect the part (this is currently not the case for most properties).
Changes made in the part never affect the score. In fact, soon as an item is edited in one part, it will be marked as unlinked and, when selected, will turn orange. Changes made to the score will not affect items that have been unlinked.
Items can be re-linked by using two toggles in a new dedicated section of the property panel which separately control the linking of "Position" and "Appearance" properties. The "Reset shapes and position" command (ctrl+R) also has the effect of re-linking the item properties to the main score.

For a few selected item types (namely Staff text, System text, Clefs, Ottava lines, and frames) a new possibility is now introduced to exclude the selected item from the part (or from the score). This enables to introduce items that may be relevant only in the score (or only in the part), and currently can't be achieved by making the item invisible. When ottavas and clefs are excluded from parts or score, the notation updates accordingly. Importantly, frames (with the exclusion of title frames) default to be excluded. This can also be controlled via a new option in the property panel.

cbjeukendrup · 2023-09-24T19:07:23Z

Found a bug:

Add ottava
Exclude it from parts
Drag a note in a part => the note does not properly follow the mouse. It always stays an octave higher than the mouse. (the laggy-ness is just my computer; that's not the problem.)

Schermopname.2023-09-24.om.20.59.31.mov

cbjeukendrup · 2023-09-24T22:40:40Z

Another questionable bit of behaviour:

Select a system header clef
Enable "exclude from parts"
Disable "exclude from parts" => Now there is suddenly a new clef in the parts, that is not a system header clef but a real clef change! But that clef change is not in the main score.
Enter some notes before and after the system break in question, via the main score
When I then switched to the part score, MuseScore crashed.

So the question is: should the "exclude from parts" checkbox be available for system header clefs? And if yes, how should it behave?

cbjeukendrup

I'm still not entirely trough it, but here are some comments and questions already!

src/notation/internal/masternotation.cpp

src/inspector/view/qml/MuseScore/Inspector/parts/PartsSettings.qml

src/inspector/models/parts/partssettingsmodel.cpp

cbjeukendrup · 2023-09-24T21:58:31Z

src/engraving/dom/property.h

+extern const std::set<Pid>& positionProperties(); ///< All the properties that have to do with the position of an item
+extern const std::set<Pid>& textProperties(); ///< Text body of text items
+extern const std::set<Pid>& appearanceProperties(); ///< All the properties that are not position or text properties


Given the ways these methods are currently used, I'm thinking it might be more elegant to add a propertyGroup field in PropertyMetaData / propertyList. This has some advantages:

it forces each property to be in a category (we'd need to add a "none" category for those special excluded properties though) without the need for that logic in appearanceProperties()

retrieving the category of a property can be done in constant time

the stuff in EngravingItem::relinkPropertiesToMaster may become a bit cleaner in terms of code (just iterate over all properties and check propertyGroup equality) without becoming very different in terms of functionality and performance

it might be a slightly more extensible approach, if we ever introduce other categories

cbjeukendrup · 2023-09-24T23:50:44Z

src/engraving/dom/edit.cpp


        if (score == this) {
            result = newMeasureBase;
+        } else if (!result && score == scores.back()) {
+            result = newMeasureBase;


What is the purpose of this change? It looks like this could cause that the returned MeasureBase* points to a measure that is not part of the Score* on which insertMeasure was called. Is that desired?

src/engraving/dom/engravingitem.cpp

cbjeukendrup · 2023-09-25T00:14:48Z

src/engraving/dom/engravingitem.cpp

+    return PropertyPropagation::NONE;
+}
+
+bool EngravingItem::canBeExcludedFromOtherParts() const


Is there a specific reason to choose for this implementation instead of making this a virtual method? I consider it closely related to manageExclusionFromParts, which is virtual, so I'd personally prefer to keep these methods close to each other.

Mostly because for such simple methods I hate to spread the information across 10 different files: it makes it harder later to find what I need, for no real benefit. I agree that in this case it looks more consistent with manageExclusionFromParts though

cbjeukendrup · 2023-09-25T00:18:18Z

src/engraving/dom/textbase.cpp

-        score()->undo(new ChangeTextProperties(m_cursor, id, v, ps));
-    } else {
+
+    bool isTextSpecificProperty = id == Pid::FONT_STYLE || id == Pid::FONT_FACE || id == Pid::FONT_SIZE || id == Pid::TEXT_SCRIPT_ALIGN;


Here I see a lot more properties listed than in textProperties(). Of course, this is about something different, so the list need not be the same, but still... shouldn't these properties also be in textProperties()? Or otherwise, why only FONT_STYLE and these others not?

cbjeukendrup · 2023-09-25T00:22:45Z

src/engraving/dom/textbase.cpp

+        }
+
+        Score* linkedScore = linkedText->score();
+        TextCursor* linkedCursor = linkedText->cursor();


Am I understanding correctly that this is about setting properties to selected characters of the text (and that that is the reason that TextCursor is being used here)? In that case, can we be certain that the linkedCursor is synchronised with the "original" one? If it's not, I'm afraid that could give quite strange results, couldn't it?

No, this gets called also when changing the property of the "full" text item, without necessarily entering edit mode and therefore having an actual cursor. linkedCursor is a default-initialized one, it's not actually linked to the present cursor. When editing individual characters things get more complicated and indeed there are still imperfections, but that's a minor case that can (and will) be smoothed out later. The format of the full text works fine as expected

mike-spa · 2023-09-25T07:00:38Z

Found a bug:

Add ottava

Exclude it from parts

Drag a note in a part => the note does not properly follow the mouse. It always stays an octave higher than the mouse. (the laggy-ness is just my computer; that's not the problem.)

@cbjeukendrup not a blocker. There's a plan to open an issue with a checklist of quirks and refinements as soon as we merge this. Let's add it there and I'll work through it together with with the rest.

mike-spa · 2023-09-25T07:02:36Z

Another questionable bit of behaviour:

Select a system header clef

Enable "exclude from parts"

Disable "exclude from parts" => Now there is suddenly a new clef in the parts, that is not a system header clef but a real clef change! But that clef change is not in the main score.

Enter some notes before and after the system break in question, via the main score

When I then switched to the part score, MuseScore crashed.

So the question is: should the "exclude from parts" checkbox be available for system header clefs? And if yes, how should it behave?

@cbjeukendrup also not a blocker, same thing as previous one. Btw no, for header clefs the "exclude from part/score" checkbox shouldn't be available, and I was sure it wasn't, so I'll have to check what's happening.

mike-spa · 2023-09-25T11:36:11Z

@cbjeukendrup all done! Let's merge soon if we can 👍

cbjeukendrup · 2023-09-25T23:47:33Z

src/engraving/dom/excerpt.cpp

    MeasureBase* scoreMeasure = score->first();

-    if (!scoreMeasure || !scoreMeasure->isVBox()) {
+    if ((!scoreMeasure || !scoreMeasure->isVBox()) && !masterMeasure->excludeFromOtherParts()) {


Note: this causes a crash when creating parts for a score that contains no measures nor boxes. Although 4.1.1 also shows some funny behaviour in that situation, it does not crash, so this will be a regression.

(just making a note of it so that we can log/fix it later)

The large diffs are due to: - Increased file version to 4.20 - Introduction of the new property "Exclude from other parts". The property now defaults to true for frames, but we set it to false for frames done pre-4.2 for compatibility.

(a little bit)

cbjeukendrup

🥳🥳🥳🥳🥳

cbjeukendrup · 2023-09-26T01:37:08Z

@mike-spa I'll leave the honour of pressing the big green button to you! I've already rebased #19127 on top of this; that looks a bit messy now, but it will be alright.

The procedure is now as follows:

Merge this PR to master and refresh your local master branch
Pull the latest version of #19127 to your computer
Rebase that on top of master -> there should be no conflicts, because I've already fixed those
Push -> now #19127 should only contain the commits that belong there
Merge #19127

(Don't wait for me tomorrow morning, I'll probably be late!)

mike-spa requested review from oktophonie and bkunda July 20, 2023 13:28

mike-spa force-pushed the partScorePropertyLinking branch 4 times, most recently from 7d926f9 to 20eb630 Compare July 27, 2023 15:56

mike-spa force-pushed the partScorePropertyLinking branch 4 times, most recently from f72747d to 27517cb Compare August 1, 2023 13:19

mike-spa force-pushed the partScorePropertyLinking branch from 27517cb to 856b437 Compare August 8, 2023 08:17

mike-spa force-pushed the partScorePropertyLinking branch 2 times, most recently from 70df530 to 74da07b Compare August 18, 2023 16:06

mike-spa force-pushed the partScorePropertyLinking branch 9 times, most recently from 424e4db to f6c7b02 Compare August 29, 2023 07:27

mike-spa changed the title ~~Part score property linking~~ New score and parts interactions Aug 29, 2023

mike-spa marked this pull request as ready for review August 29, 2023 08:16

mike-spa requested a review from RomanPudashkin August 29, 2023 08:16

mike-spa force-pushed the partScorePropertyLinking branch from f6c7b02 to 7d453d0 Compare August 31, 2023 10:00

cbjeukendrup mentioned this pull request Sep 4, 2023

For files created in development versions, the content does not always match the file format version number #17846

Open

cbjeukendrup force-pushed the partScorePropertyLinking branch 2 times, most recently from fc9b1e6 to 9554aa1 Compare September 19, 2023 10:11

mike-spa force-pushed the partScorePropertyLinking branch from 9554aa1 to c2c2c1d Compare September 20, 2023 09:33

cbjeukendrup mentioned this pull request Sep 21, 2023

Introducing new options for ties, and a general overhaul of tie layout #19127

Merged

cbjeukendrup force-pushed the partScorePropertyLinking branch 2 times, most recently from 34c65c6 to 375421e Compare September 25, 2023 00:27

cbjeukendrup reviewed Sep 25, 2023

View reviewed changes

mike-spa and others added 13 commits September 26, 2023 01:51

New score and parts interactions

9f3d216

Fix unit tests

52c9873

The large diffs are due to: - Increased file version to 4.20 - Introduction of the new property "Exclude from other parts". The property now defaults to true for frames, but we set it to false for frames done pre-4.2 for compatibility.

New unit tests

508465f

Add missing include

de064db

Code review fixes - clean up PropertyToggle

7a60376

Code review fixes - simplify PartsSettings

95ec349

Code review fixes - slightly simplify tests

a64c014

Code review fixes - update code style in EngravingConfiguration

1cad497

Code review fixes - a few more small ones

c7d2055

Code review fixes - Move property group into PropertyMedaData

e79f65d

Code review fixes - Move method from MeasureBase to Box class

8c23bf4

Code review fixes - more corrections

c35963f

Last small code review fixes

c51ec22

cbjeukendrup force-pushed the partScorePropertyLinking branch from 15737ad to 6342655 Compare September 26, 2023 01:10

Const perfectionism

64cbb5b

(a little bit)

cbjeukendrup force-pushed the partScorePropertyLinking branch from 6342655 to 64cbb5b Compare September 26, 2023 01:13

cbjeukendrup approved these changes Sep 26, 2023

View reviewed changes

mike-spa merged commit 62fe329 into musescore:master Sep 26, 2023
11 checks passed

cbjeukendrup mentioned this pull request Sep 30, 2023

Refinements checklist for the new Parts/Score interaction feature #19527

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New score and parts interactions #18722

New score and parts interactions #18722

mike-spa commented Jul 20, 2023 •

edited by oktophonie

cbjeukendrup commented Sep 24, 2023

cbjeukendrup commented Sep 24, 2023

cbjeukendrup left a comment

cbjeukendrup Sep 24, 2023

cbjeukendrup Sep 24, 2023

cbjeukendrup Sep 25, 2023

mike-spa Sep 25, 2023

cbjeukendrup Sep 25, 2023

cbjeukendrup Sep 25, 2023

mike-spa Sep 25, 2023

mike-spa commented Sep 25, 2023

mike-spa commented Sep 25, 2023

mike-spa commented Sep 25, 2023

cbjeukendrup Sep 25, 2023

cbjeukendrup left a comment

cbjeukendrup commented Sep 26, 2023

New score and parts interactions #18722

New score and parts interactions #18722

Conversation

mike-spa commented Jul 20, 2023 • edited by oktophonie

cbjeukendrup commented Sep 24, 2023

cbjeukendrup commented Sep 24, 2023

cbjeukendrup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mike-spa commented Sep 25, 2023

mike-spa commented Sep 25, 2023

mike-spa commented Sep 25, 2023

Choose a reason for hiding this comment

cbjeukendrup left a comment

Choose a reason for hiding this comment

cbjeukendrup commented Sep 26, 2023

mike-spa commented Jul 20, 2023 •

edited by oktophonie