Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

return noteheads after build #1590

Merged
merged 1 commit into from
Jul 24, 2023
Merged

return noteheads after build #1590

merged 1 commit into from
Jul 24, 2023

Conversation

accelerationa
Copy link
Contributor

No description provided.

@AaronDavidNewman
Copy link
Collaborator

LGTM. @rvilarl , @ronyeh any objections?

@accelerationa
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Collaborator

ronyeh commented Jul 24, 2023

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
Copy link
Contributor Author

@AaronDavidNewman @ronyeh
can one of you answer this question? I would appreciate some guidence here #1590 (comment)

@ronyeh ronyeh merged commit 6208749 into 0xfe:master Jul 24, 2023
2 checks passed
@ronyeh
Copy link
Collaborator

ronyeh commented Jul 24, 2023

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
Copy link
Collaborator

ronyeh commented Jul 24, 2023

image

Congrats BTW. This was the 800th PR!

@AaronDavidNewman
Copy link
Collaborator

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants