Skip to content

Conversation

@mike-spa
Copy link
Contributor

Resolves: #17368

Copy link
Contributor

@miiizen miiizen left a comment

Choose a reason for hiding this comment

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

Lovely stuff, very neat!

default:
LOGE() << "Not handled: " << item->typeName();
IF_ASSERT_FAILED(false) {
/*IF_ASSERT_FAILED(false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep some sort of assert here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops that was committed by mistake, good catch

const double y1 = ldata->bbox().top() + halfLineWidth;
const double y2 = ldata->bbox().bottom() - halfLineWidth;

double w = item->hookLength().toMM(item->spatium());
Copy link
Contributor

Choose a reason for hiding this comment

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

Wherever we want a spatium value to scale with staff size, we should use absoluteFromSpatium. eg. item->absoluteFromSpatium(item->hookLength()).
(All uses of toMM are due an audit anyway)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Excellent point, thanks

@mike-spa mike-spa force-pushed the arpeggioBracketImprovements branch from eb52c43 to 014933b Compare December 18, 2025 10:38
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.

Add vertical lines (to create resizable brackets etc.)

2 participants