0xfe / vexflow

A JavaScript library for rendering music notation and guitar tablature.
http://www.vexflow.com
Other
3.82k stars 657 forks source link

return noteheads after build #1590

Closed accelerationa closed 10 months ago

AaronDavidNewman commented 10 months ago

LGTM. @rvilarl , @ronyeh any objections?

accelerationa commented 10 months ago

LGTM. @rvilarl , @ronyeh any objections?

Thank you.

Just to add some context for this PR: I am primarily interested in gaining finer control over the positioning of Noteheads within the StaveNote context. Specifically, my use case necessitates the ability to set the x and y coordinates of a Notehead after it has already been created, followed by drawing the Notehead

If there are alternative approaches to achieve this goal without this change, please let me know.

accelerationa commented 10 months ago

I also considered setting x and y as a part of Notehead initialization, however, I am concerned this will break dependent callers, as this approach gives default x and y values

AaronDavidNewman commented 10 months ago

Now that I'm thinking about it a bit more, I guess I don't see the need for this. The noteheads are built automatically, and you can access them from the noteHeads property of stavenotes. Will that work?

accelerationa commented 10 months ago

Now that I'm thinking about it a bit more, I guess I don't see the need for this. The noteheads are built automatically, and you can access them from the noteHeads property of stavenotes. Will that work?

I have a use case where I need to draw a staveNote twice. The second time, I want to draw it in a different color. My solution is to use the buildNoteHeads function to get the noteheads, set the x and y positions manually, and then draw the noteheads.

const noteheads = staveNote.buildNoteHeads();
noteheads[0].setX(staveNote.getNoteHeadBeginX());

noteheads[0].setY(staveNote.getYs()[0]);
staveNote.setContext(renderContext);
staveNote.drawNoteHeads();

(Note: if I don't set x and y manually, head_x and y are 0s, I can't explain why)

I have a feeling what I do is not correct. I would appreciate any guidance on how to accomplish this more effectively.

ronyeh commented 10 months ago

My feedback is "why not"? :-) The current function returns nothing, and if there is a better thing to return, then we should consider doing it.

Otherwise, I don't see what's wrong with making this function slightly more convenient.

I see that noteHeads is normally private (and in VexFlow 5, we made it private so far since no one has declared that they want access to it). But I'm OK with providing more access to internals if it actually helps developers get their job done.

If we end up merging this, and it proves useful, we can cherrypick it over to V5 as well.

accelerationa commented 10 months ago

@AaronDavidNewman @ronyeh can one of you answer this question? I would appreciate some guidence here https://github.com/0xfe/vexflow/pull/1590#issuecomment-1647111650

ronyeh commented 10 months ago

Now that I'm thinking about it a bit more, I guess I don't see the need for this. The noteheads are built automatically, and you can access them from the noteHeads property of stavenotes. Will that work?

I have a use case where I need to draw a staveNote twice. The second time, I want to draw it in a different color. My solution is to use the buildNoteHeads function to get the noteheads, set the x and y positions manually, and then draw the noteheads.

const noteheads = staveNote.buildNoteHeads();
noteheads[0].setX(staveNote.getNoteHeadBeginX());

noteheads[0].setY(staveNote.getYs()[0]);
staveNote.setContext(renderContext);
staveNote.drawNoteHeads();

(Note: if I don't set x and y manually, head_x and y are 0s, I can't explain why)

I have a feeling what I do is not correct. I would appreciate any guidance on how to accomplish this more effectively.

Can you describe the use case a bit more, or provide an image showing the result you are looking for?

So you want the exact same staveNote drawn twice on the staff, in different places?

ronyeh commented 10 months ago
image

Congrats BTW. This was the 800th PR!

AaronDavidNewman commented 10 months ago

@accelerationa, if you just want to change the color of the note head, or animate it, it's probably easiest to do with CSS. Look at the style_tests.ts to see how to do this. It's not clear to us why you want to draw the note head twice, or what you're going to do with the x coordinate when you have it.

To answer your question about X coordinate: you need to create a formatter and format the music before you have X coordinates. See fomatter_tests.ts, for instance the noteHeadPadding test, for how to do this. Once you have called formatter.format, you can use getAbsoluteX on stavenote and the note heads to get the X coordinate. The y is not available until after the notes are rendered.

Until you have a formatter and had it format your voices, there are no coordinates associated with the notes at all, which is maybe why you are getting 0.

Try to play around with the tests cases in the 'tests' directory, there is probably something there that is close to what you want to do.