Skip to content

Fix lyric line visibility#32944

Merged
miiizen merged 4 commits intomusescore:4.7from
miiizen:ll-visibility
Apr 9, 2026
Merged

Fix lyric line visibility#32944
miiizen merged 4 commits intomusescore:4.7from
miiizen:ll-visibility

Conversation

@miiizen
Copy link
Copy Markdown
Contributor

@miiizen miiizen commented Apr 8, 2026

Resolves: #32926

Lyric line's visibility can now be set separately from its lyric. Setting the visibility setting for lyrics still changes the lyric line's visibility, but this can be overriden.

Summary by CodeRabbit

  • Bug Fixes

    • Lyrics visibility now stays consistent with their separator elements; legacy scores have lyric-line visibility reconciled during compatibility processing, triggering layout refresh when needed.
  • Chores

    • Added translation support to the percussion shortcut editor.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 27610337-bc80-496f-8267-700d5449e817

📥 Commits

Reviewing files that changed from the base of the PR and between 3626608 and 90b5c6a.

📒 Files selected for processing (3)
  • src/engraving/compat/engravingcompat.cpp
  • src/engraving/compat/engravingcompat.h
  • vtest/scores/lyrics-33.mscz

📝 Walkthrough

Walkthrough

Lyric visibility changes are propagated from Lyrics::setProperty() to separator spanners; layout no longer mirrors visibility during lyrics-line creation; a post-layout compat step syncs existing LyricsLine spanners' visibility; and a missing translation.h include was added.

Changes

Cohort / File(s) Summary
Lyrics property handling
src/engraving/dom/lyrics.cpp
Handle Pid::VISIBLE explicitly: call setVisible(...) on Lyrics and, if present, on separator()->setVisible(...).
Layout initialization cleanup
src/engraving/rendering/score/lyricslayout.cpp
Remove explicit mirroring of parent Lyrics visible state onto LyricsLine separator during createOrRemoveLyricsLine; retain styleChanged() call.
Post-layout compatibility
src/engraving/compat/engravingcompat.cpp, src/engraving/compat/engravingcompat.h
Add setLyricLineVisibility(MasterScore*), call it from doPostLayoutCompatIfNeeded for mscVersion < 470; iterate unmanaged spanners, sync LyricsLine visibility with lyrics()->visible(), return whether changes occurred to trigger relayout.
Missing include
src/notationscene/qml/MuseScore/NotationScene/editpercussionshortcutmodel.cpp
Add #include "translation.h" to support existing qtrc(...) usage.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as Property Setter
    participant Lyrics as Lyrics Item
    participant Sep as Lyrics Separator (Spanner)

    Caller->>Lyrics: setProperty(Pid::VISIBLE, bool)
    Lyrics->>Lyrics: setVisible(bool)
    alt separator exists
        Lyrics->>Sep: setVisible(bool)
    end
Loading
sequenceDiagram
    participant EngravingCompat as Compat
    participant MasterScore as MasterScore
    participant Score as Score
    participant Spanners as UnmanagedSpanners
    participant LL as LyricsLine
    participant Lyrics as Lyrics

    EngravingCompat->>MasterScore: setLyricLineVisibility(master)
    MasterScore->>Score: for each score
    Score->>Spanners: iterate unmanagedSpanners
    Spanners->>LL: select isLyricsLine()
    LL->>Lyrics: ll->lyrics()
    alt visibility differs
        LL->>LL: setVisible(lyrics->visible())
        LL-->>EngravingCompat: mark changed
    end
    EngravingCompat-->>Caller: return changed?
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

vtests

Suggested reviewers

  • mike-spa
  • RomanPudashkin
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title "Fix lyric line visibility" directly addresses the main objective: making lyric line visibility independent from lyric visibility while maintaining the default linkage.
Description check ✅ Passed The description includes the issue reference and explains the key functionality change, but lacks complete template details such as CLA signature confirmation and testing information.
Linked Issues check ✅ Passed The pull request successfully addresses issue #32926 by implementing independent visibility control for lyric lines while preserving default linkage to lyric visibility.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing lyric line visibility; the addition of translation.h include is a supporting change with no unrelated modifications.

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

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

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

@miiizen miiizen linked an issue Apr 8, 2026 that may be closed by this pull request
4 tasks
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8c72c9ab-102c-4233-9f0a-6d0e320d6ccd

📥 Commits

Reviewing files that changed from the base of the PR and between 88530fe and 3626608.

📒 Files selected for processing (3)
  • src/engraving/compat/engravingcompat.cpp
  • src/engraving/compat/engravingcompat.h
  • vtest/scores/lyrics-33.mscz

@davidstephengrant
Copy link
Copy Markdown
Contributor

A couple of remaining possible refinements raised separately as #32961.

Tested and approved on Ubuntu 24.04.4 LTS.

@miiizen miiizen merged commit 1a13659 into musescore:4.7 Apr 9, 2026
12 of 13 checks passed
@miiizen miiizen mentioned this pull request Apr 10, 2026
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.

Visible property does not work for lyric extenders or hyphens

3 participants