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

Fix ornament delayed draw, such as on a grand staff. #1598

Merged
merged 5 commits into from
Oct 5, 2023

Conversation

RyoSusami
Copy link
Contributor

When building a TickContext with multiple voices, such as a grand staff, the order of ticks was not continuous, so detection of the end of voice element failed and the Ornament display position was incorrect.

Before
image

After
image

@rvilarl
Copy link
Collaborator

rvilarl commented Oct 3, 2023

Hi @RyoSusami, thanks for that! Would it be possible to add the testcase?

@RyoSusami
Copy link
Contributor Author

Additionally, fix considering the modifier width at the beginning of stave.

Before
image

After
image

@RyoSusami
Copy link
Contributor Author

@rvilarl
I have never created a VexFlow test case, but I will check it and try.
Is there anything I should be careful about?

@rvilarl
Copy link
Collaborator

rvilarl commented Oct 5, 2023

just start duplicating one within ornament_test.ts

@RyoSusami
Copy link
Contributor Author

@rvilarl
Added test case.
This is what the changes with this fix look like.

Before
image

After
image

@RyoSusami
Copy link
Contributor Author

I noticed that in the case above, the TickID order of TickContexts is 0,8192,4096, and the next context at the beginning of the lower voice is 8192, so the turn position is closer to the right than expected.
Need different approach than TickContext.getNextContext(context)?

@RyoSusami
Copy link
Contributor Author

Improved selection of nextContext.

image

@rvilarl
Copy link
Collaborator

rvilarl commented Oct 5, 2023

Thanks a lot @RyoSusami !

@rvilarl rvilarl merged commit 7e7eb97 into 0xfe:master Oct 5, 2023
2 checks passed
RyoSusami added a commit to RyoSusami/vexflow that referenced this pull request Oct 6, 2023
@RyoSusami RyoSusami deleted the hotfix/ornament_delayed_draw branch October 6, 2023 23:50
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.

None yet

2 participants