Ledger Line Logic

[gristow]

Fixes #566, fixes ledger line formatting issues remaining in #557.

Changes:

• Ledger line drawing is now handled entirely by StaveNote
• All ledger line logic is removed from NoteHead
• Ledger lines are now drawn with the same style of the staff they're related to
• If both displaced and non-displaced notes are present above or below the staff, ledger lines are drawn for both positions until no longer needed in either position.
• Stave.setStyle() is now available to match the api's of other Elements.
• StaveNote.setLedgerLineStyle() uses Stave.getStyle() as its default style, merging onto it any changes.

This results in two obvious visual changes throughout with ledger lines:

• Ledger lines are now thinner than before (1px, matching Vex.Flow.STAVE_LINE_THICKNESS)
• Ledger lines are now colored as #999999 by default (matching Stave.options.fill_style's default).

Before:

After:

To my eyes, I really appreciate the consistency of style between the ledger lines and the staves. However: I find myself wondering if #999999 is too light of a default now that the ledger lines are the same color and thinner. Thoughts?

Finally, the NoteHead Bounding Boxes test loses a ledger line because this logic is moved into StaveNote. This isn't a grave concern to me -- I think it's pretty unlikely anyone is using VexFlow in a way that they're relying on NoteHead to draw a ledger line. (And, anyway, it would only ever draw the final ledger line before a note -- not all of the ones between the note and the staff.)

Before:

After:

That said: if anyone sees an issue with consolidating ledger lines in StaveNote please let me know. I'll hold off on merging this for a few days.

Thanks!

[0xfe]

Thanks for the change. What do the visual regression diffs look like? (particularly the rests)

Re: #999999, I'm open to darkening it.

[gristow]

Rests are unchanged -- except for where the notes have ledger lines:

Biggest changes are where the corrected ledger line algorithm for displaced notes made a difference:

Because so many of the tests have a ledger line, there are 285 diffs as a result of this PR. (I've looked over them all... but would love more eyes on them. I'll paste in a new comment... my apologies in advance for the length of that comment!)

[gristow]

[0xfe]

These look sane to me. Feel free to merge. 👍

[gristow]

Great -- will do.

Also: I'm wondering if we should bump the release version sometime soon. The current version (1.2.83, Sept. 2016) doesn't reflect the new accidental types, and many of the rewritten tests from last summer. (I only noticed because to get accurate visual regressions I had to build from a clean git pull, then grunt copy:release, apply my commits, and then run npm test.)

[0xfe]

Ah, yes. I'll bump it.

[mscuthbert]

Hi -- I think i'm late to the game, but I'm strongly against the coloring of the ledger lines the same color as the staff -- they look too easy to miss count to me. There's no notational rule i can cite -- but then of course having staves a different color as the notes is also a new thing (at least new since 1500).

[0xfe]

How about leaving the default as-is (i.e., before this commit) and add an option? (Also, darkening the stave lines might help a bit.) On Jul 30, 2017 7:29 PM, "Michael Scott Cuthbert" <notifications@github.com> wrote: Hi -- I think i'm late to the game, but I'm strongly against the coloring of the ledger lines the same color as the staff -- they look too easy to miss count to me. There's no notational rule i can cite -- but then of course having staves a different color as the notes is also a new thing (at least new since 1500). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#569 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOuk4OvLzsTlUYiFtZKsExUE4BvgGObks5sTRHZgaJpZM4Onavo> .

[gristow]

I tend to agree, Michael -- though having the ledger lines a different style from the stave lines feels very odd to me. In both Finale & Sibelius they are good old fashioned black (#000000), which would be my preference. For comparison:

Staves & Ledgers at #000000 (black):

Staves & Ledgers at #333333:

Staves & Ledgers at #666666:

Staves & Ledgers at #999999 (current default):

How do we feel about changing the default? Would this solve it from your perspective, @mscuthbert?

[mscuthbert]

I LOVE the gray staff lines -- I think that when @0xfe or whoever chose this as a default, it's brilliant; it makes the music stand out the way ink on really good manuscript staff paper does. Because the stafflines run the whole extent of the horizontal span, they don't need to be very prominent -- they're constantly in your field of vision. Ledger lines, however are mostly covered up so they need to be more prominent. Maybe thickness would be more important than color. Elaine Gould, Behind Bars, p. 26, says "Ledger lines are an extension of the stave. They are spaced the same width apart as the stave-lines, but they are about twice as thick." (emphasis added) Gould continues "It is important that ledger lines are visibly thicker than stave-lines so that a player reading a passage of ledger-line notes can take in the number of ledger lines at a glance." So perhaps they're fine as #999999 but just thicker?

[gristow]

Here it is at #999999 but ledger lines twice as thick as stave lines -- which looks odd to me:

And detail:

Given that Gould's world is that of monochromatic print music, maybe darker ledger lines would basically have the same effect. (Though, of course, if someone is using VexFlow for print they might like the option to change that.)

Here's stave & ledger lines the same thickness, but ledger's drawn at #000000:

Here's double thickness, ledger's drawn at #000000:

Whatever we wind up with as the default, I find myself wondering about allowing users to set a default for some of these 'house style' questions. For instance, in stave.js we might catch defaults via closure, and do something like:

const style = {
  strokeStyle: `#999999`,
  lineWidth: 1
}

const ledgerLineStyle = {
  strokeStyle: `#000000`,
  lineWidth: 1
}

class Stave extends Element {
  ...
  static setDefaultStyle(newStyle) {
    Object.assign(style, newStyle);
  }

  static setDefaultLedgerLineStyle(newStyle) {
    Object.assign(ledgerLineStyle, newStyle);
  }
  ...
}

Alternately, these defaults could be in some central location and accessible as variables on Vex.Flow to be changed by a user...

[mscuthbert]

I think that the last looks best, but if lineWidth can take a float then 1.5 and #333333 might be the best compromise?

[gristow]

A few more swatches to look at -- since floats are fine for lineWidth.

Here's stave & ledger lines at #999999 with ledger lines at 1.5x the stave line width:

And detail:

Here's stave & ledger lines at #666666 with ledger lines at 1.5x the stave line width:

And detail:

Finally, stave lines at #999999 and ledger lines at #000000 and 1.5x the stave line width:

And detail:

[mscuthbert]

The last looks best to me. Thanks for all the hard work!

I'll try to look through examples that make counting difficult (generally, telling the difference between 3, 4, and 5 ledger lines on monophonic music) and see how much thickness is necessary to make it stand out properly. But it's looking much better!