feat: model glissando, slide, and wavy-line spanners in mx::api - #414
Merged
Conversation
Closes #389. Glissando and slide were dropped entirely on read and could not be authored; a wavy line read and wrote as an unpaired mark that always emitted type="start", so a trill extension read as a start/stop pair came back as two starts and never closed. - New api::GlissandoStart/GlissandoStop (GlissandoData.h), one family for <glissando> and <slide> discriminated by a GlissandoType field, matching the existing CurveType (slur/tie) pattern. Carries pairing, printed text, line appearance, print style, and (for slide) trill-sound playback timing. - New api::WavyLineStart/Continue/Stop (WavyLineData.h) for <wavy-line>, a start/continue/stop spanner living inside <ornaments>. Carries placement, color, an optional SMuFL glyph override, and trill-sound playback. - Both element families go through the existing SpannerNumber identity mechanism (#297, #350): SpannerNumberResolver gained glissando, slide, and wavy-line number pools, independent of each other and of the existing slur/tie/wedge/octave-shift/bracket/dashes pools. - mx::impl: new GlissandoFunctions.h (templated over Glissando/Slide, mirroring CurveFunctions.h) and WavyLineFunctions.{h,cpp}; wired through NoteFunctions, OrnamentsFunctions, and NotationsWriter with correct containment (glissando/slide as top-level <notations> children, wavy-line inside <ornaments>) and stop-before-start ordering on a shared note (#139). - Removed MarkType::wavyLine (breaking, as the issue anticipated -- its mark-based model could not express pairing at all) and the dead SlideFunctions stub. - New GlissandoApiTest.cpp / WavyLineApiTest.cpp: 13 unit tests covering round-trip pairing, printed text, line appearance, slide/wavy-line playback attributes, container placement, stop-before-start ordering on a chained note, and independent identity-number pools, plus fromXml reads of the synthetic fixtures. - roundtrip-baseline.txt gained 13 newly-passing corpus files, including the three the issue names (ly33h_Spanners_Glissando.xml, k005a_Spanners_Slides.xml, and progress on k003a_Ornaments.xml, which still fails on an unrelated pre-existing tremolo bug). make test-all (core roundtrip, core unit, api-test, api-roundtrip) and make fmt-check are green; a unity build also compiles clean.
webern
commented
Aug 23, 2026
Signed-off-by: Matthew James Briggs <matthew.james.briggs@gmail.com>
Signed-off-by: Matthew James Briggs <matthew.james.briggs@gmail.com>
Owner
Author
|
/coverage |
Contributor
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28524 / 36672 |
| Functions | 74.2% | 6358 / 8572 |
| Branches | 50.7% | 22686 / 44753 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 88.4% | 9106 / 10304 |
| Functions | 81.4% | 3901 / 4793 |
| Branches | 54.8% | 7788 / 14222 |
Core HTML report | API HTML report
Commit 67f829f433c63f83bcc2379c3c4b5cf21139c303.
The emscripten CI job (#392) started failing after the glissando/slide/ wavy-line spanner writer was added (#389): mxwrite crashed under Node with "RuntimeError: memory access out of bounds" inside mx::impl::NotationsWriter::getNotations(), even on a note with zero notations. Root cause: Emscripten's default wasm stack is 64 KiB, and the write path is a deep, unoptimized (Debug build) C++ call chain -- DocumentManager -> ScoreWriter -> PartWriter -> MeasureWriter -> NoteWriter -> NotationsWriter -- that was already close to that limit. Extracting the new glissando/slide and wavy-line loops into their own methods (mirroring NotationsWriter's existing addArticulation/addOrnament/addTechnical pattern) shrinks getNotations()'s own frame, but the crash persisted even in the extracted method: the call chain's *cumulative* depth was the real constraint, not any one function's frame size. Bisecting confirmed the default 64 KiB stack was only marginally insufficient (as little as ~80 KiB sufficed); CMakeLists.txt now requests 1 MiB via emscripten's -sSTACK_SIZE link flag, leaving comfortable headroom for further growth in this call chain (e.g. #412, which plans to extend the same writer). Verified locally with the pinned emsdk 6.0.6 toolchain: reproduced the crash on the pre-fix commit, confirmed the extraction method alone did not fix it, bisected the minimum working -sSTACK_SIZE, and confirmed a clean `make wasm-test` (mxread/mxwrite/mxhide) passes with the CMakeLists.txt change in place. `make api-test` (560 cases, 6159 assertions) still passes natively, confirming the NotationsWriter extraction is behavior-preserving.
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.
Human Summary
Adds glissando, slide and wavy-line to mx::api. These were relatively similar to established patterns and thus straightforward.
Summary
glissandoandslidewere dropped entirely on read and could not be authored:NoteFunctions::parseNotationshad an emptybreakfor glissando and a commented-out body for slide, and the write side had noCurveType-style path for either.wavy-lineread and wrote as an ordinaryMarkType::wavyLinemark carrying only position/print data;core::WavyLine::type()is a requiredStartStopContinuethe writer never set, so every wavy line came outtype="start"regardless of source, and a trill extension read as a start/stop pair was written back as two starts that never close.This adds all three as proper note-attached spanners:
GlissandoStart/GlissandoStop(newGlissandoData.h): one family for<glissando>and<slide>, discriminated by aGlissandoTypefield, following the same patternCurveTypealready uses to share one shape between<slur>and<tied>. Carries pairing, the printed text (the element value, e.g. "gliss."), line appearance (line-type/dash-length/space-length), position, font, and color, plus slide-only trill-sound playback (accelerate/beats/first-beat/last-beat).WavyLineStart/WavyLineContinue/WavyLineStop(newWavyLineData.h):<wavy-line>is a start/continue/stop spanner living inside a note's<ornaments>, not<notations>directly. Carries placement, color, an optional SMuFL glyph override, and its own trill-sound playback (start-note/trill-step/two-note-turn/accelerate/beats/second-beat/last-beat).SpannerNumberidentity mechanism from Add API-level spanner identity for writer-sidenumberassignment #297/SpannerNumberResolver pools numbers per staff, butnumber-levelscope is the part #350:SpannerNumberResolvergainedglissando,slide, andwavyLinenumber pools, independent from each other and from the existing slur/tie/wedge/octave-shift/bracket/dashes pools, and independent of each other even when<glissando>and<slide>overlap in the stream.mx::impl: newGlissandoFunctions.h(templated overcore::Glissando/core::Slide, mirroringCurveFunctions.h) andWavyLineFunctions.{h,cpp}, wired throughNoteFunctions,OrnamentsFunctions, andNotationsWriter. Stops are always emitted before starts on a shared note, per the Question about ordering of starts and stops for e.g. slurs #139 rule.MarkType::wavyLineand the deadSlideFunctionsstub (already commented out, referencing aSpannerTypeenum deleted years ago). This is a breaking change:MarkType::wavyLine's mark-based model could not express pairing at all, so it could never be more than a fallback.Testing
GlissandoApiTest.cppandWavyLineApiTest.cpp: 13 cases covering round-trip pairing, printed text, line appearance, slide/wavy-line playback attributes, container placement (<notations>vs<ornaments>), stop-before-start ordering on a chained note, independent identity-number pools, andfromXmlreads of the synthetic fixturesmake test-allgreen: core roundtrip, core unit, api/impl suite (6159 assertions in 560 test cases), api roundtrip (379 pinned, 0 failed)roundtrip-baseline.txt, including the three files the issue names (lysuite/ly33h_Spanners_Glissando.xml,ksuite/k005a_Spanners_Slides.xml, and progress onksuite/k003a_Ornaments.xml, whose wavy-line mismatch is now fixed though it still fails on an unrelated pre-existing tremolo attribute bug). No file regressed.make fmt-checkpassesCMAKE_UNITY_BUILD=ON, batch size 0) compiles cleanReferences
numberassignment #297 and SpannerNumberResolver pools numbers per staff, butnumber-levelscope is the part #350mx::api#412 (hammer-on/pull-off), which is explicitly deferred to follow whatever shape this PR settles on