> Le 07/09/2022 13:02 CEST, Martín Rincón Botero <martinrinconbot...@gmail.com> > a écrit : > > > Hi, > > this question is related to my last merge request > https://gitlab.com/lilypond/lilypond/-/merge_requests/1611. To my surprise, > there seem to be strong objections against it, despite being an improvement > in all tests. I wanted to know more about penalties in > slur-configuration.cc in general. The idea for tweaking the penalty in the > case of the presence of dots is not mine at all. In the function > score_edges, amongst others, we find that the penalty (named "demerit" > here) for a specific case involving stems is tweaked so (line 464 of > current master): > > Real demerit = factor * dy; > if (state.extremes_[d].stem_ > && state.extremes_[d].stem_dir_ == state.dir_ > // TODO - Stem::get_beaming() should be precomputed. > && !Stem::get_beaming (state.extremes_[d].stem_, -d)) > * demerit /= 5;* > > To me this signals that the programmer is aware that there isn't a magical > formula to imitate hand engraving and particular cases will need a > different penalty. Is there something I'm missing? Is the penalty reduced > here in this particular case to fulfill a higher purpose or is that a case > of "last resort fix"? This kind of "tweaking" is all over the code, so it's > difficult not to interpret that as something that is a natural part of the > way penalties are handled. For any information in this regard that helps me > understand the code better I'm thankful.
I can't immediately tell if this bit of code looks good or in need of improvement, but adding something similar in the case of slurs on dotted notes is solving the wrong problem. The problem is that while trying to avoid a collision with a dot, the slur pushes itself away too far from it, while it could have avoided the collision and still be much closer. Tweaking the penalty for dots means telling the slur that a collision with the dot isn't all that important, whereas what we want to tell it is that it doesn't need to go so far to avoid the dot. So it is not solving the root problem, but masking it, until the root problem reappears in a different form. (See the attachment for a more humoristic view.) Having never worked on this code, I can't give much useful information about it, sorry. (A year ago, I would have jumped into the code to try to solve the problem myself so there would be a happy ending to the criticism on the merge request, but doing that has exhausted me a few times, so I would like not to do it this time.) Thanks, Jean
