Fixing some quirky slur rendering

[RhinoHaggis]

I'd like to start getting familiar with the MuseScore codebase, but I'm not a real coder, and I was hoping I could get a little help. I've contrived a silly project and maybe someone can help direct me.

Here is a largely fictional bug report about slur rendering:

1. Looping a slur over itself exposes an unfortunate underlying rendering geometry
2. Bringing a slur baseline vertical makes the slur stop responding, and then vanish. Bad things can be done to the control points, and to no effect.

Now. This is actually true:

the party slur.mscz.zip

However, it's hard to imagine a usage case for either.

This is an excuse to dig into the codebase. I am not a coder, but I am a mathematician, and I know a fair amount about splines. I'd like to find where the slur shape and rendering are defined from the control points and see how you've implemented it. I generally expect rendering to be highly optimized, but there's always a chance I can clean up the behavior and make things a bit more efficient... and easily extensible to multi-segment splines, allowing more control over the curvature (especially for long slurs), and allow more complex slur shapes.

Can someone help me get oriented?

[RhinoHaggis]

I'm expecting the slur render to be deliberate: a simplification that is not exposed under any normal usage, and that avoids the extra calculation needed to (correctly) define the slur as a closed planar figure, and fill it as such. However, the slurs crapping out when the baseline goes vertical makes me suspect the spline is defined in an unfortunate and costly way, and that we might be able to fix the latter, with enough savings left over to correct the former... and still come out ahead.

[RhinoHaggis]

Ok well in the meantime…
what I see on the surface is an order 3 Bézier curve and its four defining points, the bounding box (unrestricted quadrilateral) of the slur, with added midpoints on the baseline and upper line, to shift the whole slur and the general upper shape, respectively, but having no contribution to the defining equation.

[RhinoHaggis]

So then there is the question of how you’ve defined the width.
-Upper curve and lower curve, and a method of filling the space between?
-A single spline down the center, with a fattening parameter along its Normal?

The behavior of the slur on the right (above) seems to rule out the latter. I have a guess how the shading is being done, based on this curious behavior, so i’ll go see if I can reproduce it from scratch.

If someone can direct me to these parts of the code I’d be obliged 🙏

[cbjeukendrup]

@RhinoHaggis Sorry that I couldn't reply sooner, but here are some pointers:

• The basic bezier calculation takes place in SlurTieLayout::computeBezier:
  • For slurs:

    MuseScore/src/engraving/rendering/score/slurtielayout.cpp

    Line 2136 in ace8209

    void SlurTieLayout::computeBezier(SlurSegment* slurSeg, PointF shoulderOffset)

  • For ties:

    MuseScore/src/engraving/rendering/score/slurtielayout.cpp

    Line 2054 in ace8209

    void SlurTieLayout::computeBezier(TieSegment* tieSeg, PointF shoulderOffset)

• But there is a whole lot of logic around it: check where computeBezier is called, or, more generally, where bezier occurs in slurtielayout.cpp
• Drawing happens in TDraw::draw(const SlurSegment* item, Painter* painter), but that's quite boring, because that just draws the path calculated during the layout phase

To my regret, I'm personally not very familiar with the details, but I hope that this gets you started!
And, @mike-spa knows a lot about this area!

[RhinoHaggis]

thank you!!

[RhinoHaggis]

Ok my first question is are you committed to all of these atan() and Rotate() calls? All rotations can be performed by direct transformation of the x, y components, with a simple component wise (parallel) MADD, without ever calculating the angle. This will also bulletproof you against any unfortunate behavior if the user moves the control points in a way you don’t expect (atan() can’t tell left from right).

[mike-spa]

@RhinoHaggis the slur is defined by a Bezier curve. To be more precise, an outer bezier curve, and an inner bezier curve. The two curves share the same start and end points, but split in the middle as the inner curve has a lower shoulder height. The painter then fills the region between the two curves and that gives us the tapered shape (thick in the middle, thin at the endpoints). This isn't perfect, and it certainly won't work for arbitrary shapes if you go crazy with the shoulder points, but it's also very simple and efficient and it works for all of our reasonable use cases, so I don't think it's worth overengineering that aspect.

You are almost certainly correct about the arc tangent though, which obviously breaks down at 90 degrees. It's used in a bunch of different places around slurs. If you want to try improving the math there, you're welcome to! Just be sure that the end result is the same (when you'll make a PR, the vtests will tell us). Also, I have some big changes in the pipeline to improve the slur anti-collision algorithm (SlurTieLayout::avoidCollisions), so maybe don't touch too much the code around there or we'll step on each other :)

Also, please log an actual issue about that disappearing slur.