Issue 5919 Make InstrumentName.X-offset more robust (issue 553930044 by thomasmorley65@gmail.com)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Issue 5919 Make InstrumentName.X-offset more robust (issue 553930044 by thomasmorley65@gmail.com)

Thomas Morley-2
Reviewers: ,

Description:
Issue 5919 Make InstrumentName.X-offset more robust

Currently system-start-text::calc-y-offset takes right end of
system-start-grobs into account, which may lead to bad results, if
one of them is omitted (collapse-height) or moved to the right.
Instead we always use the value of indent for the right end and
calculate the left end similar as before.

Please review this at https://codereview.appspot.com/553930044/

Affected files (+9, -10 lines):
  M scm/output-lib.scm


Index: scm/output-lib.scm
diff --git a/scm/output-lib.scm b/scm/output-lib.scm
index 364b66d936442cb808acd89675f1ed9bc590748c..d11b0a1536cfe62e4cb5bbb154a7c3d1833b0052 100644
--- a/scm/output-lib.scm
+++ b/scm/output-lib.scm
@@ -1409,29 +1409,28 @@ parent or the parent has no setting."
          (my-extent (ly:grob-extent grob system X))
          (elements (ly:grob-object system 'elements))
          (common (ly:grob-common-refpoint-of-array system elements X))
-         (total-ext empty-interval)
+         (total-left +inf.0)
          (align-x (ly:grob-property grob 'self-alignment-X 0))
          (padding (min 0 (- (interval-length my-extent) indent)))
          (right-padding (- padding
                            (/ (* padding (1+ align-x)) 2))))
 
     ;; compensate for the variation in delimiter extents by
-    ;; calculating an X-offset correction based on united extents
+    ;; calculating an X-offset correction based on the extents
     ;; of all delimiters in this system
-    (let unite-delims ((l (ly:grob-array-length elements)))
+    ;; finally we take most-left coordinate and indent
+    (let most-left-delim-x ((l (ly:grob-array-length elements)))
       (if (> l 0)
           (let ((elt (ly:grob-array-ref elements (1- l))))
-
             (if (grob::has-interface elt 'system-start-delimiter-interface)
                 (let ((dims (ly:grob-extent elt common X)))
-                  (if (interval-sane? dims)
-                      (set! total-ext (interval-union total-ext dims)))))
-            (unite-delims (1- l)))))
+                  (set! total-left (min total-left (car dims)))))
+            (most-left-delim-x (1- l)))))
 
     (+
-     (ly:side-position-interface::x-aligned-side grob)
-     right-padding
-     (- (interval-length total-ext)))))
+       (ly:side-position-interface::x-aligned-side grob)
+       right-padding
+       (- (interval-length (cons total-left indent))))))
 
 (define-public (system-start-text::calc-y-offset grob)
 



Reply | Threaded
Open this post in threaded view
|

Re: Issue 5919 Make InstrumentName.X-offset more robust (issue 553930044 by thomasmorley65@gmail.com)

nine.fierce.ballads
On 2020/04/18 18:14:33, thomasmorley651 wrote:

I ran make check and didn't see any differences.  Without test coverage,
someone could easily break this again without knowing it.

https://codereview.appspot.com/553930044/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5919 Make InstrumentName.X-offset more robust (issue 553930044 by thomasmorley65@gmail.com)

Dan Eble
On Apr 18, 2020, at 14:38, [hidden email] wrote:
>
> On 2020/04/18 18:14:33, thomasmorley651 wrote:
>
> I ran make check and didn't see any differences.  Without test coverage,
> someone could easily break this again without knowing it.

Oops.  *I* wrote that; I just neglected to delete the automatic attribution line.

Dan