Open
Conversation
- Core engine: CoachEngine, TimingEvaluator, LatencyManager, MidiInputHandler - UI components: FeedbackRenderer with smart note head detection, CoachSettingsDialog, ResultsDialog - Visual feedback: Color-coded timing circles (blue/green/yellow/red) with horizontal offset for early/late hits - Flam/grace note support: Intelligent positioning based on SVG element scanning - Test infrastructure: 45 comprehensive visual tests with reference screenshot comparison - Integration: Coach button in main UI, session lifecycle management - Reference screenshots: All feedback patterns validated and saved for regression testing
…undant PerformancePlayback tests
- M0: Use Meter Left Edge (or Clef/Key Right Edge) for precise start of rhythmic content, removing all heuristics/offsets. - Bars: Adjust centering by -1.5px to account for asymmetric symbol padding. - Debug: Remove debug lines/logs, make labels thinner (1.5px stroke). - UI: Ensure editor grid restores reliably after session. Remove debug toggle from settings.
Replace flat sniffed data structure with hierarchical staffs[] format:
{ verticalStep, staffs: [{ topY, notes, boundaries }] }
- NotationSniffer.getSniffedData() transforms internal data to new contract
- Pre-computes M0 boundary from header elements (clef/key/meter)
- All coordinates in screen pixels, no scale pass-through
- Remove legacy fields: scale, engineWidth, clefRightX, keyRightX, meterLeftX
- FeedbackRenderer iterates staffs[] for multi-staff readiness
- Sniffer still detects single staff (multi-staff detection deferred)
…estHelper - CoachEngine: add missing import for evaluateHit from TimingEvaluator - CoachController: fix undefined normalizedDrum reference, add optional chaining on log - testHelper: fix renderABCtoSVG() called without abc_source arg → use displayNewSVG()
The two horizontal beam paths in the snare drag SVG icon used class="fill" instead of class="drag_fill". Without a matching CSS rule, they defaulted to black fill and were always visible as an "=" symbol on every snare button. The old abc2svg version incidentally masked this by injecting a global .fill rule.
Replace broken SVG-boundary detection (img_out hook) with X-position-reset detection using a unified event stream. All annotations (notes, bars, headers) are collected into a single ordered buffer and split into systems based on when note X positions reset from right to left margin. Key changes: - Remove img_out hook and currentSvgIndex tracking - Collect all anno_stop events into flat events[] array - Split into systems in getSniffedData() via X-position drops - Use pending buffer to correctly assign bars/headers to systems (abc2svg emits bars BEFORE their system's notes) - Assign svgIndex by sequential order after filtering legend systems - Store hook originals on ScoreLayout instance (not on abc.user target) Fixes: multi-staff detection with legends, final barline assignment, SVG index alignment for feedback rendering
Instead of manually hiding #musicalInput, #sheetMusicTextFields, and #bottomButtonRow during coaching, leverage the existing view mode (swapViewEditMode) which hides all .edit-block elements. - Save prior view/edit state before entering coach mode - Hide the view-edit toggle button during coaching - Restore prior state when coaching ends
- Fix count-in toggle: add missing setMetronomeCountIn stub to groove_utils
- Fix MIDI init race condition: patch midiInitialized callback during coaching
to prevent it from overwriting the coaching onclick handler
- Add Mode=coach URL parameter: auto-enters coach mode without autoplay
- Bidirectional play/stop button: play when stopped, stop session when playing
- Fix timestamp reset: temporarily disable updateMidiPlayTime during stop to
prevent late MIDI callbacks from overwriting the 0:00 reset
- Remove unnecessary layout overrides on player row (timestamp stays in place)
- Solo button: vertical centering and spacing improvements
- Hide Permutations and Grooves menus during coaching
- Guard patched MIDI callbacks with isCoachingActive check
- startSession accepts {autoPlay} option (defaults to true)
- Delete .DS_Store files, log files, and debug abc2svg-pretty.js
- Add .DS_Store and *.log to .gitignore
- Remove debug console.log('[abc2svg-upgrade]') statements
- Revert formatting-only changes in groove_writer_orange.css and index.html
- Revert tom loop refactoring (independent improvement, not coach-related)
- Move css/coach.css to coach/css/coach.css for better encapsulation
- Add coach/TODO.md for independent improvements to extract later
Restore master's original indentation while preserving only substantive changes: - abc2svg library upgrade (Abc -> abc2svg.Abc) - ScoreLayout hooks for coach coordinate extraction - SVGLibCallback self-binding and updated annotation signatures - coachMode URL parameter support - renderWidth hack removal - setMetronomeCountIn stub, tablature function exports - HTML tag bugfixes (unclosed </span>, SVG class, sticking mouseenter)
- Delete coach/tests/ directory (headless, visual, fixtures, scripts, lib) - Remove devDependencies (jest, pixelmatch, pngjs, puppeteer) - Remove test scripts from package.json - Remove node_modules and package-lock.json - Clean up .gitignore test-related entries
- ScoreLayout: use s.extra.x for grace note head position (s.x gives the parent note's position, not the grace note's own) - FeedbackRenderer: match grace notes positionally (Nth timeline grace → Nth sniffed grace) since abc2svg and _refreshAbcMapping use different index counting for grace notes; skip index labels for graces - CoachController: emit grace note timeline entries for flams so the renderer can pair them with sniffed grace notes
… boundaries→measureBoundaries - Move NOTE_Y_OFFSETS from FeedbackRenderer to ScoreLayout - Add _buildNoteYs() to precompute DrumType→SVG Y map per system - Remove note y field from output (unused in SVG space for unisons) - FeedbackRenderer uses system.noteYs[drumType] directly, no offset knowledge - Fix staffData→systemData bug in measure boundary construction - Clean up all comments for terminology accuracy - Document getSniffedData() return structure with clear field descriptions
- Rename isSplash→isFoot and match any non-kick value in kick_array instead of listing specific strings. The old check missed raw ABC values like '^d,' returned by grooveDataFromClickableUI(). - Fix key format in lower voice (kickStemsUp=false) to use DrumType constants with standard format, matching getAbcIndexForHit lookups.
…ds, waypoint cursor - Add architectural comment explaining continuous-strip model, lifecycle, and clearing model - Extract _computeThresholds() from _buildWaypointData (100 lines of inverse-interpolation logic into its own semantically clear method) - _tickPlayLine: replace O(n) linear scan with _waypointCursor that advances forward only, giving O(1) amortized interpolation per frame - _interpolateXWithSystem retained for arbitrary-time lookups (drawExtraHit) - Reset _waypointCursor in scheduleMeasureClearing
- Move FeedbackRenderer.js → feedback/Renderer.js (class rename) - Extract PlaylineInterpolator.js: pure geometry, no DOM (build, interpolate, interpolateWithCursor, getMeasureIndex, _offsetToSystem, _computeThresholds) - Extract MeasureClearer.js: threshold-based circle removal (init, schedule, tick, _clearRegion) - Name 0.1 → ABC_TIME_DRIFT_WARN_MS - Add _tickPlayLine null guard for rAF/stop race - State.js: remove unreachable getToleranceWindows fallback, remove unnecessary countIn ?? DEFAULTS.countIn Renderer.js: 1032 → 660 lines
- Renderer: coach-feedback-layer id → class (IDs must be unique per DOM) - CSS: updated selector to match class change - ScoreLayoutExtractor: removed dead svgIndex initial assignment (overwritten) - Controller: replaced window.scoreLayout with imported scoreLayoutExtractor - Controller: removed unused sniffedData variable in _refreshAndSyncUI - bootstrap.js: named retry delays, added retry limits (20/10)
… of | separators - Remove | character from debug measure boundary labels (M0, M1, etc.) - Position labels beside barlines using text-anchor start/end - Add anchor parameter to _addDebugText (default 'middle', backward-compatible) - Add DEBUG_LABEL_EXTEND_PX and DEBUG_LABEL_NUDGE_PX constants
ABCIndexMapper.buildMap() counted sticking voice events (R/L) to offset drum note indices. But stickings are hidden rests (x) in abc2svg, so ScoreLayoutExtractor never sees them (it only processes type==='note'). This caused an abcIndex mismatch: ABCIndexMapper assigned drum notes starting at index 4 while ScoreLayoutExtractor started at 0. The renderer found no matches, producing an empty timeline with no playline, no feedback circles, and no debug lines when stickings were enabled.
Tap-along calibration flow: 4-beat countdown, 20 taps at 80 BPM (first 4 warmup), median offset computed and shown in real-time. Settings dialog gets a latency offset field + Calibrate button. Offset persisted to localStorage and fed into LatencyManager.
Any keypress (except Escape and repeats) now registers as a tap during calibration, using performance.now() as the timestamp.
Add `calibrated` boolean to State (persisted to localStorage). Settings dialog shows "Not calibrated" hint in red when false. The flag is set to true only when the user accepts a calibration result, so 0ms is correctly treated as a valid calibrated value.
Configurable MIDI-to-drum mapping system replacing the hardcoded GM map. JSON-based preset files with inheritance (manufacturer base → module). Ships with presets for Roland V-Drums, Yamaha DTX, and Alesis modules. Manual MIDI-learn mode for custom mapping (conflict detection included). Mapping persisted to localStorage and expressible as URL params. Combined soft-nudge prompts user to configure mapping/calibration on first session.
Replace compile-time SHOW_DEBUG constant with runtime coachState.showDebugGrid. Toggle added to settings dialog, enabled by default, persisted to localStorage.
All dialogs now share: 14px base font, 16px h2, consistent padding (15px 20px), and uniform row/button sizes. Removes the mix of 12/14/16/18px that made each row a different height.
When the dialog is open, hitting a mapped pad flashes the corresponding row and highlights the specific note chip. Unmapped notes show a brief "Unmapped note: XX" notice. Passive listening pauses during MIDI-learn mode and resumes after.
Range slider (0-100%, default 100%) controls metronome velocity during coaching sessions and calibration. Persisted to localStorage. Maps percentage to MIDI velocity (0-127) for groove playback and to oscillator gain for calibration clicks.
Efnote modules use identical MIDI assignments to Roland V-Drums (verified from official EFNOTE 3/5/7 Reference Guide). Inherits from roland/_base with no overrides.
Roland/Efnote/Yamaha modules send the same note for hi-hat hits regardless of pedal position, using CC#4 to indicate open/closed. MidiInputHandler now tracks CC messages and resolves hh_open notes to hh_closed when pedal CC >= threshold. Configurable in the drum mapping dialog with enable toggle, CC number, and threshold inputs. Passive MIDI feedback in the mapping dialog also uses CC values.
- Live CC bar with threshold marker updates in real-time as pedal moves - CC value number display next to the bar - Bar turns green (open) / orange (closed) based on threshold - CC-resolved hits flash blue on HH Closed row (vs orange for direct note match) - CC-resolved hits also highlight the source chip on the HH Open row
Reduce row padding, label/chip sizes, learn button size, and CC section spacing. Dialog fits at 1024x640 without scrolling.
Add Efnote and hi-hat CC to feature list, GitHub Pages link, and comprehensive coach architecture section to SOURCE_CODE_README covering file structure, key concepts, and GrooveWriter integration.
Define custom SVG glyphs for ghost note parentheses in %%beginsvg and reference them from the %%deco definitions, instead of relying on the arbitrary glyph name "a" that the new abc2svg no longer renders as text.
2opremio
force-pushed
the
feature/drum-coach
branch
from
d680aa8 to
3da5af7
Compare
The tuplet notation (p:p:p) means "p notes in time of p" which is no timing adjustment. Changed to (p:q:p) where q = p*2/3, producing correct triplet ratios: (3:2:3), (6:4:6), (12:8:12). Applied to all three ABC voices (stickings, HH/snare, kick) so their durations stay consistent within each measure. This fixes the coach playbar falling behind by ~1/3 of the measure because abc2svg was assigning s.time values at nominal (non-triplet) spacing.
Use engine-derived note times instead of abc2svg's s.time for the playbar timeline. abc2svg's s.time is wrong for triplets because the ABC uses (p:p:p) tuplet notation (no timing adjustment) to keep rendering correct, so s.time reflects nominal spacing instead of actual triplet timing. Rest waypoint times are interpolated from surrounding notes using their X positions, which abc2svg places correctly regardless of tuplet notation.
2opremio
force-pushed
the
feature/drum-coach
branch
from
77f7d96 to
4fb9ab9
Compare
Enabled by default so coach sessions start with synth muted (only metronome plays). The pre-session Solo state is saved and restored when the session ends.
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
1 participant
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.
TLDR: You can test this feature live at https://2opremio.github.io/GrooveScribe/
What
This pull request introduces the new Drum Coach module, a comprehensive rhythm feedback and live timing evaluation system that hooks seamlessly into GrooveScribe without impacting the
mastercodebase or existing layout properties.How It Works
localStorage.abc2svgengine and tracking multi-system SVG bounds (ScoreLayoutExtractor.js), the coach interpolates a smoothly renderedrequestAnimationFrame()play-line over the active groove.Demonstration
CoachSession_60fps.mp4
Setup:
SetupAndCalibration_60fps.mp4
Critical Updates Included
This PR also includes a library update and a fix for a bug discovered during development:
abc2svg Library Upgrade: Upgraded
abc2svg-1.jsto a newer version to resolve margin alignments and properly natively map SVG coordinates 1:1. This removed a previous scaling workaround whererenderWidthwas manually compressed to 75%. Also patchedthisbinding mapping on annotation callbacks (this.mapped toself.) and added thesparameter toanno_start/anno_stopsignature.UI HTML Tag Bugfixes (
groove_writer.js): Fixed multiple unclosed</span>tags on instrument unmute buttons (hh, tom1, snare, tom4, kick). Also corrected SVG class prefixes from"fill"to"drag_fill"on drag note SVG paths to avoid CSS conflicts.If preferred, I would be happy to spin these bugfixes and the library update off into a separate PR entirely.
Happy to create separate PRs for this if need be.
Testing & Maintenance
Includes thorough inheritance-based preset module drum-maps (Roland/Yamaha/Alesis).
Tested primarily on a Roland V51. Other module mappings are provided but yet to be tested. This will natively require a lot more testing, and I am very happy to maintain this part of the repository if merged.
Squashing
I will of course squash all these commits merging, but I have left them separate for now so that the history of this subproject can be overseen during review.
Future work
My plan is, once this has been integrated and well test it, also include dynamics feedback on top of rhythm feedback (e.g. ghost note/accent evaluation, uniform dynamics feedback etc etc ).