Skip to content

Conversation

@lvinken
Copy link
Contributor

@lvinken lvinken commented Oct 29, 2025

Resolves: #30806
Resolves: #30807

Fix various forms of corrupted measures resulting from importing common TablEdit files.

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)


// Fill one gap (tstart - tend) in this track in this measure with rest(s).

static void fillGap(Measure* measure, track_idx_t track, const Fraction& tstart, const Fraction& tend)
Copy link
Member

Choose a reason for hiding this comment

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

  • Measure has a method fillGap, maybe you can use that, but you do need to add a parameter to make the rests invisible
  • If you prefer using a dedicated static function, then you could perhaps still benefit from the toRhythmicDurationList utility, or maybe just toDurationList, both are in durationtype.h.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer not to use a dedicated function, but could not find a suitable one ... Note that I do not consider rest visibility important in gaps.


// Fill gaps in first voice of every staff in this measure for this part with rest(s).

static void fillGapsInFirstVoices(Measure* measure, Part* part)
Copy link
Member

Choose a reason for hiding this comment

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

It seems this method doesn't really depend on part, so maybe it's also possible to just iterate over all staves of the score in one go, instead of per part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course.

@lvinken
Copy link
Contributor Author

lvinken commented Oct 30, 2025

Hi Casper, thanks for reviewing. Function Measure::fillGaps() cannot be used as-is, as it is private. I tried to use Measure::checkMeasure) instead which is public. This leads to tuplets failing, which I do not yet understand. Pushed the change anyway to get feedback. Leon.

@cbjeukendrup
Copy link
Member

Sorry for not noticing fillGap is private. Perhaps we should then opt for using a dedicated method here. One could argue that Measure::fillGap mainly exists to fix up old corrupted MSCZ files, which might make it not great for reusing in TablEdit import, even while it currently happens to do the same thing.

I notice Measure::check has some logic for skipping tuplets, can the tuplet problems you mention be related to that?

Anyway, let's use a dedicated method again, but perhaps with toRhythmicDurationList to determine which rests are needed.

@lvinken lvinken force-pushed the import_tef_fix_measure_issues branch from a56cad6 to f641449 Compare November 1, 2025 16:23
@lvinken
Copy link
Contributor Author

lvinken commented Nov 1, 2025

Reworked using toDurationList(), as that is somewhat simpler to use. Indeed it seems Measure::check() does not work correctly for truplets.

@cbjeukendrup cbjeukendrup merged commit 40b0a4a into musescore:master Nov 6, 2025
12 checks passed
@Jojo-Schmitz Jojo-Schmitz mentioned this pull request Nov 6, 2025
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.

[TablEdit import] measures with gaps in voice 1 imported as corrupt [TablEdit import] pickup measures imported as corrupt

2 participants