Re: Bad slur shape at line break with change of clef

[Thomas Morley]

Am Sa., 12. Dez. 2020 um 21:07 Uhr schrieb Timothy Lanfear <timothy@lanfear.me>:
>
> The part of the slur before the line break is badly shaped when another
> staff has a clef change.
>
> \version "2.20.0"
>
> <<
>    \new Staff {
>      R1*2
>      r2 r4 r8 r16 r32 e''32( |
>      \break
>      d''8) r r4 r2 |
>      R1*3
>    }
>
>    \new Staff {
>      R1*3
>      \clef "bass"
>      R1*4
>    }
>  >>
>
>
> --
> Timothy Lanfear, Bristol, UK.
>
> _______________________________________________
> bug-lilypond mailing list
> bug-lilypond@gnu.org
> https://lists.gnu.org/mailman/listinfo/bug-lilypond

As far as I can tell it was introduced with

commit 16a328617e867892e8d5608f95bd6127196e03a2
Author: Keith OHara <k-ohara5a5a@oco.net>
Date:   Sun Jun 17 21:45:26 2012 -0700

    Consistent bounds for slurs; issue 427 and 379

in 2.15.41.

I reverted it maually with below, (my C++-skils are approx. zero, all
done by c/p and comments):

diff --git a/lily/include/slur-scoring.hh b/lily/include/slur-scoring.hh
index bddc6c3ce8..e63b23b71b 100644
--- a/lily/include/slur-scoring.hh
+++ b/lily/include/slur-scoring.hh
@@ -138,7 +138,8 @@ public:
   std::vector<Extra_collision_info> get_extra_encompass_infos () const;
   Real move_away_from_staffline (Real y, Grob *on_staff) const;

-  Interval breakable_bound_extent (Direction) const;
+  Grob *breakable_bound_item (Direction) const;
+//  Interval breakable_bound_extent (Direction) const;
 };

 void set_slur_control_points (Grob *me);
diff --git a/lily/slur-scoring.cc b/lily/slur-scoring.cc
index 8f98deedd7..93fc8ec232 100644
--- a/lily/slur-scoring.cc
+++ b/lily/slur-scoring.cc
@@ -451,12 +451,14 @@ Slur_score_state::get_best_curve () const
   return best;
 }

-Interval
-Slur_score_state::breakable_bound_extent (Direction d) const
+Grob *
+Slur_score_state::breakable_bound_item (Direction d) const
+// Interval
+// Slur_score_state::breakable_bound_extent (Direction d) const
 {
   Grob *paper_col = slur_->get_bound (d)->get_column ();
-  Interval ret;
-  ret.set_empty ();
+//  Interval ret;
+//  ret.set_empty ();

   extract_grob_set (slur_, "encompass-objects", extra_encompasses);

@@ -464,10 +466,11 @@ Slur_score_state::breakable_bound_extent
(Direction d) const
     {
       Item *item = dynamic_cast<Item *> (extra_encompasses[i]);
       if (item && paper_col == item->get_column ())
-        ret.unite (robust_relative_extent (item, common_[X_AXIS], X_AXIS));
+        return item;
+//        ret.unite (robust_relative_extent (item, common_[X_AXIS], X_AXIS));
     }
-
-  return ret;
+  return 0;
+//  return ret;
 }

 /*
@@ -583,12 +586,20 @@ Slur_score_state::get_base_attachments () const
           Real x = 0;
           Real y = 0;

-          Interval ext = breakable_bound_extent (d);
-          if (ext.is_empty ())
-            ext = Axis_group_interface::
-                  generic_bound_extent (extremes_[d].bound_,
-                                        common_[X_AXIS], X_AXIS);
-          x = ext[-d];
+          if (Grob *g = breakable_bound_item (d))
+            {
+              x = robust_relative_extent (g, common_[X_AXIS], X_AXIS)[RIGHT];
+            }
+          else if (d == RIGHT)
+            x = robust_relative_extent (extremes_[d].bound_,
common_[X_AXIS], X_AXIS)[d];
+          else
+            x = slur_->get_broken_left_end_align ();
+//          Interval ext = breakable_bound_extent (d);
+//          if (ext.is_empty ())
+//            ext = Axis_group_interface::
+//                  generic_bound_extent (extremes_[d].bound_,
+//                                        common_[X_AXIS], X_AXIS);
+//          x = ext[-d];

           Grob *col = (d == LEFT) ? note_columns_[0] : note_columns_.back ();

diff --git a/lily/spanner.cc b/lily/spanner.cc
index 5713c13d3f..bfd5765ef5 100644
--- a/lily/spanner.cc
+++ b/lily/spanner.cc
@@ -324,6 +324,34 @@ Spanner::is_broken () const
   return broken_intos_.size ();
 }

+/*
+  If this is a broken spanner, return the amount the left end is to be
+  shifted horizontally so that the spanner starts after the initial
+  clef and key on the staves. This is necessary for ties, slurs,
+  crescendo and decrescendo signs, for example.
+*/
+Real
+Spanner::get_broken_left_end_align () const
+{
+  Paper_column *sc = dynamic_cast<Paper_column *>
(spanned_drul_[LEFT]->get_column ());
+
+  // Relevant only if left span point is first column in line
+  if (sc != NULL
+      && sc->break_status_dir () == RIGHT)
+    {
+      /*
+        We used to do a full search for the Break_align_item.
+        But that doesn't make a difference, since the Paper_column
+        is likely to contain only a Break_align_item.
+      */
+      return sc->extent (sc, X_AXIS)[RIGHT];
+    }
+
+  return 0.0;
+}
+
+
+
 void
 Spanner::derived_mark () const
 {

Then I used the following ly-tests:

%%%% start
<<
   \new Staff {
     R1*2
     r2 r4 r8 r16 r32 e''32( |
     \break
     d''8) r r4 r2 |
     R1*3
   }

   \new Staff {
     R1*3
     \clef "bass"
     R1*4
   }
>>

%% from issue 427

\relative c'' {
  \key c \minor
  f e\( d( c~
  \break
  c d) e\) f
}

%% from issue 379
\relative c'' {
   \key g \major \partial 4
   \voiceOne
   g4( \break a)
}

%%%% end

I attach images with and without the revert.
While the revert fixes the reported problem, issues 427 and 379 are
present again.

Hopefully somebody with more knowledge, chimes in.

Cheers,
  Harm

atest-104.cropped-2.20.0.png
Description: PNG image

atest-104.cropped-2.23.0-with-revert.png
Description: PNG image