vexflow
vexflow copied to clipboard
0xfe
ModifierContext is too complex. Replace with a simpler formatter.
A ModifierContext has a state that has 4 properties, each which maps to a Modifier.Position.
state property |
Modifier.Position |
Unit |
|---|---|---|
top_text_line |
ABOVE |
staff space |
text_line |
BELOW |
staff space |
left_shift |
LEFT |
pixels |
right_shift |
RIGHT |
pixels |
Sidenotes:
text_line should be renamed to bottom_text_line.
Having different units for horizontal/vertical formatting is not ideal. But we can't escape that for now.
All these properties do is keep track of an incremented value in a specific direction. Isn't this already what the Formatter does? It advances a position forward along the x axis (albeit with complex logic to determine the amount...). It feels like the ModifierContext duplicates and conflates responsibilities since it tries to manage 4 positions at once. So what if we deprecated the ModifierContext, and replaced it with with a new, simpler formatter that takes Modifier.Position and advances a position given:
- a
Modifier.Position - Objects for formatting
- Config options, ie: spacing
To replace the ModifierContext, I think we'd need 4 differently parameterized versions of these (one for each Modifier.Position).
In fact, in order to format GraceNotes within a GraceNoteGroup we use the existing Formatter and set its available width to 0 that the minimum width for each note is applies. But this is clearly a hack based on the absence of a simpler formatter.
Yeah, the state units need to be rationalized for sure, but even if you use a formatter, I think you'll still need a context to keep track of the modifiers across voices. The ModiferContext does not do any formatting, it's just a container for all the modifiers in a tick (i.e., across voices), and it holds some tracking state for the independent formatters. (It also kickstarts the formatting process, but that's just for ordering, which needs to be global.)
And since each category of modifier (e.g., accidentals, fingering, articulations) have their own distinct formatting logic you'll need a way for them to both format themselves, and juxtapose alongside other modifiers.
I think a simpler (and necessary) cleanup would be to replace the various state properties with a position object that uses the same unit across all axes.
Actually, thinking about this more, I think the units are pretty sane (staff spacing for vertical, and pixels for horizontal.) It actually makes the code more natural to reason about.
I strongly disagree. Every decision about formatting should use the staff space. It is the canonical unit of modern music engraving. If one wants to change the pixel size of the staff space, it shouldn't be the burden of every formatter to keep track of that. I'd contend that the formatters should be completely independent of the pixel-to-staff-space ratio. Any resolution of staff spaces to pixels should happen at render-time -- not earlier.
It's important to note that I don't think VexFlow is ready for this at all.... yet. But this is an ideal to strive for.
I guess I don't see how you can reason about horizontal formatting using vertical spacing. Are the ratios always fixed? Will there be scenarios that break this assumption?
(also, would love some pointers to literature about this -- I'm curious about how this is the canonical unit of music engraving.)
See Behind Bars, page 5 - "The Stave"
The size of every notation symbol is measured in proportion to the stave size. A stave-space is the distance between two stave lines and is used as a measurement for notational symbols and spacing in this book.
Then she provides this diagram:
In "The Art of Music Engraving and Processing" by Ted Ross, the units are in staff spaces as well.
Using staff spaces as the unit of measurement allows us to reason about formatting without being coupled to a specific pixel/staffspace ratio.
Many MusicXML units are in tenths of a staff space ("tenths"). They are allowed to be floating point, so I wish they had chosen "spaces" to begin with, but it's an easy conversion.
What is important though is to precisely define where a staff-space begins and ends. My thought would be that a staff-space should go from the top of one staff line to the top of the next, so that a standard five-line staff's total bounding area is 4*staff-space + 1 * staff-line, rather than the space between lines (4 * staff-space + 5 * staff-line). If the staff space includes the staff-line in it, then it makes spacing out things like ledger line placement much simpler and allows for adjusting the thickness of staff lines without affecting the overall size of the staff.
I do agree on the the staff's total bounding height (4 spaces + 1 line), but I think it's more intuitive to have the staff space go from staff line center to center. This way, both the size and boundaries of the staff space are completely independent of the staff line thickness. This also means that the top of the staff and the position of notehead on the top-most line have distinct y values (and similarly, the bottom y values). In my experience, both values are required for formatting.
After working a bit with ModifierContext I believe that the ideas here would be agood approach.
@AaronDavidNewman @0xfe what do your think? I refer to:
- Vertical formatting with reference middle line of stave
- Usage of stave-space for horizontal position.
- Rename text_line to bottom_text_line
I was looking for something else and came across this old, but still so useful discussion. Hope that even with @Silverwolf90 not being active right now, we can still work towards this goal.