feat(tab): add H/P text labels on hammer-on / pull-off arcs#2609
feat(tab): add H/P text labels on hammer-on / pull-off arcs#2609rafaelsales wants to merge 2 commits intoCoderLine:developfrom
Conversation
ee5fd63 to
f23e3bd
Compare
In the Tab renderer, the arc connecting hammer-on and pull-off notes is now annotated with an "H" (ascending fret = hammer-on) or "P" (descending fret = pull-off) label above the arc midpoint. The label is drawn via an overridden paint() in TabSlurGlyph, reusing the same canvas.fillText path already used for whammy/bend slurText. TieGlyph's coordinate fields (_startX/Y, _endX/Y, _tieHeight, _shouldPaint) are widened from private to protected to allow the subclass to read them during paint. Fixes CoderLine#2608
f23e3bd to
6901cd3
Compare
|
Great addition, thanks for the change. We might need to check some additional edge cases like:
|
- Individual arcs per H/P pair in chains (e.g. 5{h} 7{h} 5 now
renders separate H and P arcs instead of one collapsed arc)
- Prevent tryExpand from merging slurs with different H/P labels
(fixes label loss when multiple H/P on same beat share beam direction)
- Guard existing effectSlur blocks to skip H/P notes, keeping legato
slide rendering unaffected
|
Thanks for the review! I've pushed a follow-up commit addressing both edge cases: 1. Direct H/P chainsThe model ( Fix: The renderer now uses 2. Multiple H/P on the same beatWhen two notes on the same beat pair had H/P effects with the same beam direction, Fix: Both are covered by new test cases in the test page (sections 5 and 6). |
Summary
Fixes #2608.
In the
Tab(andTabMixed) renderer, hammer-on and pull-off connections are drawn as arcs between notes but the conventional H / P text labels were never rendered.This PR adds H/P label rendering and correctly handles edge cases like H/P chains and multiple H/P on the same beat.
Changes
Commit 1 —
feat(tab): add H/P text labels to hammer-on and pull-off arcsTieGlyph.ts— adds agetSlurText()virtual method (returnsundefinedby default) and draws the label text at the arc midpoint inpaint()when present.TabSlurGlyph.ts— accepts an optionalslurText?: stringconstructor parameter and overridesgetSlurText()to return it.TabBeatContainerGlyph.ts— passes'H'or'P'when creating effect slurs for hammer-pull origin notes.Commit 2 —
fix(tab): handle H/P chain and same-beat edge casesTabSlurGlyph.ts—tryExpand()now accepts an optionalslurTextparameter and rejects merging slurs with different labels (prevents label loss when multiple H/P on same beat share beam direction).TabBeatContainerGlyph.ts— creates individual arcs per H/P pair usinghammerPullOrigin/hammerPullDestinationlinks (instead of the collapsedeffectSlurOrigin/effectSlurDestination). This renders chains like5{h} 7{h} 5as two separate arcs (H then P) instead of one collapsed arc. Non-H/P effect slurs (e.g. legato slides) continue to use the existingeffectSlurpath, guarded by!n.isHammerPullOrigin/!n.isHammerPullDestination.How it looks
Test plan
5{h} 7{h} 5) → individual H and P arcs5{h} 7{h} 5{h} 7) → alternating H/P/H arcs{sl}) → arc without label (regression check)Running the local test page
Step 1 — Save the HTML below as
test-hp-labels.htmlin the repo root.Step 2 — Build the web bundle and start a local server from the repo root:
Step 3 — Open http://localhost:8765/test-hp-labels.html in your browser.