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

stretchForce works with the staveProfile values 3 or 4 #394

Closed
nikolayrantsev opened this issue Aug 26, 2020 · 7 comments · Fixed by #397
Closed

stretchForce works with the staveProfile values 3 or 4 #394

nikolayrantsev opened this issue Aug 26, 2020 · 7 comments · Fixed by #397
Assignees
Labels
area-rendering Everything related to the rendering platform-javascript Related to the JavaScript version of alphaTab state-wrong-template The wrong template was used (e.g. bug reported as question) type-bug 🕷️
Milestone

Comments

@nikolayrantsev
Copy link

nikolayrantsev commented Aug 26, 2020

Expected Results

stretchForce settings with all staveProfile values should work

Observed Results

If staveProfile is 1 or 2 (display with the scores), the stretchForce doesn't affect the fitting

Steps to Reproduce (for bugs)

  1. I've added display settings to the sample final file https://www2.alphatab.net/docs/tutorial-web/player#final-file

display: { staveProfile: 1, stretchForce: 0.5 },

  1. stretchForce values less than 1 (I want to include more bars in the raw) works when staveProfile values are
    3 - Tab
    4 - TabMixed
    only.

Your environment (if applicable)

  • Version used: https://www2.alphatab.net/docs/tutorial-web/player#final-file
  • Platform used: JavaScript
  • Rendering engine used: SVG (default),
  • Browser Name and Version: Chrome Version 84.0.4147.135 (Official Build) (64-bit)
  • Operating System and version (desktop or mobile): macOS 10.15.1 (19B88)
  • Link to your project: local
@Danielku15 Danielku15 self-assigned this Aug 29, 2020
@Danielku15 Danielku15 added state-wrong-template The wrong template was used (e.g. bug reported as question) type-bug 🕷️ area-rendering Everything related to the rendering platform-javascript Related to the JavaScript version of alphaTab labels Aug 29, 2020
@Danielku15 Danielku15 added this to the 1.1 milestone Aug 29, 2020
@Danielku15
Copy link
Member

This looks more like a bug report than a question. Can you please update your question to the "bug" template?

As general hint: A "Question" should rather be something which is unclear on the usage. Either you do not know at all how to do something or docs might be unclear. A "Bug" should be reported in case something is not working as expected. You have an actual and expected behavior and it is reproducible with some dedicated steps/settings. If a some functionality is fully missing it would need to be a "Feature Request".

@nikolayrantsev
Copy link
Author

Thank you,
The only reason I've submitted this as a Question - it might be my own incorrect implementation of the settings,
Updated the description as a bug

@Danielku15
Copy link
Member

It's fine to report it as bug if you have the feeling it's a misbehavior. I might just flag it as "no-bug" and provide the details in case it turns out to be the right behavior as it is 😉 The "bug" template is just more optimized to allow me reproducing the scenario you're experiencing.

I started to look already into this topic and I fear the issue is way deeper in the layout algorithm than just an issue with particular stave profiles. alphaTab pre-calculates how much space is needed at minimum to place some notation elements. This way it ensures there are no overlaps with notes and accidentals. As soon as a standard notation is visible, different minimum widths are registered to have enough space. In this case it seems that alphaTab thinks it must reserve more space than actually needed and overrules your preferred spacing of 0.5.

It was a rather quick fix to allow a more dense layout, but this caused some issues that accidentals are overlapping with previous notes if it gets a bit more dense. This actually made the overall display of some notes already more dense by default which fits your initial goal. I am fighting currently with the Gourlay layout algorithm to get all spacings and minimum widths right to avoid collisions.

@Danielku15
Copy link
Member

Phew. This issue turned out to be more effort than expected. 😅 I retested and checked in detail the spacing and sizing of notes and had to rework quite some bits. Now alphaTab really calculates correctly the required minimum widths of notes and ensures the right positioning of elements accordingly. This allows you to specify smaller stretch forces which allows you almost to squeeze notes that dense that there is no white-space between them anymore.

Also this rework results in some cases in a more dense layouting by default.

@nikolayrantsev
Copy link
Author

Files changed 90 ....

Daniel, thank you so much!

@Danielku15
Copy link
Member

The effective change is only ~5 files. 16 other files were rather changed to make things cleaner or render some debug output. The remaining files are updating the reference files of the visual regression tests 😉

@rhclayto
Copy link

rhclayto commented Nov 3, 2020

Awesome developer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-rendering Everything related to the rendering platform-javascript Related to the JavaScript version of alphaTab state-wrong-template The wrong template was used (e.g. bug reported as question) type-bug 🕷️
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants