Fix 32949: layout for non solid slurs and ties#32987
Open
rettinghaus wants to merge 1 commit intomusescore:masterfrom
Open
Fix 32949: layout for non solid slurs and ties#32987rettinghaus wants to merge 1 commit intomusescore:masterfrom
rettinghaus wants to merge 1 commit intomusescore:masterfrom
Conversation
📝 WalkthroughWalkthroughThis change modifies thickness handling for tie and slur rendering in the scoring engine. The midThickness offset is now applied only when the style type is Solid. In the computeBezier methods for both TieSegment and SlurSegment, thickness values default to zero and only receive values from midThickness for solid styles. Additionally, shape fill thickness approximation now uses midThickness directly for non-solid styles instead of applying a sinusoidal taper, while solid styles retain the previous tapered formula. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/engraving/rendering/score/slurtielayout.cpp`:
- Around line 3226-3229: The collision-width calculation currently always uses
midThickness (derived from Sid::slurMidWidth) even for non-solid slur styles;
change the logic in the block around approxThicknessAtPercent so that when
slurTieSeg->slurTie()->styleType() is not Solid you derive the base width from
Sid::slurDottedWidth (the same style value used at paint time) instead of
midThickness, then apply the same percent-based tapering; keep using
midThickness only for Solid styles so collision geometry matches visible stroke
width.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8de60889-935e-4e38-8426-677115bd2f44
📒 Files selected for processing (1)
src/engraving/rendering/score/slurtielayout.cpp
Comment on lines
+3226
to
+3229
| double approxThicknessAtPercent = midThickness; | ||
| if (slurTieSeg->slurTie()->styleType() == SlurStyleType::Solid) { | ||
| approxThicknessAtPercent *= (1 - 2 * std::abs(0.5 - percent)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Paint-time width sources for non-solid slur/tie styles
rg -n -C3 --type=cpp 'SlurStyleType::(Dotted|Dashed|WideDashed)|Sid::slurDottedWidth|Sid::slurWideDashedWidth' src/engraving
# 2) Layout-time thickness source in this change
rg -n -C3 --type=cpp 'approxThicknessAtPercent|midThickness|styleType\(\) == SlurStyleType::Solid' src/engraving/rendering/score/slurtielayout.cpp
# 3) Style/UI plumbing: confirm whether global slur/tie thickness updates dotted/wide-dashed widths too
rg -n -C3 --type=cpp --type=h 'slur.*thickness|tie.*thickness|Sid::slurMidWidth|Sid::slurEndWidth|Sid::slurDottedWidth|Sid::slurWideDashedWidth' srcRepository: musescore/MuseScore
Length of output: 21169
🏁 Script executed:
#!/bin/bash
# 1. Find computeMidThickness implementation
rg -n -A20 'void.*computeMidThickness\(' src/engraving/rendering/score/slurtielayout.cpp
# 2. Check if slurDottedWidth is consulted during layout
rg -n 'slurDottedWidth|Sid::slurDottedWidth' src/engraving/rendering/score/slurtielayout.cpp
# 3. Check style change invalidation for slurs
rg -n 'onStyleChanged|invalidate' src/engraving/dom/slur.cpp | head -20Repository: musescore/MuseScore
Length of output: 1322
🏁 Script executed:
#!/bin/bash
# 1. Check if slurs are invalidated when style changes
rg -n -B5 -A10 'onStyleChanged' src/engraving/dom/slur.cpp
# 2. Check if slurDottedWidth is initialized/coupled anywhere
rg -n 'slurDottedWidth' src/engraving/style/style.cpp | head -20
# 3. Verify whether shape is recalculated on style change
rg -n 'updateLayout|invalidateLayout' src/engraving/dom/slurtie.h
rg -n 'styleChanged' src/engraving/dom/slurtie.hRepository: musescore/MuseScore
Length of output: 231
🏁 Script executed:
#!/bin/bash
# 1. Check SlurTieSegment class - find where midThickness is used/initialized
rg -n 'midThickness' src/engraving/dom/slurtie.h
# 2. Check if there's any special handling for non-solid in layout
rg -n -B5 -A5 'styleType.*Solid' src/engraving/rendering/score/slurtielayout.cpp
# 3. Verify if slurDottedWidth should affect collision box
rg -n 'collision|Shape|bbox' src/engraving/rendering/score/slurtielayout.cpp | grep -A2 -B2 '3216\|3225\|3230'Repository: musescore/MuseScore
Length of output: 2459
🏁 Script executed:
#!/bin/bash
# 1. Check endWidth() and midWidth() implementations
rg -n -A3 'endWidth\(\)|midWidth\(\)' src/engraving/dom/slur.cpp
# 2. Check if slurMidWidth or slurEndWidth have any relationship to slurDottedWidth
rg -n 'slurMidWidth|slurEndWidth' src/engraving/style/style.cpp
# 3. Check git history or comments about non-solid style width handling
rg -n 'non-solid|dotted.*width|dashed.*width' src/engraving/rendering/score/slurtielayout.cppRepository: musescore/MuseScore
Length of output: 601
🏁 Script executed:
#!/bin/bash
# 1. Search for any comments or documentation about non-solid width handling
rg -n -B3 'Dotted|Dashed|WideDashed' src/engraving/rendering/score/slurtielayout.cpp
# 2. Check if slurDottedWidth is supposed to be synced with slurMidWidth
rg -n 'slurDottedWidth.*slurMidWidth|slurMidWidth.*slurDottedWidth' src
# 3. Look at default style values to see if they're related
rg -n 'Sid::slurMidWidth|Sid::slurDottedWidth' src/engraving/style/styledef.cppRepository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
#!/bin/bash
# 1. Find where style defaults are defined
fd -type f 'styledef\|stylev2\|style.*default' src/engraving/style/
# 2. Search for any initialization of slurDottedWidth to understand its purpose
rg -n 'slurDottedWidth' src/engraving/style/
# 3. Check if there's any comment about width differences between solid and non-solid
rg -n -B10 'fillShape' src/engraving/rendering/score/slurtielayout.cpp | head -30Repository: musescore/MuseScore
Length of output: 1841
🏁 Script executed:
#!/bin/bash
# 1. Check the default values of slurMidWidth and slurDottedWidth
rg -n 'styleDef.*slurMidWidth|styleDef.*slurDottedWidth' src/engraving/style/styledef.cpp
# 2. Look at the actual fillShape implementation to confirm it uses midThickness
rg -n -A25 'void SlurTieLayout::fillShape' src/engraving/rendering/score/slurtielayout.cpp
# 3. Check if there's any relationship or coupling in other files
rg -n 'slurDottedWidth|slurMidWidth' src/engraving/ | grep -E '(couple|sync|relation|same)'Repository: musescore/MuseScore
Length of output: 1601
Non-solid slur/tie collision shape uses wrong width source.
The shape collision box for non-solid styles (Dotted/Dashed/WideDashed) is based on midThickness (computed from Sid::slurMidWidth, default 0.21 sp), while the paint-time stroke uses Sid::slurDottedWidth (default 0.10 sp). These are independent style values with no synchronization. Updating one does not update the other, causing collision geometry and visible width to diverge.
Solid slurs are unaffected because both shape and paint derive from the same midWidth value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/engraving/rendering/score/slurtielayout.cpp` around lines 3226 - 3229,
The collision-width calculation currently always uses midThickness (derived from
Sid::slurMidWidth) even for non-solid slur styles; change the logic in the block
around approxThicknessAtPercent so that when slurTieSeg->slurTie()->styleType()
is not Solid you derive the base width from Sid::slurDottedWidth (the same style
value used at paint time) instead of midThickness, then apply the same
percent-based tapering; keep using midThickness only for Solid styles so
collision geometry matches visible stroke width.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: #32949