Am 14.10.2017 um 12:56 schrieb James Lowe:
Hello,
Here is the current patch countdown list. The next countdown will be on
October 17th.
[…]
Push:
[…]
5211 Merge_rests_engraver: fix vertical rest positions - Malte Meyn
https://sourceforge.net/p/testlilyissues/issues/5211
http://codereview.appspot.com/334740043
[…]
Hi,
IIUC (see CG section 3.3.7) now I should rebase my work on staging and
push it or email it to my mentor. Because I have neither a mentor nor
push ability I assume it’s ok to send the patch files to this list
instead? See attachment.
Cheers,
Malte
>From aa040c5b32a49a4b534fc7df17364c9b9be9552a Mon Sep 17 00:00:00 2001
From: Malte Meyn <lilyp...@maltemeyn.de>
Date: Wed, 4 Oct 2017 09:58:34 +0200
Subject: [PATCH 1/2] Issue 5211/1: fix positioning of merged rests
When used with \magnifyStaff the Merge_rests_engraver failed to position
merged rests correctly. Using 'staff-position for Rests and 'direction
for MultiMeasureRests instead of 'Y-offset fixes this.
This also fixes the positioning of brevis MMRs as in
{ \time 4/2 R\breve }
---
scm/scheme-engravers.scm | 78 +++++++++++++++++++-----------------------------
1 file changed, 31 insertions(+), 47 deletions(-)
diff --git a/scm/scheme-engravers.scm b/scm/scheme-engravers.scm
index 3480610c4c..e346985939 100644
--- a/scm/scheme-engravers.scm
+++ b/scm/scheme-engravers.scm
@@ -125,57 +125,41 @@ This works by gathering all rests at a time step. If they
are all of the same
length and there are at least two they are moved to the correct location as
if there were one voice."
- (define (is-single-bar-rest? mmrest)
- (eqv? (ly:grob-property mmrest 'measure-count) 1))
-
- (define (is-whole-rest? rest)
- (eqv? (ly:grob-property rest 'duration-log) 0))
-
- (define (mmrest-offset mmrest)
- "For single measures they should hang from the second line from the top
- (offset of 1). For longer multimeasure rests they should be centered on the
- middle line (offset of 0).
- NOTE: For one-line staves full single measure rests should be positioned at
- 0, but I don't anticipate this engraver's use in that case. No errors are
- given in this case."
- (if (is-single-bar-rest? mmrest) 1 0))
-
- (define (rest-offset rest)
- (if (is-whole-rest? rest) 1 0))
-
- (define (rest-eqv rest-len-prop)
- "Compare rests according the given property"
- (define (rest-len rest) (ly:grob-property rest rest-len-prop))
- (lambda (rest-a rest-b)
- (eqv? (rest-len rest-a) (rest-len rest-b))))
-
- (define (rests-all-unpitched rests)
+ (define (measure-count-eqv? a b)
+ (eqv?
+ (ly:grob-property a 'measure-count)
+ (ly:grob-property b 'measure-count)))
+
+ (define (rests-all-unpitched? rests)
"Returns true when all rests do not override the staff-position grob
property. When a rest has a position set we do not want to merge rests at
that position."
(every (lambda (rest) (null? (ly:grob-property rest 'staff-position)))
rests))
- (define (merge-mmrests rests)
- "Move all multimeasure rests to the single voice location."
- (if (all-equal rests (rest-eqv 'measure-count))
- (merge-rests rests mmrest-offset)))
-
- (define (merge-rests rests offset-function)
- (let ((y-offset (offset-function (car rests))))
- (for-each
- (lambda (rest) (ly:grob-set-property! rest 'Y-offset y-offset))
- rests))
+ (define (merge-mmrests mmrests)
+ "Move all multimeasure rests to the single voice location."
+ (if (all-equal? mmrests measure-count-eqv?)
+ (begin
+ (for-each
+ (lambda (rest) (ly:grob-set-property! rest 'direction CENTER))
+ mmrests)
+ (for-each
+ (lambda (rest) (ly:grob-set-property! rest 'transparent #t))
+ (cdr mmrests)))))
+
+ (define (merge-rests rests)
+ (for-each
+ (lambda (rest) (ly:grob-set-property! rest 'staff-position 0))
+ rests)
(for-each
(lambda (rest) (ly:grob-set-property! rest 'transparent #t))
(cdr rests)))
- (define has-one-or-less (lambda (lst) (or (null? lst) (null? (cdr lst)))))
- (define has-at-least-two (lambda (lst) (not (has-one-or-less lst))))
- (define (all-equal lst pred)
- (or (has-one-or-less lst)
- (and (pred (car lst) (cadr lst)) (all-equal (cdr lst) pred))))
- (define moment=?
- (lambda (a b) (not (or (ly:moment<? a b) (ly:moment<? b a)))))
+ (define (has-one-or-less? lst) (or (null? lst) (null? (cdr lst))))
+ (define (has-at-least-two? lst) (not (has-one-or-less? lst)))
+ (define (all-equal? lst pred)
+ (or (has-one-or-less? lst)
+ (and (pred (car lst) (cadr lst)) (all-equal? (cdr lst) pred))))
(let ((curr-mmrests '())
(mmrests '())
@@ -213,14 +197,14 @@ if there were one voice."
'duration)))
rests)))
(if (and
- (has-at-least-two rests)
- (all-equal durs moment=?)
- (rests-all-unpitched rests))
+ (has-at-least-two? rests)
+ (all-equal? durs equal?)
+ (rests-all-unpitched? rests))
(begin
- (merge-rests rests rest-offset)
+ (merge-rests rests)
;; ly:grob-suicide! works nicely for dots, as opposed to rests.
(if (pair? dots) (for-each ly:grob-suicide! (cdr dots)))))
- (if (has-at-least-two curr-mmrests)
+ (if (has-at-least-two? curr-mmrests)
(set! mmrests (cons curr-mmrests mmrests)))))
((finalize translator)
(for-each merge-mmrests mmrests)))))
--
2.14.1
>From 660db241f08c3e4a90b11783ee0eeb8bf74ee55d Mon Sep 17 00:00:00 2001
From: Malte Meyn <lilyp...@maltemeyn.de>
Date: Fri, 6 Oct 2017 22:45:38 +0200
Subject: [PATCH 2/2] Issue 5211/2: add regtest
Add a regtest for Merge_rests_engraver with \magnifyStaff.
Also add a brevis MMR to the original Merge_rests_engraver regtest.
---
input/regression/merge-rests-engraver.ly | 9 ++++++++-
input/regression/merge-rests-magnify-staff.ly | 28 +++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 1 deletion(-)
create mode 100644 input/regression/merge-rests-magnify-staff.ly
diff --git a/input/regression/merge-rests-engraver.ly
b/input/regression/merge-rests-engraver.ly
index c03b33ce54..f24a88c136 100644
--- a/input/regression/merge-rests-engraver.ly
+++ b/input/regression/merge-rests-engraver.ly
@@ -24,7 +24,13 @@ voiceA = \relative {
% compressed multi-measure rests are combined
R1*3 |
+ % multi-measure rests longer than semi-breve
+ % are merged at the correct vertical position
+ \time 4/2
+ R\breve
+
% combining between beams, slurs
+ \time 4/4
c8[( r c]) r c16[( r c] r c[ r c]) r |
% combining in tuplets
@@ -62,6 +68,7 @@ voiceB = \relative {
r2_"Down" r4 r8 r16 r32 r64 r128 r |
R1_"Lower text" |
R1*3 |
+ R\breve |
c8[( r c]) r c16[( r c] r c[ r c]) r |
\tuplet 3/2 { c8 r r } r4 \tuplet 3/2 { c4 r r } |
r4-> r-. r r |
@@ -75,7 +82,7 @@ voiceB = \relative {
voiceC = \relative {
s1*2 |
r2 r4 r8 r16 r32 r64 r128 r | % Combines rests from more than 2 voices
- s1*11
+ s1*13
r4. r8 r4. r8 |
r4. r8 r4. r8 |
}
diff --git a/input/regression/merge-rests-magnify-staff.ly
b/input/regression/merge-rests-magnify-staff.ly
new file mode 100644
index 0000000000..1119581dc9
--- /dev/null
+++ b/input/regression/merge-rests-magnify-staff.ly
@@ -0,0 +1,28 @@
+\version "2.21.0"
+
+\header {
+ texidoc = "Test for vertical positions of merged rests in magnified staves."
+}
+
+music = {
+ R1
+ R1*2
+ \compressFullBarRests
+ R1*7
+ R1*11
+ \time 4/2
+ R\breve
+ r\breve
+ r1 r2 r4 r8 r
+}
+
+<<
+ \new Staff \with {
+ \magnifyStaff #3/4
+ \consists "Merge_rests_engraver"
+ } << \music \\ \music >>
+ \new Staff \with {
+ \magnifyStaff #4/3
+ \consists "Merge_rests_engraver"
+ } << \music \\ \music >>
+>>
\ No newline at end of file
--
2.14.1
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
