Tighten up positioning, some refactoring.

Author: MatthewDorner

README.md

@@ -17,10 +17,10 @@ const renderOptions = {
  xOffset: 3,
  widthFactor: 1.7,
  lineHeight: 185,
- clefWidth: 45,
- meterWidth: 30,
+ clefWidth: 60,
+ meterWidth: 25,
  repeatWidthModifier: 35,
- keySigAccidentalWidth: 20,
+ keySigAccidentalWidth: 10,
  tabsVisibility: 1,
  voltaHeight: 25,
  renderWidth: 500,

lib/main/draw-to-context.js

@@ -23,7 +23,7 @@ export default function drawToContext(context, tune) {
  part.bars.forEach((bar) => {
  let tabsPositionY;
  if (tune.renderOptions.staveVisibility) {
- tabsPositionY = bar.position.y + 60;
+ tabsPositionY = bar.position.y + 65;
  } else {
  tabsPositionY = bar.position.y;
  }
@@ -96,13 +96,15 @@ export default function drawToContext(context, tune) {
  });
 
  // DRAW CURVES
- part.curves.forEach((partCurve) => {
- const vexCurve = new Curve(partCurve.startNote, partCurve.endNote, {
- thickness: 2,
- cps: [{ x: -5, y: 7 }, { x: 0, y: 7 }],
- x_shift: -10
+ if (staveVisibility) {
+ part.curves.forEach((partCurve) => {
+ const vexCurve = new Curve(partCurve.startNote, partCurve.endNote, {
+ thickness: 2,
+ cps: [{ x: -5, y: 7 }, { x: 0, y: 7 }],
+ x_shift: -10
+ });
+ vexCurve.setContext(context).draw();
  });
- vexCurve.setContext(context).draw();
- });
+ }
  });
 }

lib/main/fill-empty-space.js

@@ -49,7 +49,6 @@ function expandBars(line, renderOptions) {
  const expandedBars = line.map((bar, i) => {
  const newBar = Object.assign({}, bar);
 
- // if it's a lead-in bar, don't add space
  let spaceToAdd;
  if ((bar.isFirst || (bar.repeats.includes(Vex.Flow.Barline.type.REPEAT_BEGIN) && bar !== line[line.length - 1])) && bar.notes.length < 3) {
  spaceToAdd = 0;

lib/main/generate-vex-objects.js

@@ -163,21 +163,24 @@ export default function generateVexObjects(partRegions, tuneAttrs, renderOptions
  tabNoteToAdd.addModifier(tabGraceNoteGroup);
  }
 
- obj.pitches.forEach((pitch) => {
+ obj.pitches.every((pitch) => {
  if (pitch.startTie) {
  currentPart.curves.push({
  startNote: noteToAdd
  });
+ return false;
  }
  if (pitch.endTie) {
  currentPart.curves[currentPart.curves.length - 1].endNote = noteToAdd;
+ return false;
  }
  if (pitch.startSlur) {
  pitch.startSlur.forEach(() => {
  currentPart.curves.push({
  startNote: noteToAdd
  });
  });
+ return false;
  }
  if (pitch.endSlur) {
  pitch.endSlur.forEach(() => {
@@ -190,13 +193,15 @@ export default function generateVexObjects(partRegions, tuneAttrs, renderOptions
  i += -1;
  }
  });
+ return false;
  }
+ return true;
  });
 
  currentBar.tabNotes.push(tabNoteToAdd);
  } else { // IS a rest
  noteToAdd = new StaveNote({
- clef: tuneAttrs.clef, keys: ['b/4'], duration: `${duration}r`
+ clef: tuneAttrs.clef, keys: ['b/4'], duration: `${duration}r`,
  });
 
  const tabNoteToAdd = new TabNote({
@@ -252,6 +257,9 @@ export default function generateVexObjects(partRegions, tuneAttrs, renderOptions
  if (meter === '4/4') {
  currentBar.beams = Beam.generateBeams(currentBar.notes, { groups: [new Vex.Flow.Fraction(2, 4)] });
  } else if (meter === '3/8' || meter === '6/8' || meter === '9/8' || meter === '12/8') {
+ // 3/8 isn't technically a compound time signature, maybe should call the function something else.
+ // I believe this is handling beams correctly for all time signatures though, need a visual test suite
+ // & compare to VexFlow visual tests https://www.vexflow.com/tests/
  currentBar.beams = generateBeamsCompound(currentBar.notes);
  } else {
  currentBar.beams = Beam.generateBeams(currentBar.notes, { groups: Beam.getDefaultBeamGroups(meter) });

lib/main/set-bar-positions.js

@@ -1,6 +1,8 @@
 import Vex from 'vexflow';
 import { Formatter } from 'vexflow/src/formatter';
 
+const NEW_PART_MULTIPLIER = 1.25;
+
 /**
  * Takes the Tune and sets the .position properties of each Bar object contained there.
  * Sets width of each Bar according to the renderOptions and based on the contents of
@@ -22,7 +24,7 @@ export default function setBarPositions(tune) {
 
  newTune.parts = tune.parts.map((part, partIndex) => {
  const newPart = Object.assign({}, part);
- newPart.bars = newPart.bars.map((bar, i) => {
+ newPart.bars = newPart.bars.map((bar, barIndex) => {
  const newBar = Object.assign({}, bar);
 
  // calculate space needed for clefSigMeterWidth
@@ -35,9 +37,12 @@ export default function setBarPositions(tune) {
  // get vexFlow to tell me how much space the notes need
  const formatter = new Formatter();
  formatter.createTickContexts([newBar.voice]);
-
  let notesWidth = formatter.preCalculateMinTotalWidth([newBar.voice]) * widthFactor;
 
+ // it still doesn't give enough space, so add more if there are few notes in the bar
+ notesWidth *= 1 + 3 / ((newBar.notes.length + 1) * (newBar.notes.length + 1));
+
+ // extra space for accidentals (sharp and flat signs)
  const notesWithAccidentals = newBar.notes.reduce((acc, note) => {
  if (note.modifiers) {
  note.modifiers.forEach((modifier) => {
@@ -48,16 +53,7 @@ export default function setBarPositions(tune) {
  }
  return acc;
  }, 0);
-
  notesWidth += notesWithAccidentals * keySigAccidentalWidth;
- // still had crowding in bars with few notes, even when using
- // preCalculateMinTotalWidth, seems to be the same problem my own code had before
- if (newBar.notes.length === 3) { notesWidth *= 1.25; }
- if (newBar.notes.length === 2) { notesWidth *= 1.55; }
- if (newBar.notes.length === 1) { notesWidth *= 1.75; }
- // also note I am adding width here if the length is lower than 4, but at the
- // fillEmptySpace stage am preventing it from sharing space with the bar
- // if it's lower than 4. that seems wrong ??
 
  // determine total min width of the bar including clefSigMeterWidth
  let minWidth = notesWidth + newBar.position.clefSigMeterWidth;
@@ -66,25 +62,23 @@ export default function setBarPositions(tune) {
  }
  if (minWidth > renderWidth) { minWidth = renderWidth; }
 
- // 14 another magic number to represent the height needed when 6 strings are in the tab
- // when there are less strings, it will reduce the vertical space used by an appropriate amount
  const spaceForStave = lineHeight * 0.5;
- const spaceForTabs = lineHeight * ((14 - (6 - tabNumberOfLines)) / 14) - spaceForStave;
+ const spaceForTabs = 0.5 * lineHeight - (6 - tabNumberOfLines) / 14;
 
- if (i === 0) { // first bar of part
+ if (barIndex === 0) { // first bar of part
  newBar.position.x = xOffset;
 
  if (partIndex === 0) { // first part
- yCursor += (lineHeight * 1.25); // very top of tune
+ yCursor += (lineHeight * NEW_PART_MULTIPLIER); // very top of tune
  } else {
  if (tabsVisibility) {
- yCursor += spaceForTabs * 1.25;
+ yCursor += spaceForTabs * NEW_PART_MULTIPLIER;
  }
  if (staveVisibility) {
- yCursor += spaceForStave * 1.25;
+ yCursor += spaceForStave * NEW_PART_MULTIPLIER;
  }
  }
- } else if (newPart.bars[i - 1].position.x + newPart.bars[i - 1].position.width + minWidth > renderWidth) { // first bar of a new line
+ } else if (newPart.bars[barIndex - 1].position.x + newPart.bars[barIndex - 1].position.width + minWidth > renderWidth) { // first bar of a new line
  newBar.position.x = xOffset;
  if (tabsVisibility) {
  yCursor += spaceForTabs;
@@ -93,7 +87,7 @@ export default function setBarPositions(tune) {
  yCursor += spaceForStave;
  }
  } else { // bar on an incomplete line
- newBar.position.x = newPart.bars[i - 1].position.x + newPart.bars[i - 1].position.width;
+ newBar.position.x = newPart.bars[barIndex - 1].position.x + newPart.bars[barIndex - 1].position.width;
  }
  newBar.position.y = yCursor;
  newBar.position.width = minWidth;

lib/utils/tests/get-tab-position.test.js

@@ -4,20 +4,14 @@ import { TUNINGS } from '../../constants/tunings';
 import AbcObjects from '../../test-data/abcjs-objects';
 import AbcKeySignatures from '../../test-data/abc-key-signatures';
 
-const tuning = {
- displayName: 'Guitar (EADGBE)',
- strings: ['e/3', 'a/3', 'd/4', 'g/4', 'b/4', 'e/5'],
- type: 'strings_fretted'
-};
-
 // TODO: separate unit tests from integration
 
 test('e3 is lowest note on guitar', () => {
  const keys = ['e/3'];
  const abcKeySignature = AbcKeySignatures.CMajor;
  const barContents = [AbcObjects.Notes.Eighths.F];
 
- expect(getTabPosition(keys, abcKeySignature, barContents, 0, tuning, false)).toStrictEqual([{ str: 6, fret: 0 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 0, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 6, fret: 0 }]);
 });
 
 test('sharped note causes itself and later notes to be sharped', () => {
@@ -28,9 +22,9 @@ test('sharped note causes itself and later notes to be sharped', () => {
  const barContents = [f, fSharp, f];
 
  // str: 1, fret: 1 is F natural
- expect(getTabPosition(keys, abcKeySignature, barContents, 0, tuning, false)).toStrictEqual([{ str: 1, fret: 1 }]);
- expect(getTabPosition(keys, abcKeySignature, barContents, 1, tuning, false)).toStrictEqual([{ str: 1, fret: 2 }]);
- expect(getTabPosition(keys, abcKeySignature, barContents, 2, tuning, false)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 0, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 1, fret: 1 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 1, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 2, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 1, fret: 2 }]);
 });
 
 test('sharp in key signature causes note to be sharped', () => {
@@ -40,7 +34,7 @@ test('sharp in key signature causes note to be sharped', () => {
  const barContents = [f];
 
  // str: 1, fret: 2 is F sharp
- expect(getTabPosition(keys, abcKeySignature, barContents, 0, tuning, false)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 0, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 1, fret: 2 }]);
 });
 
 // bugfix: in abcjs key sig, accidental.note when the note is 'b',
@@ -52,9 +46,9 @@ test('in B Flat Minor key, B notes get flatted', () => {
  const barContents = [b];
 
  // str: 1, fret: 6 is B flat
- expect(getTabPosition(keys, abcKeySignature, barContents, 0, tuning, false)).toStrictEqual([{ str: 1, fret: 6 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 0, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 1, fret: 6 }]);
  // now with C major key sig, should be a B natural
- expect(getTabPosition(keys, AbcKeySignatures.CMajor, barContents, 0, tuning, false)).toStrictEqual([{ str: 1, fret: 7 }]);
+ expect(getTabPosition(keys, AbcKeySignatures.CMajor, barContents, 0, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 1, fret: 7 }]);
 });
 
 test('sharped note, when sharp already exists in key sig, doesnt sharp the note twice', () => {
@@ -65,7 +59,7 @@ test('sharped note, when sharp already exists in key sig, doesnt sharp the note
 
  // str: 1, fret: 1 is F natural and str: 1, fret: 2 is F sharp. we're checking to make
  // sure it doesn't somehow end up str: 1, fret: 3
- expect(getTabPosition(keys, abcKeySignature, barContents, 0, tuning, false)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 0, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 1, fret: 2 }]);
 });
 
 test('natural accidental cancels out key sig sharp', () => {
@@ -76,7 +70,7 @@ test('natural accidental cancels out key sig sharp', () => {
 
  // the natural accidental on the specific note should cancel out the sharp accidental in the key sig
  // str: 1, fret: 1 is F natural
- expect(getTabPosition(keys, abcKeySignature, barContents, 0, tuning, false)).toStrictEqual([{ str: 1, fret: 1 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 0, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 1, fret: 1 }]);
 });
 
 test('grace note - sharped grace note causes itself and later grace notes to be sharped', () => {
@@ -88,9 +82,9 @@ test('grace note - sharped grace note causes itself and later grace notes to be
 
  // str: 1, fret: 1 is F natural
  // passing is GraceNote true, meaning we're getting tab position for the grace notes here
- expect(getTabPosition(keys, abcKeySignature, barContents, 0, tuning, true)).toStrictEqual([{ str: 1, fret: 1 }]);
- expect(getTabPosition(keys, abcKeySignature, barContents, 1, tuning, true)).toStrictEqual([{ str: 1, fret: 2 }]);
- expect(getTabPosition(keys, abcKeySignature, barContents, 2, tuning, true)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 0, TUNINGS.GUITAR_STANDARD, true)).toStrictEqual([{ str: 1, fret: 1 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 1, TUNINGS.GUITAR_STANDARD, true)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 2, TUNINGS.GUITAR_STANDARD, true)).toStrictEqual([{ str: 1, fret: 2 }]);
 });
 
 test('grace note - sharp in key signature causes grace note to be sharped', () => {
@@ -100,7 +94,7 @@ test('grace note - sharp in key signature causes grace note to be sharped', () =
  const barContents = [cWithFGraceNote];
 
  // str: 1, fret: 2 is F sharp
- expect(getTabPosition(keys, abcKeySignature, barContents, 0, tuning, true)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 0, TUNINGS.GUITAR_STANDARD, true)).toStrictEqual([{ str: 1, fret: 2 }]);
 });
 
 test('grace note - sharped grace note causes later regular notes to be sharped', () => {
@@ -112,7 +106,7 @@ test('grace note - sharped grace note causes later regular notes to be sharped',
 
  // passing isGraceNote as false because we're testing a regular F note at position 1 and
  // confirming the sharp from previously sharped F grace note applies to it
- expect(getTabPosition(keys, abcKeySignature, barContents, 1, tuning, false)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 1, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 1, fret: 2 }]);
 });
 
 test('grace note - sharped regular note causes later grace notes to be sharped', () => {
@@ -124,7 +118,7 @@ test('grace note - sharped regular note causes later grace notes to be sharped',
 
  // passingisGraceNote as true because we're testing the F grace note attached to the C note
  // in position 1 and confirming it's sharped by the accidental from the F Sharp at position 0
- expect(getTabPosition(keys, abcKeySignature, barContents, 1, tuning, true)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 1, TUNINGS.GUITAR_STANDARD, true)).toStrictEqual([{ str: 1, fret: 2 }]);
 });
 
 
@@ -136,7 +130,7 @@ test('grace note - natural accidental on grace note cancels out key sig sharp',
 
  // the natural accidental on the specific grace note should cancel out the sharp accidental in the key sig
  // should be str: 1, fret: 1 for F natural
- expect(getTabPosition(keys, abcKeySignature, barContents, 0, tuning, true)).toStrictEqual([{ str: 1, fret: 1 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 0, TUNINGS.GUITAR_STANDARD, true)).toStrictEqual([{ str: 1, fret: 1 }]);
 });
 
 
@@ -148,7 +142,7 @@ test('grace note - accidental on grace note does affect attached regular note',
 
  // The regular F has a F Sharp grace note attached (before) it, so the note should
  // become sharped, testing the regular note F so isGraceNote is false
- expect(getTabPosition(keys, abcKeySignature, barContents, 0, tuning, false)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 0, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 1, fret: 2 }]);
 });
 
 test('grace note - grace is not affected by accidental on attached regular note', () => {
@@ -159,7 +153,7 @@ test('grace note - grace is not affected by accidental on attached regular note'
 
  // Now it's a F Sharp regular note with a F grace note attached having no accidental. The
  // regular note comes "after" the grace not, so the grace note pitch should be F Natural
- expect(getTabPosition(keys, abcKeySignature, barContents, 0, tuning, true)).toStrictEqual([{ str: 1, fret: 1 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 0, TUNINGS.GUITAR_STANDARD, true)).toStrictEqual([{ str: 1, fret: 1 }]);
 });
 
 test('grace note - accidental on n-1 regular note will apply to regular note n over accidental on n-1 grace note', () => {
@@ -170,7 +164,7 @@ test('grace note - accidental on n-1 regular note will apply to regular note n o
  const barContents = [fSharpWithFFlatGraceNote, f];
 
  // The sharp accidental from the n-1 note should be applied, not the flat from the n-1 grace note
- expect(getTabPosition(keys, abcKeySignature, barContents, 1, tuning, false)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 1, TUNINGS.GUITAR_STANDARD, false)).toStrictEqual([{ str: 1, fret: 2 }]);
 });
 
 test('grace note - accidental on n-1 regular note will apply to grace note n over accidental on n-1 grace note', () => {
@@ -181,7 +175,7 @@ test('grace note - accidental on n-1 regular note will apply to grace note n ove
  const barContents = [fSharpWithFFlatGraceNote, f];
 
  // The sharp accidental from the n-1 note should be applied, not the flat from the n-1 grace note
- expect(getTabPosition(keys, abcKeySignature, barContents, 1, tuning, true)).toStrictEqual([{ str: 1, fret: 2 }]);
+ expect(getTabPosition(keys, abcKeySignature, barContents, 1, TUNINGS.GUITAR_STANDARD, true)).toStrictEqual([{ str: 1, fret: 2 }]);
 });
 
 // TEST FOR TIN WHISTLE

visual-tool/scripts/index.js

@@ -24,15 +24,15 @@ const allNottinghamTunes = Tunes1 + Tunes2 + Tunes3 + Tunes4 + Tunes5 + Tunes6 +
 const defaultRenderOptions = {
  xOffset: 3,
  widthFactor: 1.5,
- lineHeight: 185,
- clefWidth: 40,
- meterWidth: 30,
+ lineHeight: 190,
+ clefWidth: 60,
+ meterWidth: 25,
  repeatWidthModifier: 35,
- keySigAccidentalWidth: 20,
+ keySigAccidentalWidth: 10,
  tabsVisibility: 1,
  staveVisibility: 1,
  tabStemsVisibility: 0,
- voltaHeight: 25,
+ voltaHeight: 27,
  renderWidth: 650,
  tuning: AbcjsVexFlowRenderer.TUNINGS.GUITAR_STANDARD,
 };
@@ -93,7 +93,7 @@ tuning.onchange = (e) => {
 };
 
 const vexRendererWidth = 500;
-const vexRendererHeight = 1000;
+const vexRendererHeight = 2000;
 
 const customOptions = [];
 const nottinghamOptions = [];