Am Fr., 15. Apr. 2022 um 13:27 Uhr schrieb Jean Abou Samra <j...@abou-samra.fr>:
>
>
>
> Le 15/04/2022 à 13:17, Thomas Morley a écrit :
> > Am Fr., 15. Apr. 2022 um 12:52 Uhr schrieb Jean Abou Samra 
> > <j...@abou-samra.fr>:
> >>
> >>
> >> Le 15/04/2022 à 11:45, Thomas Morley a écrit :
> >>> Am Fr., 15. Apr. 2022 um 10:39 Uhr schrieb Jean Abou Samra 
> >>> <j...@abou-samra.fr>:
> >>>>
> >>>> Le 15/04/2022 à 10:13, Thomas Morley a écrit :
> >>>>> Am Mi., 13. Apr. 2022 um 17:57 Uhr schrieb Jean Abou Samra 
> >>>>> <j...@abou-samra.fr>:
> >>>>>> Le 13/04/2022 à 17:03, Thomas Morley a écrit :
> >>>>>>> Am Di., 12. Apr. 2022 um 11:54 Uhr schrieb Jean Abou Samra 
> >>>>>>> <j...@abou-samra.fr>:
> >>>>>>>> Le 12/04/2022 à 11:16, Thomas Morley a écrit :
> >>>>>>> [...]
> >>>>>>>>> In many details DurationLine was tailored after Glissando and
> >>>>>>>>> Glissando _is_ effected by the breathing sign.
> >>>>>>> [...]
> >>>>>>>
> >>>>>>>> Well, if the goal is to have DurationLine similar to Glissando,
> >>>>>>>> how about reusing line-spanner-interface code? I haven't reviewed
> >>>>>>>> the code very thoroughly, but I assumed there was a difference
> >>>>>>>> warranting a different code path. If you put
> >>>>>>>> ly:line-spanner::calc-{left,right}-bound-info in
> >>>>>>>> DurationLine.{left,righ}-bound-info, that gives you X values for free
> >>>>>>>> in the bound info properties. Would that solve the problem?
> >>>>>>> If I go for ly:line-spanner::calc-{left,right}-bound-info then
> >>>>>>>
> >>>>>>> \layout {
> >>>>>>>        \context {
> >>>>>>>          \Voice
> >>>>>>>          \consists Duration_line_engraver
> >>>>>>>        }
> >>>>>>> }
> >>>>>>>
> >>>>>>> { b1\- }
> >>>>>>>
> >>>>>>> errors with
> >>>>>>>         "programming error: extremal broken spanner's bound has no 
> >>>>>>> parent
> >>>>>>> vertical axis group"
> >>>>>>> More complex examples may add:
> >>>>>>>         "programming error: bound item has no parent vertical axis 
> >>>>>>> group"
> >>>>>>> Both coming from `ly:line-spanner::calc-right-bound-info' based upon
> >>>>>>> `Line_spanner::calc_bound_info' in /lily/line-spanner.cc
> >>>>>>>
> >>>>>>> As a mere user I'd say: "Nice error-message, but why should I 
> >>>>>>> care...?"
> >>>>>>>
> >>>>>>> As a programmer, I doubt the example above can be made working with
> >>>>>>> ly:line-spanner::calc-right-bound-info.
> >>>>>>> At least, I don't know how.
> >>>>>> Oh, I'm sorry, I missed that detail of my own code. A DurationLine will
> >>>>>> always be horizontal, thus you can and should use
> >>>>>> ly:horizontal-line-spanner::calc-{left,right}-bound-info (with 
> >>>>>> 'horizontal'
> >>>>>> in the name).  The difference is that the functions without 
> >>>>>> 'horizontal'
> >>>>>> try to compute vertical positions. This falls apart here, for 
> >>>>>> understandable
> >>>>>> reasons: the right bound of your DurationLine is a 
> >>>>>> NonMusicalPaperColumn,
> >>>>>> so how would one tell where the line should end vertically?
> >>>>> Using ly:horizontal-line-spanner::calc-{left,right}-bound-info makes
> >>>>> for huge simplifications.
> >>>>> Alas, it introduces a minor and reintroduces a major problem.
> >>>>>
> >>>>> The minor: the X-value depends now on attach-dir. For the start of the
> >>>>> DurationLine this is ofcourse RIGHT. But if starting at a skip,
> >>>>> left-bound is PaperColumn there we would want LEFT. Can be fixed by
> >>>>> accurate conditions. Count it as done.
> >>>>>
> >>>>> The major: `ly:horizontal-line-spanner::calc-right-bound-info' and
> >>>>> `unbroken-or-last-broken-spanner?' (and friends) don't agree what's
> >>>>> the last part of the spanner.
> >>>>> To illustrate:
> >>>>>
> >>>>> \version "2.23.7"
> >>>>>
> >>>>> \layout {
> >>>>>      \context {
> >>>>>        \Voice
> >>>>>        \consists "Duration_line_engraver"
> >>>>>      }
> >>>>> }
> >>>>>
> >>>>> {
> >>>>>     \override DurationLine.bound-details.right.foo =
> >>>>>       "I am last part (bound-details)"
> >>>>>     \override DurationLine.bound-details.right-broken.foo =
> >>>>>       "I am not last part (bound-details)"
> >>>>>
> >>>>>     \override DurationLine.after-line-breaking =
> >>>>>     #(lambda (grob)
> >>>>>       (pretty-print
> >>>>>         (if (unbroken-or-last-broken-spanner? grob)
> >>>>>             "I am last part (unbroken-or-last-broken-spanner?)"
> >>>>>             "I am not last part (unbroken-or-last-broken-spanner?)"))
> >>>>>       (pretty-print
> >>>>>         (assoc-get 'foo 
> >>>>> (ly:horizontal-line-spanner::calc-right-bound-info grob))))
> >>>>>
> >>>>>     b1\-
> >>>>> }
> >>>>>
> >>>>> =>
> >>>>> "I am last part (unbroken-or-last-broken-spanner?)"
> >>>>> "I am not last part (bound-details)"
> >>>>>
> >>>>> The problem occurs if we have a broken DurationLine with end-items
> >>>>> running to the very end of the score.
> >>>>> To fix it one would again need something like:
> >>>>>
> >>>>> {
> >>>>>      \override DurationLine.bound-details =
> >>>>>      #(grob-transformer 'bound-details
> >>>>>        (lambda (grob orig)
> >>>>>         (if (end-broken-spanner? grob)
> >>>>>             (cons
> >>>>>               (cons 'right-broken
> >>>>>                 (cons '(end-style . arrow) (assoc-get 'right-broken 
> >>>>> orig)))
> >>>>>               orig)
> >>>>>             orig)))
> >>>>>      b1\-
> >>>>>      \break
> >>>>>      s1
> >>>>>      \break
> >>>>>      s1
> >>>>>      \bar "|."
> >>>>> }
> >>>>>
> >>>>> Or like \lastEndStyle which you cleared with
> >>>>>
> >>>>> commit 46671d13257f6ad68d1778a1cc850e59116c856a
> >>>>> Author: Jean Abou Samra <j...@abou-samra.fr>
> >>>>> Date:   Wed Dec 29 00:16:46 2021 +0100
> >>>>>
> >>>>>        Fix broken spanner functions
> >>>>>
> >>>>>        They were broken (pun intended) on spanners of which one bound 
> >>>>> is a
> >>>>>        non-musical column at a system boundary.  For example,
> >>>>>        unbroken-or-last-broken-spanner? would return false on a spanner
> >>>>>        running to the NonMusicalPaperColumn at the end of a score.  This
> >>>>>        fixes the known issue with duration lines running to the end of 
> >>>>> the
> >>>>>        piece, and will be needed in a refactoring of spanner-placement
> >>>>>        treatment for balloon-interface.
> >>>>>
> >>>>>        While at it, document these functions.
> >>>>>
> >>>>> This is unfortunate. Any chance making
> >>>>> `ly:horizontal-line-spanner::calc-right-bound-info' and friends
> >>>>> identify the last spanner-part correctly, i.e. like
> >>>>> `unbroken-or-last-broken-spanner?'
> >>>>> ?
> >>>>
> >>>> Ah, this one is showing up again. Yes, this is not too hard to
> >>>> fix (but please allow me some time).
> >>>>
> >>> No problem.
> >>> If you don't mind I'll open an issue for it with the code-example above.
> >>> Meanwhile I found a different workaround, without the need to have the
> >>> apply an extra override.
> >>> If testings suceed, I'll upload a patch with the new code for 
> >>> DurationLine.
> >>> The workaround could be removed later easily.
> >>
> >> The attached patch fixes it for me.
> >>
> >> Thanks,
> >> Jean
> > Will test in the evening, real life will occupy me the afternoon.
> > Though, at first glance:
> > I fear using "unbroken-or-first-broken-spanner?" will not cover middle
> > part(s) of a multiple broken spanner.
> > Shouldn't it be "unbroken-or-not-last-broken-spanner?"
> > ?
> > To be defined as:
> > (define-public (unbroken-or-not-last-broken-spanner? spanner)
> >    "Is @var{spanner} either unbroken or not the last of its broken
> > siblings?"
> >    (check-broken-spanner
> >     #t
> >     (lambda (siblings)
> >       (not (eq? spanner (last siblings))))
> >     spanner))
> >
> > Though, not tested.
>
>
>
> Are you sure? unbroken-or-first-broken-spanner? is used to decide
> between left and left-broken. Only the first broken spanner should
> use left and not left-broken, all the others need left-broken on
> top of left.

My concern at first glance was for the right side: Only the last
broken spanner should use right, all the others right-broken.
Though, I've tested it now, doing a full regtest-comparison with your
patch on top of my own (patch attached).
All works nicely, afaict.

Now, how should we upload our work?
I.e. in which sequence and who does it?
I'd volunteer to upload both, although I'm not sure how gitlab deals
with patches from different authors in one MR (or doing it
separately?)

Cheers,
  Harm
From d0253ac7b8feaaf5fa41097e37959f37ad346c47 Mon Sep 17 00:00:00 2001
From: Thomas Morley <thomasmorle...@gmail.com>
Date: Mon, 11 Apr 2022 00:48:55 +0200
Subject: [PATCH] Simplify duration-line::calc

Let ly:horizontal-line-spanner::calc-left/right-bound-info do most
work to get all the needed values.  Making it possible to remove
huge parts of manual calculations.
To make this work attach-dir is set in bound-details left/right.
While on introducing details.extra-dot-padding to give the user
the possibility to only tweak left-padding for DurationLine
starting at rests.
Some FIXME and TODOs are cleared.
Because ly:horizontal-line-spanner::calc-left/right-bound-info and
unbroken-or-last-broken-spanner? don't agree about the last part
of a (broken) DurationLine running to the very end of the score
a workaround is coded.  This is important if end-style is wanted
as 'hook or 'arrow for this last part.
---
 scm/define-grobs.scm |  11 ++--
 scm/output-lib.scm   | 154 +++++++++++--------------------------------
 2 files changed, 47 insertions(+), 118 deletions(-)

diff --git a/scm/define-grobs.scm b/scm/define-grobs.scm
index c6312c9d7c..58659a5156 100644
--- a/scm/define-grobs.scm
+++ b/scm/define-grobs.scm
@@ -1123,15 +1123,17 @@ contain mixed durations.  See also @iref{PercentRepeat},
         (arrow-length . 2)
         (bound-details
          .
-         ((right . ((end-on-accidental . #t)
+         ((right . ((attach-dir . -1)
+                    (end-on-accidental . #t)
                     (end-on-arpeggio . #t)
                     (padding . 0.4)
                     ;; possible values for endstyle: arrow, hook
                     (end-style . #f)))
           (right-broken . ((padding . 0.4)
                            (end-style . #f)))
-          (left-broken . ((padding . 0.4)))
-          (left . ((padding . -0.3)
+          (left-broken . ((padding . 0.5)))
+          (left . ((attach-dir . 1)
+                   (padding . -0.3)
                    (start-at-dot . #f)))))
         (breakable . #t)
         (details
@@ -1139,7 +1141,8 @@ contain mixed durations.  See also @iref{PercentRepeat},
          ((hook-height . 0.34)
           ;; Unless set by the user, grob's thickness is taken as default
           (hook-thickness . #f)
-          (hook-direction . ,UP)))
+          (hook-direction . ,UP)
+          (extra-dot-padding . 0.5)))
         (minimum-length . 2)
         (minimum-length-after-break . 6)
         (springs-and-rods . ,ly:spanner::set-spacing-rods)
diff --git a/scm/output-lib.scm b/scm/output-lib.scm
index 4211456b9f..1b681d5ed7 100644
--- a/scm/output-lib.scm
+++ b/scm/output-lib.scm
@@ -2590,26 +2590,17 @@ The final stencil is adjusted vertically using @var{staff-space}, which is
                      (grob::has-interface x 'rest-interface)))))
          (staff-space (ly:staff-symbol-staff-space grob))
          (staff-line-thickness (ly:staff-symbol-line-thickness grob))
-
          (grob-layout (ly:grob-layout grob))
          (blot-diameter
           (ly:output-def-lookup grob-layout 'blot-diameter))
-         (line-width
-          (ly:output-def-lookup grob-layout 'line-width))
          (style (ly:grob-property grob 'style 'beam))
          (thickness
           (ly:grob-property grob 'thickness 2))
          (used-thick (* thickness staff-line-thickness))
-         (bound-details
-          (ly:grob-property grob 'bound-details))
          (left-bound-details
-          (if (unbroken-or-first-broken-spanner? grob)
-              (assoc-get 'left bound-details)
-              (assoc-get 'left-broken bound-details)))
+          (ly:horizontal-line-spanner::calc-left-bound-info grob))
          (right-bound-details
-          (if (unbroken-or-last-broken-spanner? grob)
-              (assoc-get 'right bound-details)
-              (assoc-get 'right-broken bound-details)))
+          (ly:horizontal-line-spanner::calc-right-bound-info grob))
          (start-at-dot?
           (assoc-get 'start-at-dot left-bound-details #f))
          (end-on-accidental?
@@ -2621,83 +2612,44 @@ The final stencil is adjusted vertically using @var{staff-space}, which is
          (right-padding
           (assoc-get 'padding right-bound-details 0))
          (right-end-style
-          (assoc-get 'end-style right-bound-details #f))
+          (if (end-broken-spanner? grob)
+              (assoc-get 'end-style
+                (assoc-get 'right (ly:grob-property grob 'bound-details)))
+              (assoc-get 'end-style right-bound-details #f)))
     ;;;;;;;;;;;;;;;;;;;;;;;;
     ;;;; DurationLine start
     ;;;;;;;;;;;;;;;;;;;;;;;;
          (left-bound (ly:spanner-bound grob LEFT))
-         ;; FIXME: misnamed because it's sometimes a PaperColumn.
-         ;; Investigate. --JeanAS
-         (left-note-column
+         (left-column
           (if (note-head-or-rest? left-bound)
               (ly:grob-parent left-bound X)
               left-bound))
-         (stem (ly:grob-object left-note-column 'stem #f))
+         (stem (ly:grob-object left-column 'stem #f))
          (stem-thick
           (if stem
               (ly:grob-property stem 'thickness)
               0))
-         (dir (if (ly:grob? stem)
-                  (ly:grob-property stem 'direction)
-                  0))
-         ;; We need to care about suspended NoteHeads to start DurationLine
-         ;; correctly.
-         ;; A NoteHead is suspended, if its grob-extent relative to the
-         ;; parent NoteColumn is not zero.
-         ;; NB NoteHeads placed left and right of Stem overlap a little, thus
-         ;; the suspended NoteHead widens the NoteColumn by the difference
-         ;; of the whole NoteColumn-width and main NoteColumn-width.
-         ;; Depending on the stem-direction this happens to the left or right.
-
-         (left-note-column-main-X-ext
-          (ly:grob-property left-note-column 'main-extent '(0 . 0)))
+         (dir
+          (if stem
+              (ly:grob-property stem 'direction)
+              0))
      ;;;;
      ;;;; adjust for DotColumn of left NoteColumn
      ;;;;
-         ;; If DotColumn is present and `start-at-dot' is enabled, we
-         ;; calculate the values to move line-start to the right of DotColumn.
-         (dot-column (ly:note-column-dot-column left-note-column))
+         ;; If DotColumn is present and `start-at-dot' is enabled, we want a
+         ;; little extra padding, taken from details.extra-dot-padding.
+         (dot-column (ly:note-column-dot-column left-column))
          (adjust-for-dot-column
           (if (and start-at-dot? (ly:grob? dot-column))
-              (let* ((lnc-dc-common-ref
-                      (ly:grob-common-refpoint
-                       left-note-column dot-column X))
-                     (dot-column-X-ext
-                      (ly:grob-extent dot-column lnc-dc-common-ref X))
-                     (left-note-column-all-X-extent
-                      (ly:grob-extent left-note-column left-note-column X))
-                     (lb-lnc-X-ext
-                      (ly:grob-extent left-bound left-note-column X))
-                     (left-bound-is-suspended-head?
-                      (not (zero? (car lb-lnc-X-ext))))
-                     (added-X-length-for-suspended-head
-                      (- (interval-length left-note-column-all-X-extent)
-                         (interval-length left-note-column-main-X-ext))))
-                (+
-                 (cond ((and (not left-bound-is-suspended-head?)
-                             (positive? dir))
-                        added-X-length-for-suspended-head)
-                       ((and left-bound-is-suspended-head?
-                             (negative? dir))
-                        added-X-length-for-suspended-head)
-                       (else 0))
-                 ;; Add a little extra padding.
-                 ;; Mmmh, hardcoded, make it a bound-details subproperty?
-                 0.4
-                 (interval-length dot-column-X-ext)))
+              (assoc-get 'extra-dot-padding (ly:grob-property grob 'details))
               0))
-         (left-bound-X-ext
-          ;; For a broken DurationLine take line-starting grobs into account.
-          ;; Otherwise use left-bound directly
-          (cond ((not-first-broken-spanner? grob)
-                 (ly:grob-robust-relative-extent left-bound left-bound X))
-                (else left-note-column-main-X-ext)))
          ;; `left-X' is line-starting X-coordinate relative to grob's system
          ;; NB the final line-stencil will start at left-bound not at `left-X'
          ;;    we need this value to calculate `right-end' lateron
          (left-X
           (ly:grob-relative-coordinate
            left-bound (ly:grob-system left-bound) X))
+         (left-info-X (assoc-get 'X left-bound-details))
          ;; `left-Y' is line-starting Y-coordinate, taken from staff-postion
          ;; of grob's first initiating NoteHead.
          (left-bound-original (ly:spanner-bound (ly:grob-original grob) LEFT))
@@ -2722,47 +2674,23 @@ The final stencil is adjusted vertically using @var{staff-space}, which is
     ;;;;
     ;;;; final calculation of `left-start'
     ;;;;
+
          ;; `left-start' is line-starting X-coordinate relative to left-bound
+         ;;
+         ;; If DurationLine is started at "skip", `left-bound' may be
+         ;; PaperColumn, with none-zero x-extent. The x-extent should be
+         ;; disregarded in this case, setting it to zero.
          (left-start
-          (+ (cdr left-bound-X-ext)
-             adjust-for-dot-column
-             left-padding))
+           (+ (if (and (unbroken-or-first-broken-spanner? grob)
+                  (grob::has-interface left-bound 'paper-column-interface))
+                  0
+                  (- left-info-X left-X))
+              adjust-for-dot-column
+              left-padding))
     ;;;;;;;;;;;;;;;;;;;;;;;;
     ;;;; DurationLine end
     ;;;;;;;;;;;;;;;;;;;;;;;;
-         ;; NB `right-bound' may not always be a NoteColumn
          (right-bound (ly:spanner-bound grob RIGHT))
-         ;; FIXME: misnamed because it's sometimes a PaperColumn
-         ;; or BarLine.  Investigate. --JeanAS
-         (right-note-column
-          (if (note-head-or-rest? right-bound)
-              (ly:grob-parent right-bound X)
-              right-bound))
-         ;; If right-bound is a NoteColumn with suspended NoteHeads or
-         ;; broken items at line-break occurring, we need to find an accurate
-         ;; value to adjust the end of DurationLine.
-         ;; Applied later while calculating `right-end'
-         (right-bound-X-ext (ly:grob-extent right-note-column right-bound X))
-         (right-bound-main-X-ext (ly:grob-property right-note-column 'main-extent '(0 . 0)))
-         (adjust-right-for-suspended-heads-and-broken-items
-          ;; Could be simple (if ...)
-          (cond ;; compensate suspended heads if present
-           ((and (unbroken-or-last-broken-spanner? grob)
-                 (grob::has-interface right-bound 'note-column-interface))
-            (if (negative? dir)
-                (- (interval-length right-bound-X-ext)
-                   (interval-length right-bound-main-X-ext))
-                0))
-           ;; respect items at line-break
-           (else
-            (- (car (ly:grob-robust-relative-extent right-bound right-bound X))))))
-         ;; `right-X' is line-ending X-coordinate relative to grob's system
-         ;; NB the final line-stencil will end at `right-end' not at `right-X'
-         ;;    we need this value to calculate `right-end' lateron
-         (right-X
-          (ly:grob-relative-coordinate
-           right-bound
-           (ly:grob-system right-bound) X))
     ;;;;
     ;;;; adjust or Arpeggio of right NoteColumn
     ;;;;
@@ -2820,19 +2748,18 @@ The final stencil is adjusted vertically using @var{staff-space}, which is
     ;;;;
     ;;;; final calculation of `right-end'
     ;;;;
-         ;; The simple difference: (- right-X left-X) will end the line
-         ;; exactly at right-bound.
-         ;; Needs to be adjusted to repect padding and other possible items.
+         (right-info-X
+          (assoc-get 'X right-bound-details 0))
+         ;; Repect padding and other possible items.
          (right-end
-          (-
-           right-X
-           left-X
-           right-padding
-           adjust-right-for-suspended-heads-and-broken-items
-           adjust-for-arrow
-           adjust-for-accidentals
-           adjust-for-arpeggio))
-         ;; TODO find a method to accept user-generated line-ending stencils
+          (- right-info-X
+             left-X
+             right-padding
+             adjust-for-arrow
+             adjust-for-accidentals
+             adjust-for-arpeggio))
+
+    ;; TODO find a method to accept user-generated line-ending stencils
 
     ;;;;
     ;;;; arrow
@@ -2853,7 +2780,7 @@ The final stencil is adjusted vertically using @var{staff-space}, which is
                right-end left-Y staff-space used-thick blot-diameter grob)
               empty-stencil)))
 
-    (if (>  left-start right-end)
+    (if (> left-start right-end)
         (ly:warning (G_ "Please consider to increase 'minimum-length")))
 
     ;;;;;;;;;;;;;;;;;;;;
@@ -2883,7 +2810,6 @@ The final stencil is adjusted vertically using @var{staff-space}, which is
          (used-thick (assoc-get 'thick vals))
          (hook-stil (assoc-get 'hook vals))
          (arrow-stil (assoc-get 'arrow vals)))
-
     (if (eq? style 'beam)
         (ly:stencil-add
          (ly:round-filled-box
-- 
2.25.1

Reply via email to