Re: Uses single algorithm for side-position spacing. (issue 6827072)
On 29 nov. 2012, at 03:23, k-ohara5...@oco.net wrote: http://codereview.appspot.com/6827072/diff/18003/lily/box-quarantine.cc File lily/box-quarantine.cc (right): http://codereview.appspot.com/6827072/diff/18003/lily/box-quarantine.cc#newcode69 lily/box-quarantine.cc:69: int mid = ii0.mid_; assert ((double)(ii0.index - mid) = 0.0) assert ((double)(ii1.index - mid) = 0.0) This will fail often...it is possible, for example, that ii0.index is 0 and mid is 3 (mid represents the middle index of the vector). http://codereview.appspot.com/6827072/diff/18003/lily/box-quarantine.cc#newcode93 lily/box-quarantine.cc:93: Offset shift (-(epsilon + (ii[0].amount_ + padding_) / 2.0), 0.0); The padding appears in some places but not others; see 'fingering-column.ly' Fixed. http://codereview.appspot.com/6827072/diff/18003/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/6827072/diff/18003/lily/skyline.cc#newcode234 lily/skyline.cc:234: // Flatten to height h What is the difference between this and Skyline:::set_minimum_height() at line 817 ? Set minimum height allows for things over the minimum height to retain their skyline-ness, whereas this creates a flat skyline at height X. http://codereview.appspot.com/6827072/diff/18003/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/6827072/diff/18003/scm/define-grob-properties.scm#newcode492 scm/define-grob-properties.scm:492: (horizon-padding ,number? The amount to pad the axis We already have 'skyline-horizontal-padding' on line 815 and 'skyline-vertical-padding', so it would be better to use those. You could look up one or the other depending on which axis, X or Y, in which the buildings grow. http://codereview.appspot.com/6827072/diff/18003/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/6827072/diff/18003/scm/define-grobs.scm#newcode253 scm/define-grobs.scm:253: (horizon-padding . 0.05) This could be 'skyline-horizontal-padding' as in System and VerticalAxisGroup. I definitely agree, but this'd be for another patch set. The eventual goal of all of this is to get all spacing out of axis-group-interface and into side position interface via one general algorithm. When that happens, I'll make these merges. For now, it is a bit difficult, as there are two concurrent systems - one that specifies vertical versus horizontal padding and one that specifies padding versus horizon-padding. The latter system (side-position) probably needs to be changed, but it'd require a major convert-ly rule. http://codereview.appspot.com/6827072/diff/18003/scm/define-grobs.scm#newcode886 scm/define-grobs.scm:886: (add-stem-support . #t) Several regression tests assume this is false by default, and then toggle it to true, so as to test both cases. add-stem-support = #f doesn't seem to work very well with beams with this patch. One can write a lambda function that returns #t if there is a beam present and #f otherwise. Otherwise, the beam would have to be added at the engraver stage. I prefer the former. http://codereview.appspot.com/6827072/diff/18003/scm/define-grobs.scm#newcode909 scm/define-grobs.scm:909: (FingeringCollision This would need a convert-ly rule. Why change the name ? You recommended that in a previous review. I can push the convert-ly rule as a separate patch. http://codereview.appspot.com/6827072/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uses single algorithm for side-position spacing. (issue 6827072)
On 2012/11/29 08:28:31, mike7 wrote: On 29 nov. 2012, at 03:23, mailto:k-ohara5...@oco.net wrote: http://codereview.appspot.com/6827072/diff/18003/lily/box-quarantine.cc#newcode69 lily/box-quarantine.cc:69: int mid = ii0.mid_; assert ((double)(ii0.index - mid) = 0.0) it is possible, for example, that ii0.index is 0 and mid is 3 (mid represents the middle index of the vector). ii0.index_ = 0; mid = 3; printf(%f\n,(double)(ii0.index_ - mid)); 4294967293.00 add-stem-support = #f doesn't seem to work very well with beams with this patch. One can write a lambda function ... But I want to set Wilder Reiter (my favorite piece when I was eight, and the example we discussed where fingerings go alongside stems) http://imslp.org/wiki/File:PMLP02707-Schumann_-_Album_für_die_Jugend.pdf and I don't know how to write lambda functions. http://codereview.appspot.com/6827072/diff/18003/scm/define-grobs.scm#newcode909 scm/define-grobs.scm:909: (FingeringCollision This would need a convert-ly rule. Why change the name ? You recommended that in a previous review. Well I must have been suffering from a case of the stupids, because FingeringColumn was a perfect name for a column of fingering indications. Now I'm suffering a case of amnesia trying to think of what I could have meant. http://codereview.appspot.com/6827072/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uses single algorithm for side-position spacing. (issue 6827072)
It was Box_quarantine that seemed a confusing name. (Why 'quarantine'? Is there something special about 40 boxes?) I take back my suggestion. http://codereview.appspot.com/6827072/diff/7041/lily/box-quarantine.cc File lily/box-quarantine.cc (right): http://codereview.appspot.com/6827072/diff/7041/lily/box-quarantine.cc#newcode63 lily/box-quarantine.cc:63: TODO: not sure if this loop causes infinite behavior... The while(dirty) loop runs 2366 times for the last chord in 'fingering-collision.ly' but that's an extreme case. http://codereview.appspot.com/6827072/diff/7041/lily/fingering-column.cc File lily/fingering-column.cc (left): http://codereview.appspot.com/6827072/diff/7041/lily/fingering-column.cc#oldcode63 lily/fingering-column.cc:63: fingerings[i]-translate_axis (-fingerings[i]-extent (common[Y_AXIS], Y_AXIS).length () / 2, Y_AXIS); This shifted all the fingerings down to align them horizontally to their note-heads, and is missing from the new code http://codereview.appspot.com/6827072/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uses single algorithm for side-position spacing. (issue 6827072)
On 29 nov. 2012, at 10:24, k-ohara5...@oco.net wrote: It was Box_quarantine that seemed a confusing name. (Why 'quarantine'? Is there something special about 40 boxes?) I take back my suggestion. http://codereview.appspot.com/6827072/diff/7041/lily/box-quarantine.cc File lily/box-quarantine.cc (right): http://codereview.appspot.com/6827072/diff/7041/lily/box-quarantine.cc#newcode63 lily/box-quarantine.cc:63: TODO: not sure if this loop causes infinite behavior... The while(dirty) loop runs 2366 times for the last chord in 'fingering-collision.ly' but that's an extreme case. I'm not proud of this... http://codereview.appspot.com/6827072/diff/7041/lily/fingering-column.cc File lily/fingering-column.cc (left): http://codereview.appspot.com/6827072/diff/7041/lily/fingering-column.cc#oldcode63 lily/fingering-column.cc:63: fingerings[i]-translate_axis (-fingerings[i]-extent (common[Y_AXIS], Y_AXIS).length () / 2, Y_AXIS); This shifted all the fingerings down to align them horizontally to their note-heads, and is missing from the new code Fixed. http://codereview.appspot.com/6827072/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uses single algorithm for side-position spacing. (issue 6827072)
On 29 nov. 2012, at 10:13, k-ohara5...@oco.net wrote: On 2012/11/29 08:28:31, mike7 wrote: On 29 nov. 2012, at 03:23, mailto:k-ohara5...@oco.net wrote: http://codereview.appspot.com/6827072/diff/18003/lily/box-quarantine.cc#newcode69 lily/box-quarantine.cc:69: int mid = ii0.mid_; assert ((double)(ii0.index - mid) = 0.0) it is possible, for example, that ii0.index is 0 and mid is 3 (mid represents the middle index of the vector). ii0.index_ = 0; mid = 3; printf(%f\n,(double)(ii0.index_ - mid)); 4294967293.00 woah...totally over my head, but ok, convinced add-stem-support = #f doesn't seem to work very well with beams with this patch. One can write a lambda function ... But I want to set Wilder Reiter (my favorite piece when I was eight, and the example we discussed where fingerings go alongside stems) http://imslp.org/wiki/File:PMLP02707-Schumann_-_Album_für_die_Jugend.pdf and I don't know how to write lambda functions. I wrote a lambda function. Regest added. http://codereview.appspot.com/6827072/diff/18003/scm/define-grobs.scm#newcode909 scm/define-grobs.scm:909: (FingeringCollision This would need a convert-ly rule. Why change the name ? You recommended that in a previous review. Well I must have been suffering from a case of the stupids, because FingeringColumn was a perfect name for a column of fingering indications. Now I'm suffering a case of amnesia trying to think of what I could have meant. from days of yore... http://codereview.appspot.com/6827072/diff/11002/lily/box-quarantine.cc#newcode24 lily/box-quarantine.cc:24: Box_quarantine::Box_quarantine (Real padding, Axis a) So far it looks like this handles fingering only. Other similar problems are handled with a XX_Collision object, so maybe just call this Fingering_Collision ?___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: markup-commands rest-by-number and rest (issue 6850073)
On 2012/11/29 00:41:14, thomasmorley65 wrote: On 2012/11/27 20:03:06, benko.pal wrote: Did you had a look on the compiled output of the new reg-tests? I did now, it's OK with me. https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest-styles.ly File input/regression/markup-rest-styles.ly (right): https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest-styles.ly#newcode25 input/regression/markup-rest-styles.ly:25: '(-3 -2 -1 0 1 2 3 4 5 6 7 please fix indentation, this and the next list are at quite different levels. similarly in the other .ly file. https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest.ly File input/regression/markup-rest.ly (right): https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest.ly#newcode27 input/regression/markup-rest.ly:27: (if (= duration 0) dots ) this if looks superfluous https://codereview.appspot.com/6850073/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
still absent
Dear friends, i hope that you're doing well. i have to remain off-list at least until Christmas :( all the best, Janek PS the conductor of my choir is in critical state after heart surgery complications. please, pray for him if you can. ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
How to do proper indentation?
Hi, reviewing https://codereview.appspot.com/6850073/ Pál admonished to do better indentation on lists at different levels. Trying to minimize my lack of knowledge on this topic I searched the CG about It. CG 10.5.3 Indentation mentions possibilities for emacs and vim, but I'm not familiar with them. I'm using jEdit or gedit Following the link in CG 10.3.2 Desired file formatting http://community.schemewiki.org/?scheme-style doesn't help here. My own guess would be (file is attached, too): showSimpleRest = #(define-scheme-function (parser location dots) (string?) (make-override-markup (cons 'baseline-skip 7) (make-column-markup (map (lambda (style) (make-line-markup (list (make-pad-to-box-markup '(0 . 20) '(0 . 0) (symbol-string style)) (make-override-markup (cons 'line-width 60) (make-override-markup (cons 'style style) (make-fill-line-markup (map (lambda (duration) (make-rest-markup (if (string? duration) duration (string-append (number-string (expt 2 duration)) (if (= duration 0) dots ) (append '(maxima longa breve)(iota 8) '(default mensural neomensural classical baroque altdefault petrucci blackpetrucci semipetrucci kievan) Is this ok? If not, how to do? Or where to look to learn it? -Harm indentation.ly Description: Binary data ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: markup-commands rest-by-number and rest (issue 6850073)
On 2012/11/29 20:42:21, benko.pal wrote: On 2012/11/29 00:41:14, thomasmorley65 wrote: On 2012/11/27 20:03:06, benko.pal wrote: Did you had a look on the compiled output of the new reg-tests? I did now, Thanks for doing this. it's OK with me. https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest-styles.ly File input/regression/markup-rest-styles.ly (right): https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest-styles.ly#newcode25 input/regression/markup-rest-styles.ly:25: '(-3 -2 -1 0 1 2 3 4 5 6 7 please fix indentation, this and the next list are at quite different levels. similarly in the other .ly file. Couldn't find a guideline to do correct indentation here. I ask on -devel https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest.ly File input/regression/markup-rest.ly (right): https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest.ly#newcode27 input/regression/markup-rest.ly:27: (if (= duration 0) dots ) this if looks superfluous Yep. Will delete it. https://codereview.appspot.com/6850073/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
PATCH: Countdown to 20121203
For 22:00 MST Monday December 3rd Crash: Issue 2972 http://code.google.com/p/lilypond/issues/detail?id=2972: Adding StringNumber 0 in TabStaff crashes - R 6858077 https://codereview.appspot.com/6858077/ Defect: Issue 732 http://code.google.com/p/lilypond/issues/detail?id=732: Alignment problems when vertically stacking horizontally centered stencils - R 6855077 https://codereview.appspot.com/6855077/ Documentation: Issue 2916 http://code.google.com/p/lilypond/issues/detail?id=2916: Doc : NR Document \hide command - R 6851102 https://codereview.appspot.com/6851102/ Enhancement: Issue 2984 http://code.google.com/p/lilypond/issues/detail?id=2984: Patch: Use define-void-function rather than define-music-function in several places - R 6854104 https://codereview.appspot.com/6854104/ Issue 2982 http://code.google.com/p/lilypond/issues/detail?id=2982: Patch: Remove selective contextmods. - R 6846107 https://codereview.appspot.com/6846107/ Ugly: Issue 2527 http://code.google.com/p/lilypond/issues/detail?id=2527: Finger-flag collision - R 6827072 https://codereview.appspot.com/6827072/ Cheers, Colin -- Dwy ddim yn cwyno; mae neb yn gwrando! ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uses single algorithm for side-position spacing. (issue 6827072)
'finger-chords.ly' is still in disagreement with its texidoc (therefore failing). You could adjust that regtest in light of the new defaults, of course. Better might be to make Fingering.add-stem-support = #only-if-beamed the new default. That is in better agreement with common practice. Regtests come out just as good. (Les-neréides.ly can remove a couple tweaks, which seems to be how somebody was keeping score.) Then users will less often want to override add-stem-support, and when they do it will be to the simpler ##f or ##t The while(dirty) loop runs 2366 times for the last chord in 'fingering-collision.ly' but that's an extreme case. I'm not proud of this... It is at least comprehensible; while the code it replaced was utterly baffling. I simplified the loop http://codereview.appspot.com/6854121/ You should at least put the filenames back to what they were, and adjust any tests or documentation or snippets using add-stem support, before pushing. I started to test with real music. The usual Chopin test case http://www.mutopiaproject.org/cgibin/piece-info.cgi?id=1776 has a badly broken cross-staff beam in measure 27. I don't yet see the cause. https://codereview.appspot.com/6827072/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
