Skip to content

Conversation

@XiaoMigros
Copy link
Contributor

This feature adds a purpose to the 'minimum distance' property for rests, controlling their vertical avoidance distance to chords or other rests. The distance is added onto the existing pre-calculated offset:
grafik

Here's an example with a large minimum distance of 2sp. The 'align with other rests' property has also offset the remaining rests in this example:

grafik

Crucially, this option allows for negative minimum distances, meaning it's possible to fix rests to a given line using a low minimum distance as well as the 'offset' property. This could improve MusicXML import as seen in #24971.

  • 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)

clearance = shape2.verticalClearance(shape1);
}
double margin = clearance - minRestToRestClearance;
double minDistance = (rest1->minDistance().val() + rest2->minDistance().val()) * lineDistance;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

or std::max(rest1->minDistance().val(), rest2->minDistance().val()) * lineDistance;

@cbjeukendrup cbjeukendrup requested a review from mike-spa November 4, 2025 18:48
@mike-spa
Copy link
Contributor

mike-spa commented Nov 5, 2025

Minimum distance is one of the messiest things we have in our codebase. It's used in different ways, it means different things for different items, and for many of them means absolutely nothing and does nothing. It's one of those things that I'd like to nuke entirely and replace with something that makes more sense, at some point. So I'm a bit hesitant to introduce a new use for it.

Having said that, I actually do like this. It's certainly better than having it do nothing, and it is consistent with what it does for other items. So I'm inclined to approve this, if @its-not-nice approves the functionality.

@mike-spa mike-spa requested a review from its-not-nice November 5, 2025 11:24
@its-not-nice
Copy link
Contributor

@XiaoMigros What real problem is this intended to solve?

@XiaoMigros
Copy link
Contributor Author

The same problem that minimum distance solves e.g. for text elements: It adds the ability to configure vertical padding between elements, something I think should be available for multi-voice rests. I prefer working with minimum distance over offset as it scales better when further adjustments to the score are made. Introducing minimum distance for rests also has the added benefit of mitigating the above linked issue.

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.

3 participants