Skip to content

Improve parenthesis layout performance#33358

Merged
miiizen merged 1 commit into
musescore:mainfrom
miiizen:paren-performance
May 15, 2026
Merged

Improve parenthesis layout performance#33358
miiizen merged 1 commit into
musescore:mainfrom
miiizen:paren-performance

Conversation

@miiizen
Copy link
Copy Markdown
Contributor

@miiizen miiizen commented May 12, 2026

This PR makes layout of parentheses quicker by reducing the number of rectangles in their shape. This cuts segment spacing time in a parenthesis-heavy test score by ~75% (10s to 2.5s)

Screenshot 2026-05-12 at 11 40 24

@miiizen miiizen requested a review from mike-spa May 12, 2026 10:42
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 12, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ef7fe037-2e05-46e2-800f-ee22a4f8e6cd

📥 Commits

Reviewing files that changed from the base of the PR and between 7aa1829 and cbaaef9.

📒 Files selected for processing (1)
  • src/engraving/rendering/score/parenthesislayout.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/engraving/rendering/score/parenthesislayout.cpp

📝 Walkthrough

Walkthrough

This change reduces the discretization used when generating filled composite parenthesis shapes: nbShapes is now round(2.0 * heightInSpatium) clamped to 3..10 instead of round(5.0 * heightInSpatium) clamped to 20..50, producing fewer shape segments in the fill loop.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description lacks required sections from the template including issue reference, CLA checkbox, and commit message verification checklist. Add the missing template sections: issue reference (Resolves: #NNNNN), CLA checkbox, and verification checklist items.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: reducing parenthesis layout computation complexity to improve performance.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@RomanPudashkin RomanPudashkin added the performance Performance issues (e.g. high CPU usage) label May 14, 2026
@miiizen miiizen force-pushed the paren-performance branch from 7aa1829 to cbaaef9 Compare May 15, 2026 09:51
@miiizen miiizen merged commit 3e8cdc9 into musescore:main May 15, 2026
13 of 14 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Needs porting in MuseScore Studio 4.7.1 May 15, 2026
@Jojo-Schmitz Jojo-Schmitz mentioned this pull request May 21, 2026
@RomanPudashkin RomanPudashkin moved this from Needs porting to Done in MuseScore Studio 4.7.2 May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance issues (e.g. high CPU usage)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants