fix #8970: remove space between accidentals and arpeggios

[Nick-Mazuk]

Resolves: #8970

Accidentals have a quarter space padding around them. This creates an unnecessarily large gap between accidentals and arpeggios. This PR decreases that gap by a quarter space, making the gap the same size as if the accidentals weren't there.

Before:

After:

Currently, the decrease of a quarter space is hardcoded in. Alternatively, we can use the same value that defines the spaces between accidentals. I have not yet found where this is defined, though.

[Nick-Mazuk]

I could go either way. I tried both and went with 0.25 spaces because then the gap size is identical with and without accidentals.

[Jojo-Schmitz]

Fair enough. To me in the images from original issue's score the distance between barline and arpeggio is still too small with 0.25. But that's nothing this PR here touches on, not directly at least. And in your example that's not an issue either.

[Nick-Mazuk]

Just added a commit that does a few things:

1. Shrinks gap to just 0.25 spaces
2. Removes the hard coded magic number for the accidental padding
3. Added a style setting for the gap size. It's currently not exposed to the user, but it can be in the future if we wish

[Nick-Mazuk]

This is a complete rewrite of the feature, but I believe it's finalized.

1. Increases gap back to 0.5 spaces
2. Decreases the gap for ♯ by the cutout to make it visually consistent with ♭
3. The gap follows the contour of the arpeggio and accidentals (i.e., if an accidental can fit closer to the arpeggio than it's bounding box, it will be closer)

For instance, take a look at this image. The sharp now fits inside of the bracket because there's space!

All test cases:

Here's the test file I used to validate the changes.

accidental-test.mscz.zip

[Nick-Mazuk]

; missing

Fixed

[Nick-Mazuk]

Assuming this looks good with everyone, I'm done working on it and it can be merged.

[Jojo-Schmitz]

You may want to squash commits. Those last 3 commits should really be just one

[Nick-Mazuk]

Fixed typo and squashed the commits

[Nick-Mazuk]

@Jojo-Schmitz, resolved your comments about the unreachable breaks and qreals.

[Nick-Mazuk]

These last few updates take care of every last edge case I can think of, including checking the output in multiple fonts. There are also a few minor tweaks as requested by Simon.

There's also some code refactoring to make sure it's easier to read.

Here's what the original test cases look like now:

And with Petaluma, a font with completely different metrics:

[vpereverzev]

At the moment we're trying to reduce dependencies from Qt in engraving module. Make sure that you're using STL containers here and in the further PRs

[Nick-Mazuk]

Ok, will do. If I notice that there's an unneeded, existing Qt dependency, I assume I should convert it to a STL container?

[Jojo-Schmitz]

In my backport of this PR to 3.x that include was not needed (Neither was the cmath) and 3.x's all.h doesn't either.

You're actually using a QVector in chord.cpp too but not the include. And in arpeggio.cpp QVector was used even before your PR. So that include seems not needed at all.

[DmitryArefiev]

Assigning to @vshalkevich

[vshalkevich]

Checked on Linux Ubunty, Windows 10, MacOS
Works fine