Skip to content

Refactor: move mandatory migrations into EngravingCompat#32847

Draft
cbjeukendrup wants to merge 5 commits intomusescore:masterfrom
cbjeukendrup:mandatory-migrations-to-engraving-compat
Draft

Refactor: move mandatory migrations into EngravingCompat#32847
cbjeukendrup wants to merge 5 commits intomusescore:masterfrom
cbjeukendrup:mandatory-migrations-to-engraving-compat

Conversation

@cbjeukendrup
Copy link
Copy Markdown
Collaborator

No description provided.

@cbjeukendrup cbjeukendrup force-pushed the mandatory-migrations-to-engraving-compat branch from cd8f780 to bb8ab16 Compare March 29, 2026 22:54
It is already called in `MscLoader::loadMscz`, so no need to call it in individual reader implementations.
`Score`’s implementation of these methods just its calls `MasterScore`’s version.

Also, `scoreList` includes the master score itself, so no need to call `createPaddingTable` separately.
It serves no purpose and meta tags are not the place for `mscVersion`.
@cbjeukendrup cbjeukendrup force-pushed the mandatory-migrations-to-engraving-compat branch from bb8ab16 to d6c966f Compare March 29, 2026 23:07
@cbjeukendrup
Copy link
Copy Markdown
Collaborator Author

The VTests changes are expected, because now they represent how the score will look when opening it in the UI (regardless of selected migration options).

if (mscVersion < 300) {
resetAllElementsPositions(score);
resetCrossBeams(score);
needRelayout = true;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Question to the team: do we think it's necessary to do this after initial layout, requiring a second layout? Or is it not necessary to do a layout before it?

fullCross only seems to be updated at layout time, that's why I was thinking it would be necessary. But it is not entirely clear to me whether a layout has already been done by the time that IProjectMigrator does its job, where these resets were performed previously.

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.

1 participant