Skip to content

Group brackets#32815

Open
mike-spa wants to merge 2 commits intomusescore:masterfrom
mike-spa:groupBrackets
Open

Group brackets#32815
mike-spa wants to merge 2 commits intomusescore:masterfrom
mike-spa:groupBrackets

Conversation

@mike-spa
Copy link
Copy Markdown
Contributor

No description provided.

@miiizen
Copy link
Copy Markdown
Contributor

miiizen commented Mar 30, 2026

It's slightly annoying that bracketItem.cpp/h is camel case! We can sort this out separately if you'd prefer though.

ldata->setShape(shape);

if (!item->text()) {
const_cast<Bracket*>(item)->setText(new Text(const_cast<Bracket*>(item), TextStyleType::GROUP_BRACKET));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
const_cast<Bracket*>(item)->setText(new Text(const_cast<Bracket*>(item), TextStyleType::GROUP_BRACKET));
Text* bracketText = new Text(const_cast<Bracket*>(item), TextStyleType::GROUP_BRACKET);
bracketText->setGenerated(true);
const_cast<Bracket*>(item)->setText(bracketText);

You can then remove the Text::isEditable override.

{
initElementStyle(&defaultStyle);

setSelectable(!parent->isBracket());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can add the GROUP_BRACKET text style type to styleIsSelectable instead of this line. (Unless in future we want users to be able to change the text style of individual instances of bracket text)

// (the second case happens at least when the bracket is initially dropped)
bool notHidden = ctx.conf().styleB(Sid::alwaysShowBracketsWhenEmptyStavesAreHidden)
? (staffIdx1 <= staffIdx2) : (staffIdx1 < staffIdx2) || (b->span() == 1 && staffIdx1 == staffIdx2);
if (notHidden) { // set vert. pos. and height to visible spanned staves
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be better to check the inverse of this and continue to skip layout completely? (Unless we need to lay out brackets with 0 height for horizontal spacing?)

staffIdx += nstaves;
}

for (staff_idx_t staffIdx = 0; staffIdx < system->staves().size(); ++staffIdx) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Compiler warning

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.

4 participants