vexflow icon indicating copy to clipboard operation
vexflow copied to clipboard

Improve Positioning API: Extract out current positioning code into stand-alone functions.

Open Silverwolf90 opened this issue 9 years ago • 4 comments

Currently, positioning in VexFlow is represented by various enums (eg. Modifier.Position) which map to specific code branches in each class. The user has absolutely no control over how each Position was implemented for each class.

For an example of this, take a look at StaveNote.prototype.getModifierStartXY. This method is problematic because each returned { x, y } pair comes with a non-obvious value. Calling stavenote.getModifierStartXY(Position.ABOVE, 1) gives no indication of the result of the x coordinate. Is it the stem's x value? The center of the notehead for the provided index? The center of the entire stavenote's bbox? What happens with chords with displaced notes? What about rests? None of this is obvious or configurable. Confusingly, the x value represents the "center of the non-displaced noteheads" regardless of the provided key index. And the y value is the same for Position.ABOVE and Position.BELOW.

What if we extracted these code branches into stand-alone functions?

A more flexible and powerful API might look something like:

const Position {
  H: {
    noteStem: (note) => note.getStem().getX(),
  },
  V: {
    noteStemTip: (note) => note.getStemExtents().topY;
  }
};

// Place articulation at stem tip
articulation.setInitialPosition(Position.H.noteStem, Position.V.noteStemTip);

With functions we get the advantage of composability:

const stemTipPadded = compose(withPadding(20), Position.H.noteStemTip);

higher-order functions:

const noteheadPosition =
  (position) => (keyIndex) => (chord) =>
    position(chord.getNotehead(keyIndex));

const noteheadCenter = noteheadPosition(Position.H.center);

more complex, but still low-level, positioning logic is separated from higher level classes

const centeredBetween = (startNote, endNote) => {
  const left = Position.H.left(startNote);
  const right = Position.H.right(endNote);
  const width = right - left; 
  return left + (width / 2);
}

To me, this seems much simpler, more flexible, more powerful, and more obvious.

Curious to hear thoughts on this.

Silverwolf90 avatar Jun 19 '16 18:06 Silverwolf90

So firstly, I hate the current positioning mechanism, and really wish that I had put in well-defined anchor points from the start.

I like your idea better, because you have anchor functions instead of anchor points, which brings in a lot of benefits (like composability.) (BTW, I also think that it's easy to go crazy with this approach and have difficult-to-read higher order functions.)

I'm wondering if we can take this a step further so as positions change as other elements join the context. E.g., if I want a modifier below a note, it renders underneath other modifiers that are already there. So the anchor functions are functions of both the element and the context.

0xfe avatar Jun 20 '16 20:06 0xfe

Thinking about it more, it seems like we're missing abstractions around both grouping and positioning. A lot of code in VexFlow is dedicated to both of these things, but they're being performed in slightly different ways in each class. It's likely that a lot of logic could be consolidated.

For grouping, we have temporal groups (TickContext), and formatting groups (ModifierContext), but we don't have a notion of simple graphical groups of glyphs/text/paths.

For positioning, we're in need of a much better design than what we currently have, as mentioned in the first post.

If VexFlow had better lower-to-mid level abstractions around positioning and grouping I bet we'd be able to simplify the codebase substantially. Any graphical class which consists mainly of formatting and drawing would benefit.

Edit: clarity

Silverwolf90 avatar Jun 30 '16 20:06 Silverwolf90

Yes, agreed. A few years ago, I started work on a Container class, which abstracted away the grouping logic because I wanted to build a new StaveContext for cross-stave formatting... but that fizzled. :-)

I'm all for new abstractions, especially those that will add more advanced formatting without the need for hard coded constants. An interesting experiment would be elements that know their own identities and contexts, and are responsible for positioning themselves -- e.g., at every iteration of some positioning loop, they would adjust their positions to minimize some cost function (which would include things like spacing, collisions, connections to anchor points, etc.)

How about we start collaborating on a high-level design/API for something like this? If we can do this incrementally (i.e., slowly move existing elements into the new design) that would be ideal.

0xfe avatar Jul 01 '16 11:07 0xfe

Personally, I would hesitate to think about high level ideas without a stronger low-level foundation. Part of the problem there is that the API's across objects are inconsistent and lack a common interface past preFormat/postFormat/render -- which are just lifecycle methods to trigger side-effects; they don't expose useful information.

A big improvement would be better bounding box support. Most objects don't expose one, but ideally every graphical object should expose their bounding box. From there, we can extract the common operations we're doing in every class (eg: collision avoidance, offset calculations, etc) into generic utilities which operate on bounding boxes. Once that's in place, we're in a better position to think about advanced formatting like you're describing.

Silverwolf90 avatar Jul 09 '16 08:07 Silverwolf90