isc / isc/arabesque

Bump OSMD when the next release ships, and drop the workarounds it makes redundant

Open
#349 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
0
Forks
0
Avg merge
5h 11m
Merged PRs (30d)
123

Description

Three upstream commits of ours are merged on OSMD's develop but not in any release yet — the vendored bundle is stock 2.1.2 (released 2026-08-06). When the next OSMD release ships, bumping it lets us delete two workarounds outright — three if #1731 (below) is merged by then. This issue is the investigation, so it doesn't have to be redone.

Line numbers are as of a3196e8.

What the release brings

Upstream What it does
#17239d288d69 GraphicalLabel.sourceNote: every fingering label carries a reference to its note
#1730e28cc5d2 draws the notehead of a hidden unison note, and counts it in its tuplet
maintainer follow-up → 26f89749 that notehead takes the colour of the visible note it shares its head with; adds Note.visibleUnisonNoteSharingNotehead(), which returns the partner note
#1731open, not merged yet findTieGraphicalNoteFromNote() returns the tie's own note before a same-pitch one, so a tie starting or ending at a notehead shared with a hidden unison note is drawn instead of skipped

Check they are all in before starting (#1731 may land in a later release than the other three — then do section 4 on its own when it does):

gh release list --repo opensheetmusicdisplay/opensheetmusicdisplay --limit 3

1. Bump the bundle

The release is stock — no local carry any more — so the built opensheetmusicdisplay.min.js from the release (or from npm) can be dropped in directly; no need to rebuild from ~/Code/opensheetmusicdisplay. Copy it to public/vendor/opensheetmusicdisplay.<version>.min.js and update the single <script src=…> in public/score.html:53. The filename is the cache-buster — never reuse the old one. public/js/musicxml.js uses the global opensheetmusicdisplay object, not the filename. No service-worker list to touch: the precache list is generated from public/ at deploy time.

Sanity check after loading a score: osmd.Version reports the new version.

2. Unison noteheads — public/js/musicxml.js

fixUpInvisibleNotes() (musicxml.js:322) does two things to a note hidden by print-object="no": it clears pointer-events so clicks fall through, and it inks the notehead back in. The inking becomes OSMD's job.

  • Delete hiddenPath.setAttribute('fill', visiblePath.getAttribute('fill')) (musicxml.js:357). OSMD now inks that head itself, with the visible partner's colour — literally what this line was doing.
  • Keep visibleHead.appendChild(hiddenPath) (musicxml.js:358). This one is ours, not a bug workaround: the head moves into the visible note's notehead group so our CSS "played/active" colouring paints both heads under the single keypress that validates the pitch.
  • Replace visibleUnisonNotehead() (musicxml.js:377-389) with the new API — it walks the sibling voice entries by hand to find the same pitch, which is exactly what note.visibleUnisonNoteSharingNotehead() returns:
    const partner = note.visibleUnisonNoteSharingNotehead?.()
    const visibleHead = partner && svgNotehead({ note: partner, noteheadIndex: /* its index in its voice entry */ })
    
    svgNotehead() needs the note's index inside its voice entry, so keep that lookup (other.Notes.indexOf(partner)).
  • Probably delete areSideBySide() (musicxml.js:394-398) and the pairs.filter(areSideBySide) at musicxml.js:347. It exists because inking a merged head would have overprinted the visible one; now that OSMD inks it with the partner's colour, reparenting a merged head is a no-op that costs nothing and even keeps our colouring consistent. It also guards against unmeasurable heads (detached/hidden SVG) — cheap to keep if that feels safer, but then it stays the only reason getBoundingBoxesForNotes() is called here, which is a forced layout read per render. Decide at implementation time; measure before keeping it.

unisonNoteheadPair() (musicxml.js:367-373) still has a job — finding the pair to reparent — but its visiblePath field only served the fill, so it can return { hiddenPath, visibleHead }.

Also update the block comment at musicxml.js:315-321, which says the fill choice "belongs upstream too, in the same routine that already spares the stem" — that is now done.

3. Fingering labels — public/js/fingeringEditor.js

This is the bigger win. Today we recover which staffEntry.FingeringEntries[i] belongs to which note by re-implementing OSMD's calculateFingerings() stacking order and pairing by index. The comment says the liability out loud:

Mirrors calculateFingerings() in OSMD's MusicSheetCalculator — keep in sync when the vendored bundle moves.

With GraphicalLabel.sourceNote, the label says which note it belongs to. Both call sites collapse:

  • findFingeringEntry() (fingeringEditor.js:235-258): instead of ordering the fingerings and taking FingeringEntries[finalIndex], find the entry whose sourceNote === targetNoteData.note.
  • alignFingeringLabelsToNoteheads() (fingeringEditor.js:266-298): the loop pairs entries[i] with fingerings[i]; it becomes for (const entry of entries) with const note = entry.sourceNote.

Then these become dead and go:

  • orderedFingeringsForStaffEntry() (fingeringEditor.js:201-231) — the mirrored heuristics
  • collectFingeringsFromStaffEntry() (fingeringEditor.js:181)
  • findTopNoteInStaffEntry() (fingeringEditor.js:168)
  • halfToneOf (fingeringEditor.js:165)
  • isFingeringsPlacedAbove() (fingeringEditor.js:194)
  • staffEntryContainsNote() (fingeringEditor.js:155) — only used to pick the staff entry in findFingeringEntry(), which the sourceNote match subsumes

Watch out: FingeringEntries holds GraphicalLabels, and it is entry.sourceNote (lowercase — the upstream squash took the rename). Guard with ?. for one release so a stale cached bundle degrades instead of throwing.

4. Tie lookup — public/js/musicxml.js (only once #1731 is released)

matchTiesToTheirOwnNotes() (added by #380) patches OSMD at runtime so a tie's graphical note is looked up by identity before pitch: stock 2.1.2 can match a hidden unison note of another voice, and handleTie() then skips the tie. #1731 makes the same change upstream, in GraphicalStaffEntry.findTieGraphicalNoteFromNote().

  • Delete matchTiesToTheirOwnNotes(), its tieLookupHooked flag, and its call right after osmd.load() in renderMusicXML().
  • Check first that the release has it: in the bundle, findTieGraphicalNoteFromNote should call findGraphicalNoteFromNote before its pitch loop.

5. Verify

  • bundle exec rake test:parallel > tmp/test-output.txt 2>&1test/fingering_annotation_test.rb is the net here, in particular test_editing_a_fingering_in_a_two_voice_stack_rewrites_its_own_label, plus the Pathétique 2nd mvt m.24 case (chord in voice 1 + lower note in voice 2) that motivated the ordering code.

  • Debussy Arabesque no. 1, bar 3 of the left hand: the hidden unison head is still inked, and the triplet's 3 is now centred over the beam instead of sitting half a note-spacing to the right. That one needs no code change — it comes for free with the bump, and it is the visible proof #1730 is in.

  • Play a passage with a unison and check both heads still light up together under one keypress (that is what the reparenting buys).

  • Beethoven Pathétique 2nd mvt: the realized gruppetto notes written alongside the turn symbol — clicks must still fall through to the visible note, and fingering labels must stay centred on their noteheads.

  • Bach Prelude no. 2 BWV 847, bar 35 of the left hand: the held C2 (dotted eighth → quarter → half) keeps both ties once matchTiesToTheirOwnNotes() is gone.

Not in scope

No CHANGELOG or in-app changelog entry: the tuplet number recentring is too small to be worth a reader's time, and the rest is invisible.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by checking the next OSMD release with gh release list, then inspect public/score.html, public/js/musicxml.js, and public/js/fingeringEditor.js for the vendored bundle and workarounds described. Run bundle exec rake test:parallel and review test/fingering_annotation_test.rb, then verify the listed scores and interactions; done means the new OSMD APIs replace redundant logic without breaking note coloring, fingering alignment, or ties.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend, testing-qa, web-dev
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.