Re: Glyphs for Kievan Notation (issue 4951062)
http://codereview.appspot.com/4951062/diff/99002/lily/stem.cc File lily/stem.cc (right): http://codereview.appspot.com/4951062/diff/99002/lily/stem.cc#newcode284 lily/stem.cc:284: string style = robust_symbol2string (heads[0]-get_property (style), default); I think Aleksandr is right. See http://lilypond.org/doc/v2.15/Documentation/contributor/comparison http://codereview.appspot.com/4951062/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes width of NR A.11 - List of special character (issue 5504091)
LGTM. http://codereview.appspot.com/5504091/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: lily-guile updates and CG: Scheme-C interface section. (issue 4917044)
Hi David, Could you make your own patch for the doc changes? And, as you mentionned, the unused function should be removed. Do you want me to commit this change? Bertrand http://codereview.appspot.com/4917044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: lily-guile updates and CG: Scheme-C interface section. (issue 4917044)
lily-guile updates pushed as 6ee8c04678442855cb794d4598c056c15c42673b. http://codereview.appspot.com/4917044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Rename \markuplines to \markuplist (before running convert-ly) (issue 5312050)
LGTM. You'll be happy to know that Mike and I are currently trying to get rid of \markuplines, so that there will only be a \markup command. http://codereview.appspot.com/5312050/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Adds Scheme function for spring constructor. (issue 5306050)
LGTM. http://codereview.appspot.com/5306050/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds Scheme function for spring constructor. (issue 5306050)
http://codereview.appspot.com/5306050/diff/2001/lily/spring-smob.cc File lily/spring-smob.cc (left): http://codereview.appspot.com/5306050/diff/2001/lily/spring-smob.cc#oldcode42 lily/spring-smob.cc:42: return a == b ? SCM_BOOL_T : SCM_BOOL_F; On 2011/10/21 11:27:35, dak wrote: scm_is_true (scm_is_eqv (a, b)) You mean scm_is_true(scm_eqv_p (a, b))? :) For me, this function is totally useless. If we want to check whether they are equal, we use scm_equal_p, if we want to see whether they are the same object, we use scm_eqv_p. Besides, I can't find any use for this function with git grep. http://codereview.appspot.com/5306050/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Sketch for in-notes. (issue 5293053)
http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc File lily/page-layout-problem.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode79 lily/page-layout-problem.cc:79: footnotes_added = g-get_property (footnote-stencil) != SCM_EOL; Of course not :o$ I recommend to_boolean. It returns true for everything except SCM_EOL and SCM_BOOL_F. http://codereview.appspot.com/5293053/diff/12001/lily/system.cc File lily/system.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/system.cc#newcode298 lily/system.cc:298: System::internal_get_note_heights_in_range (vsize start, vsize end, bool foot) I wanted to express my disgust. Not the french Ugh that could refer to your amerindian origins :) http://codereview.appspot.com/5293053/diff/12001/lily/system.cc#newcode305 lily/system.cc:305: if (foot ? !to_boolean (footnote_grobs[i]-get_property (footnote)) : to_boolean (footnote_grobs[i]-get_property (footnote))) Ok. I thought footnote_grobs was only using footnotes. Maybe you should change the names of footnote_grobs and get_footnote_grobs_in_range? And change this long line for: if (!to_boolean (footnote_grobs[i]-get_property (footnote))) http://codereview.appspot.com/5293053/diff/12001/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/5293053/diff/12001/scm/define-grob-properties.scm#newcode299 scm/define-grob-properties.scm:299: (footnote ,boolean? Should this be a footnote or in-note?) Something like that. I think style implicitly stands for musical notation style, so we need to find something better. http://codereview.appspot.com/5293053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Sketch for in-notes. (issue 5293053)
A small comment to show you a possible solution for the indentation. This can be apply at every line length comment. http://codereview.appspot.com/5293053/diff/12001/lily/page-breaking.cc File lily/page-breaking.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/page-breaking.cc#newcode189 lily/page-breaking.cc:189: old.in_note_heights_.begin (), old.in_note_heights_.end ()); I'm not sure, but maybe: compressed.footnote_heights_.insert (compressed.footnote_heights_.begin (), old.footnote_heights_.begin (), old.footnote_heights_.end ()); http://codereview.appspot.com/5293053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Sketch for in-notes. (issue 5293053)
Sorry for this total mistake. !ly_is_equal (..., SCM_EOL) should do it, then. http://codereview.appspot.com/5293053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: First stab at getting script offsets right. (issue 5235052)
LGTM, but the result is really disturbing. We need to think about a better approach of character boxes in MetaFont. The ideal solution would be to create a mask for each character. For example, a mask for the espressivo glyph would be a fill between its 6 dots. I know it's impossible to interpret this mask in C++... Anyway, I think this patch can be pushed as the first part of a fix to issue 1951. Bertrand http://codereview.appspot.com/5235052/diff/23001/lily/script-engraver.cc File lily/script-engraver.cc (right): http://codereview.appspot.com/5235052/diff/23001/lily/script-engraver.cc#newcode208 lily/script-engraver.cc:208: int script_count = scripts_.size (); vsize instead of int. Could you change the others above? http://codereview.appspot.com/5235052/diff/23001/lily/script-engraver.cc#newcode209 lily/script-engraver.cc:209: for (int i = 0; i script_count; i++) vsize. http://codereview.appspot.com/5235052/diff/23001/lily/slur.cc File lily/slur.cc (right): http://codereview.appspot.com/5235052/diff/23001/lily/slur.cc#newcode275 lily/slur.cc:275: // cannot use is empty because some 0-extent scripts I don't understand. Did you meant if empty? http://codereview.appspot.com/5235052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Optimizes note-heads.cc and introduces robust_symbol2string. (issue 5233042)
Pushed as 8adeb99e344bf047b9b3b9b48a9e97e59e8fc4d3 and 71aa438bce0f68b0e8ab8c633b4902c971ede48b. http://codereview.appspot.com/5233042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: lily-guile updates and CG: Scheme-C interface section. (issue 4917044)
I pushed the doc as b4a2cb2cf00347c477ed595f1435cc212e70ce33. Could the remaining C part of the patch be 'countdowned'? http://codereview.appspot.com/4917044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Glyphs for Kievan Notation (issue 4951062)
LGTM. Two tiny changes and it'll be ready to push. http://codereview.appspot.com/4951062/diff/78001/ly/engraver-init.ly File ly/engraver-init.ly (right): http://codereview.appspot.com/4951062/diff/78001/ly/engraver-init.ly#newcode1107 ly/engraver-init.ly:1107: \remove Stem_engraver Sorry, I made a mistake. This line is producing errors when we use the kievanvoice/staff. You have to remove this and add a \override Stem #'stencil = ##f instead. http://codereview.appspot.com/4951062/diff/78001/ly/engraver-init.ly#newcode1114 ly/engraver-init.ly:1114: \override Slur #'transparent = ##t Could you change 'transparent' into 'stencil'? With transparent, the stem is invisible, but still there. The calculus of grob positions can be disturbed by that. http://codereview.appspot.com/4951062/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: CG: clarify git-cl and contributor quick start (issue 5240054)
LGTM http://codereview.appspot.com/5240054/diff/1/Documentation/contributor/source-code.itexi File Documentation/contributor/source-code.itexi (right): http://codereview.appspot.com/5240054/diff/1/Documentation/contributor/source-code.itexi#newcode953 Documentation/contributor/source-code.itexi:953: Note that a google account need not be a gmail account; you can doesn't need to be a gmail account? http://codereview.appspot.com/5240054/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Optimizes note-heads.cc and introduces robust_symbol2string. (issue 5233042)
Thanks Neil, I'll make two commits. Do you want to see them both on codereview? Comment in lily-guile restored. http://codereview.appspot.com/5233042/diff/12001/lily/lily-guile.cc File lily/lily-guile.cc (left): http://codereview.appspot.com/5233042/diff/12001/lily/lily-guile.cc#oldcode72 lily/lily-guile.cc:72: Ugh. this is not very efficient. This was a mistake, sorry. http://codereview.appspot.com/5233042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Optimizes note-heads.cc and introduces robust_symbol2string. (issue 5233042)
Reviewers: , Message: Hi, This is a try to increase performance, especially under Windows (see http://code.google.com/p/lilypond/issues/detail?id=1926). As I don't use Windows to compile LilyPond, can someone test it? Regards, Bertrand Description: Optimizes note-heads.cc and introduces robust_symbol2string. Potential fix to issue 1926. Please review this at http://codereview.appspot.com/5233042/ Affected files: M lily/include/lily-guile.hh M lily/lily-guile.cc M lily/lily-parser-scheme.cc M lily/note-head.cc M lily/page-turn-engraver.cc M lily/paper-column-engraver.cc M lily/program-option-scheme.cc M lily/rest.cc M lily/stem.cc M lily/time-signature.cc ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves some parmesan noteheads. (issue 4639065)
http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc File lily/stem.cc (right): http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc#newcode855 lily/stem.cc:855: if (attach !scm_is_eq (style, ly_symbol2scm (mensural)) I meant: the stem is aligned to the right of the attachment point (and to its left for down-stems). For centered stems like we have in ancient styles, the stems have to be centered around the attachment point, which is positioned at the center of the notehead. http://codereview.appspot.com/4639065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Glyphs for Kievan Notation (issue 4951062)
Here's the second part of my review. I saw that kievan notation has beams, contrary to what I thought. If you're intrepid, you can also try to implement the kievan beaming parameters in you KievanVoice. Hold on, it's the end! Bertrand http://codereview.appspot.com/4951062/diff/64002/mf/parmesan-clefs.mf File mf/parmesan-clefs.mf (right): http://codereview.appspot.com/4951062/diff/64002/mf/parmesan-clefs.mf#newcode1738 mf/parmesan-clefs.mf:1738: % TODO: merge this code with the above Obviously, this must be done. This is easy, you just have to define the glyph and create two characters with it: def draw_kievan_do_clef = z1 = [...] [...] enddef; fet_beginchar ([...]); draw_kievan_do_clef; fet_endchar; fet beginchar ([...]_change); % TODO: make a different glyph for changes, but % dunno what a kievan clef looks like in changes... draw[...]; endchar; http://codereview.appspot.com/4951062/diff/64002/mf/parmesan-noteheads.mf File mf/parmesan-noteheads.mf (right): http://codereview.appspot.com/4951062/diff/64002/mf/parmesan-noteheads.mf#newcode1861 mf/parmesan-noteheads.mf:1861: fet_beginchar (kievan half note (space position), s1rkievan); Still sr1kievan. http://codereview.appspot.com/4951062/diff/64002/scm/output-lib.scm File scm/output-lib.scm (right): http://codereview.appspot.com/4951062/diff/64002/scm/output-lib.scm#newcode101 scm/output-lib.scm:101: (min 2 Maybe you could try to change min 2 for min 3 and remove what you added before. I'm totally unsure of that, we must check what's going on downstream to be sure. http://codereview.appspot.com/4951062/diff/64002/scm/output-lib.scm#newcode595 scm/output-lib.scm:595: (0 . accidentals.vaticana0) I guess there's no natural glyph in kievan notation since there is no time signature and no accidental 'remembering'. Can you confirm this? Maybe we need to remove this line, then. This will produce a warning (but no crash) if one tries to use a natural in kievan style. http://codereview.appspot.com/4951062/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Optimizes note-heads.cc and introduces robust_symbol2string. (issue 5233042)
http://codereview.appspot.com/5233042/diff/1/lily/note-head.cc File lily/note-head.cc (right): http://codereview.appspot.com/5233042/diff/1/lily/note-head.cc#newcode79 lily/note-head.cc:79: out = fm-find_by_name (idx_either); Unfortunately, this doesn't work. If the condition line 73 is false, then out will be an empty stencil. But this gave me an idea: use two stencils instead of one. Instead of overwriting the working stencil 'out' to check if the new one is empty, we can use another 'test' stencil. I test that now and upload it. http://codereview.appspot.com/5233042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Glyphs for Kievan Notation (issue 4951062)
You can't solve the Kievan bar line problem without calculate every dot
position. I made a pure MetaFont version of the Kievan bar line:
fet_beginchar (kievan end of piece (slash), barline.kievan);
% this draws the end of piece figure
% this figure is placed at the end of the musical piece, after the
staff
save hair_thickness, thick_thickness, width, depth, height, path;
hair# = 1.9 linethickness#;
thick# = 6.0 linethickness#;
width# = .8 staff_space#;
height# + depth# = 4 staff_space#;
depth# = height# + hair# / 2;
set_char_box (0, width#, depth#, height#);
define_pixels (hair, thick);
x1r - x2l = w;
y1 - y3r = d + h + linethickness / 2;
z3 = z2;
z4 = .5 [z1, z2] = (w / 2, hair / 8);
z5 = (x2 - .17 staff_space, 9/10 [y2, y1]);
z7 - z6 = (.5 staff_space, -.2 staff_space);
.4 [z6, z7] = 7/6 [z2, z1];
pickup pencircle scaled blot_diameter;
penpos1 (hair, 0);
penpos2 (hair, 0);
penpos3 (hair, -90);
penpos4 (thick, 10);
penpos5 (thick, 35);
penpos6 (hair, -90);
penpos7 (.5 thick, -120);
penlabels (1, 2, 3, 4, 5, 6, 7);
fill z1r
-- z2r
-- z2l
-- z1l
-- cycle;
fill simple_serif (z3l, z3r, 90){1.5 right}
.. z4r
.. z5r
.. z6r
.. simple_serif (z7r, z7l, 80)
.. {left}z6l
.. z5l
.. z4l
.. cycle;
fet_endchar;
I will make a complete review later.
Bertrand
http://codereview.appspot.com/4951062/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves some parmesan noteheads. (issue 4639065)
Thanks Keith, I'll quickly fix that in a new issue. http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc File lily/stem.cc (right): http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc#newcode853 lily/stem.cc:853: extract_grob_set (me, note-heads, heads); ... OK. http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc#newcode855 lily/stem.cc:855: if (attach !scm_is_eq (style, ly_symbol2scm (mensural)) Of course, but we only choose the position of the attachment. By default, the stem is right-aligned... http://codereview.appspot.com/4639065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New partcombineUp and partcombineDown functions (issue 4514042)
Pushed as bb7cac7b276bd7d1335523f3be8df815cf894ece. http://codereview.appspot.com/4514042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue 4553056)
Pushed as 688f5f1711d8ca07338385a2ae0191b1a8aae315. http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Unifies mensural ligatures with blot-diameter. (issue 5030053)
Pushed as 829f0ded77ee44ea6f0566fb5e5318802a8857ad. http://codereview.appspot.com/5030053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: lily-guile updates and CG: Scheme-C interface section. (issue 4917044)
Update done. I think the C part is ok. There's maybe a few things to change in the doc. Shall I wait for a countdown or push directly? Bertrand http://codereview.appspot.com/4917044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue 4553056)
New patch set. I hope this is ready for to be pushed, now. http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue 4553056)
Thanks a lot, Neil. Could you have a last look at the Scheme files? I'm not sure of the indentation. I created a new scm/text.scm file for the definitions I couldn't put elsewhere. Bertrand http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue 4553056)
Hey! I'm currently writing a doc entry that explains how to use replacements. I have a few questions: Where do you think I should put it? In NR 1.8.1 or 1.8.2? Do you think I have to move the table from the regtest to the Appendix A (and keep the rest of the regtest as a regtest)? Bertrand PS: I'm also writing a changelog entry. http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: T1780 remove scheme format calls with no destination parameter - deprecated in Guile V2 (issue 4974078)
Thanks for applying these! Sorry to bother you again with indentation, but you don't have to replace spaces with tabulators in scripts/musicxml2ly.ly . We decided that the rule for Python is 4 spaces per indentation level. For more infos, see: http://lilypond.org/~graham/gop/gop_1.html#GOP-1-_002d-python-formatting Thanks, Bertrand http://codereview.appspot.com/4974078/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue 4553056)
On 2011/09/20 12:07:31, J_lowe wrote: Where do you think I should put it? In NR 1.8.1 or 1.8.2? Hmm.. I'd say 3.3.3 actually Oh, yes! This is better. Do you think I have to move the table from the regtest to the Appendix A (and keep the rest of the regtest as a regtest)? I am not experienced enough to answer this, but which Appendix A did you intend? A new one or add to an existing as I cannot see where this would be appropriate. In a new one called Special characters, just after Text markup list commands. http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: T1780 remove scheme format calls with no destination parameter - deprecated in Guile V2 (issue 4974078)
LGTM. On 2011/09/20 14:15:53, Ian Hulin (gmail) wrote: don't have to == don't need to, I assume you mean you shouldn't with tabulators in scripts/musicxml2ly.ly . Yes, this was a subtle DON'T DO IT! :) http://codereview.appspot.com/4974078/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue 4553056)
New patch set including some documentation work. http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue 4553056)
Hmm, I don't understand one comment, but I agree with the others. Thanks, Bertrand http://codereview.appspot.com/4553056/diff/103001/Documentation/notation/input.itely File Documentation/notation/input.itely (right): http://codereview.appspot.com/4553056/diff/103001/Documentation/notation/input.itely#newcode1636 Documentation/notation/input.itely:1636: @end lilypond What do you mean? http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue 4553056)
http://codereview.appspot.com/4553056/diff/103001/Documentation/notation/input.itely File Documentation/notation/input.itely (right): http://codereview.appspot.com/4553056/diff/103001/Documentation/notation/input.itely#newcode1636 Documentation/notation/input.itely:1636: @end lilypond Yes, unfortunately, this isn't simple. I didn't find any technical way to achieve this. http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New short and long lyric ties. (issue 4912041)
Pushed as 2fff263f10fd542454455994aea5ff3bbe075c7d http://codereview.appspot.com/4912041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add some polyphonically directed grobs (issue 4387046)
Pushed as f9251331576c94fa6aa4b85776917d3774b13b63 http://codereview.appspot.com/4387046/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Unifies mensural ligatures with blot-diameter. (issue 5030053)
Hi Pal! I updated the patch. This wasn't working since Werner's e10a13. It couldn't be applied to 0dcc93: I forgot some files that I added in 0e31cd. Bertrand http://codereview.appspot.com/5030053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Unifies mensural ligatures with blot-diameter. (issue 5030053)
On 2011/09/18 21:47:04, janek wrote: I think LGTM, but it would be great if you'd add a regtest to demonstrate what this patch is fixing. I don't think so. mensural-ligatures.ly contains every case fixed by this patch. If I make a regtest to show such tiny graphical differences, then we would need to do the same thing for almost every graphical object... http://codereview.appspot.com/5030053/diff/9001/lily/mensural-ligature.cc#newcode79 lily/mensural-ligature.cc:79: stencil = Lookup::beam (corrected_slope, width * 0.5, staff_space, blotdiameter); What does this do? (sorry for a stupid question) This is making a beam with round corners (blotdiameter is the radius of the round corner). These beams are used to represent the flexa (the big 'slide' at the end of the before/after PNGs). http://codereview.appspot.com/5030053/diff/9001/lily/mensural-ligature.cc#newcode201 lily/mensural-ligature.cc:201: (noteheads.sM2ligmensural).extent (Y_AXIS).length () * 0.5 I don't get it - why is this commented? Because the stems of these noteheads are not inside their Y-extent. This comment shows how this should ideally work. http://codereview.appspot.com/5030053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: T1780 remove scheme format calls with no destination parameter - deprecated in Guile V2 (issue 4974078)
Hi Ian, I have some comments. The rest of the patch LGTM. Regards, Bertrand http://codereview.appspot.com/4974078/diff/3001/scm/document-identifiers.scm File scm/document-identifiers.scm (right): http://codereview.appspot.com/4974078/diff/3001/scm/document-identifiers.scm#newcode31 scm/document-identifiers.scm:31: Why a new line? http://codereview.appspot.com/4974078/diff/3001/scm/lily.scm File scm/lily.scm (right): http://codereview.appspot.com/4974078/diff/3001/scm/lily.scm#newcode353 scm/lily.scm:353: (ly:format Err... Why is this required? Be careful with the indentation: there shouldn't be tabulators. http://codereview.appspot.com/4974078/diff/3001/scripts/musicxml2ly.py File scripts/musicxml2ly.py (right): http://codereview.appspot.com/4974078/diff/3001/scripts/musicxml2ly.py#newcode71 scripts/musicxml2ly.py:71: (ly:format #f ~a:~a den num))) #f? http://codereview.appspot.com/4974078/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves some parmesan noteheads. (issue 4639065)
Pushed as: 0dcc93c0a5a97d048db2f7631966f41ae0059ab5 and 0e31cd90e44673eca7ac59705ce4bed33dd8e80e Thank you all for this review! Bertrand http://codereview.appspot.com/4639065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Fix 155: parentheses include accidentals and dots. (issue 5047048)
Hi Joe, At last, a fix for that! But this looks unfinished. Are you working on a solution that would avoid having to specify the X-extent? A fix that calculates the font size of the ParenthesesItem according to the Y-extent of the parenthesized grobs would also be awesome :) Bertrand http://codereview.appspot.com/5047048/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Unifies mensural ligatures with blot-diameter. (issue 5030053)
Reviewers: benko.pal, Message: Hi, The new mensural style introduced with commit 0dcc93c0a5a97d048db2f7631966f41ae0059ab5 created some ugliness's in mensural ligatures. This patch fix these. I also added serifs to flexas. Regards, Bertrand Description: Unifies mensural ligatures with blot-diameter. Reverts some changes of commit 0dcc93c0a5a97d048db2f7631966f41ae0059ab5 Use blot-diameter either in C and MetaFont. Please review this at http://codereview.appspot.com/5030053/ Affected files: M lily/mensural-ligature.cc M mf/parmesan-noteheads.mf ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New short and long lyric ties. (issue 4912041)
New patch set. After thinking about that during three weeks, I decided to remove the long tie and even reduce the length of the default tie. Janek, thank you for showing me that overshooting vowels was useless and maybe disturbing. I also changed the è for a e... Thanks, Bertrand http://codereview.appspot.com/4912041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves some parmesan noteheads. (issue 4639065)
New patch set (inspired by Janek's ideas). On 2011/09/14 21:05:28, benko.pal wrote: * Use the left-stemmed longa in ligatures. exactly what is this last one? When note_shape is MLP_LONGA and the direction of the stem is UP, then the left-stemmed longa is used. I know, this should be Mensural_ligature_engraver's job. The mensural ligature engraver needs to be rewritten. All the postscript stuff should somehow be replaced with good old MetaFont. Thanks, Bertrand. http://codereview.appspot.com/4639065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New short and long lyric ties. (issue 4912041)
Thanks Janek. Does someone else thinks the long tie should be removed? Bertrand http://codereview.appspot.com/4912041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add some polyphonically directed grobs (issue 4387046)
Great. I removed the dynamics. Is someone opposed to this patch to be pushed? I suggest we put this issue in the next countdown. Thanks, Bertrand http://codereview.appspot.com/4387046/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves some parmesan noteheads. (issue 4639065)
Ok Pal, I removed the left-stemmed longa. I don't know where you could put these rules. Maybe we should start a new doc part that references the established engraving rules? Bertrand http://codereview.appspot.com/4639065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue 4553056)
New patch set. '' and '@' have been added to the lexer. '' is therefore a perfectly working escape character in lyrics. Bertrand http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: change longas similarly to how breves were changed (issue 4962072)
LGTM, with the same comment. http://codereview.appspot.com/4962072/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves some parmesan noteheads. (issue 4639065)
On 2011/09/12 22:37:05, janek wrote: i've looked at latest screenshot attached to tracker issue and... wow! It looks really great! Thanks a lot :) http://codereview.appspot.com/4639065/diff/13002/ly/engraver-init.ly#newcode1063 ly/engraver-init.ly:1063: \override Stem #'thickness = #2 I'd make them just a bit thinner, perhaps 1.8. I think that 2 might get too thick with smaller font-size (as font-size decreases, the relative thickness increases). You're right, I'll do that. Do you want to make a patch that sets some other parameters for the PetrucciStaff? http://codereview.appspot.com/4639065/diff/13002/mf/parmesan-noteheads.mf#newcode272 mf/parmesan-noteheads.mf:272: nm_red_holeheight := 2.5 linethickness; I'd make this 3 linethickness. That's what I tried at first, but this strangely looks more readable with 2.5. But I haven't yet compared printed results of different sizes, so you're maybe right. http://codereview.appspot.com/4639065/diff/13002/mf/parmesan-noteheads.mf#newcode329 mf/parmesan-noteheads.mf:329: nm_width := staff_space#; if i'm not mistaken and these are the height and width of half and quarter noteheads, i'd make them very slightly bigger - something like nm_height := 1.1 noteheight#; 329 nm_width := 1.05 staff_space#; Do you know what neomensural and mensural styles are inspired of? I always thought these notes were tiny, but it's maybe intended. As I am not an expert, I kept the shapes as is, but a rebuild would be great since we could get rid of draw_diamond_head (and finally have good ancient fonts). Anyway, the next steps will be (in order) to redesign the ancient rests, accidentals, clefs, time signatures and flags. Do I count you in? Bertrand http://codereview.appspot.com/4639065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves some parmesan noteheads. (issue 4639065)
On 2011/09/12 22:37:05, janek wrote: i've looked at latest screenshot attached to tracker issue and... wow! It looks really great! Thanks a lot :) http://codereview.appspot.com/4639065/diff/13002/ly/engraver-init.ly#newcode1063 ly/engraver-init.ly:1063: \override Stem #'thickness = #2 I'd make them just a bit thinner, perhaps 1.8. I think that 2 might get too thick with smaller font-size (as font-size decreases, the relative thickness increases). You're right, I'll do that. Do you want to make a patch that sets some other parameters for the PetrucciStaff? http://codereview.appspot.com/4639065/diff/13002/mf/parmesan-noteheads.mf#newcode272 mf/parmesan-noteheads.mf:272: nm_red_holeheight := 2.5 linethickness; I'd make this 3 linethickness. That's what I tried at first, but this strangely looks more readable with 2.5. But I haven't yet compared printed results of different sizes, so you're maybe right. http://codereview.appspot.com/4639065/diff/13002/mf/parmesan-noteheads.mf#newcode329 mf/parmesan-noteheads.mf:329: nm_width := staff_space#; if i'm not mistaken and these are the height and width of half and quarter noteheads, i'd make them very slightly bigger - something like nm_height := 1.1 noteheight#; 329 nm_width := 1.05 staff_space#; Do you know what neomensural and mensural styles are inspired of? I always thought these notes were tiny, but it's maybe intended. As I am not an expert, I kept the shapes as is, but a rebuild would be great since we could get rid of draw_diamond_head (and finally have good ancient fonts). Anyway, the next steps will be (in order) to redesign the ancient rests, accidentals, clefs, time signatures and flags. Do I count you in? Bertrand http://codereview.appspot.com/4639065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: parser.yy et al: turn \partial and \skip into music functions. (issue 4969076)
LGTM. Is there any way to also move \time, \key, \repeat and \alternative from the parser? Bertrand http://codereview.appspot.com/4969076/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New engraver for braces (issue 4807053)
On 2011/09/12 21:35:03, janek wrote: can you tell me what needs work in this patch? I've read Mike's comments, but i don't understand what should be done. This patch contains many copy/paste from the arpeggio engraver. We obviously need a new grob, since the braces need some special grob parameters like font-encoding=fetaBraces. But this doesn't necessarily require to add an engraver that would be a copy/paste from arpeggio-engraver. My other wish is to add text to braces, so that it can be used for annotations. I am close to a good solution, but still need some time to finish it. I hope this will be finished in a few weeks. Thanks, Bertrand http://codereview.appspot.com/4807053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves some parmesan noteheads. (issue 4639065)
Thanks for your reviews! On 2011/09/12 19:16:26, benko.pal wrote: http://codereview.appspot.com/4639065/diff/13002/lily/mensural-ligature.cc#newcode147 lily/mensural-ligature.cc:147: Direction stem_dir = stem ? get_grob_direction (stem) : CENTER; this is unneeded: there are no stemmed notes within ligaturae. I am a total ligature newbie. But I see some stemmed notes in input/regression/mensural-ligatures.ly . Of course, I agree that there's a bug. I fixed it in a new patch set. Bertrand. http://codereview.appspot.com/4639065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: include lines in breve X-extent (issue 1814) (issue 4931043)
On 2011/09/06 08:54:40, janek wrote: I'm not sure what you mean. Are you saying that i should assign (i * (gap + stemthick), 0) to a variable in the for loop? I.e. sth like for i := 0 step 1 until linecount - 1: foobar := (i * (gap + stemthick), 0); draw_gridline (z1 - foobar, z2 - foobar, stemthick); draw_gridline (z3 + foobar, z4 + foobar, stemthick); endfor; ? Yes, that would be cleaner and easier to understand. http://codereview.appspot.com/4931043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add some polyphonically directed grobs (issue 4387046)
Ok, DynamicText and DynamicLineSpanner should be removed. But what about the others? * Simple trill is directed (as a Script), so TrillSpanner logically has to be directed. * Slurs, Ties and TupletBrackets are also directed, so I don't see any reason for LigatureBrackets to behave differently. * AccidentalSuggestion in polyphony leads to mistakes if it isn't directed. Bertrand http://codereview.appspot.com/4387046/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improves horizontal spacing of axis groups that SpanBar grobs traverse. (issue 4917046)
This requires an update. The patch fails to apply. Bertrand http://codereview.appspot.com/4917046/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New partcombineUp and partcombineDown functions (issue 4514042)
Reviewers: J_lowe, dan_faithful.be,
Message:
I updated the patch and added a regtest.
Dan, I don't have time for now to rewrite the whole part combiner. Do
you want to do it ? If so, I can help you (at my level).
Bertrand
Description:
New partcombineUp and partcombineDown functions
Fix issue 1321
Please review this at http://codereview.appspot.com/4514042/
Affected files:
A input/regression/part-combine-3voices.ly
M lily/part-combine-iterator.cc
M ly/music-functions-init.ly
M scm/part-combiner.scm
Index: input/regression/part-combine-3voices.ly
diff --git a/input/regression/part-combine-3voices.ly
b/input/regression/part-combine-3voices.ly
new file mode 100644
index
..8dcb00966b4eec2b882b4562d9da3e49d79c96ab
--- /dev/null
+++ b/input/regression/part-combine-3voices.ly
@@ -0,0 +1,18 @@
+\version 2.15.10
+
+\header {
+ texidoc =It is possible to use the part combiner for three
+ voices with \\partcombineUp and \\partcombineDown.
+}
+
+
+soprano = { d''2 f'' g'' }
+alto = { a' c''4 d'' e''2 }
+tenor = { f'2 a'4 b' c''2 }
+basso = { d'4 e' f' g' g'2 }
+
+\new Staff \partcombineUp \soprano \alto \\ \basso
+
+\new Staff \soprano \\ \partcombineDown \tenor \basso
+
+
Index: lily/part-combine-iterator.cc
diff --git a/lily/part-combine-iterator.cc b/lily/part-combine-iterator.cc
index
4583d221f646ef9bcdc801dd46c4674366795fb3..9ab0495ae14ce909838c5498e5421d28590fd5bf
100644
--- a/lily/part-combine-iterator.cc
+++ b/lily/part-combine-iterator.cc
@@ -65,6 +65,11 @@ private:
Moment start_moment_;
SCM split_list_;
+ SCM direction_;
+ SCM directionOne_;
+ SCM directionTwo_;
+ SCM horizontalShiftOne_;
+ SCM horizontalShiftTwo_;
Stream_event *unisono_event_;
Stream_event *solo_one_event_;
@@ -133,6 +138,11 @@ Part_combine_iterator::Part_combine_iterator ()
first_iter_ = 0;
second_iter_ = 0;
split_list_ = SCM_EOL;
+ direction_ = SCM_BOOL_F;
+ directionOne_ = scm_from_int (1);
+ directionTwo_ = scm_from_int (-1);
+ horizontalShiftOne_ = scm_from_int (0);
+ horizontalShiftTwo_ = scm_from_int (1);
state_ = APART;
playing_state_ = APART;
last_playing_ = APART;
@@ -349,6 +359,17 @@ Part_combine_iterator::construct_children ()
{
start_moment_ = get_outlet ()-now_mom ();
split_list_ = get_music ()-get_property (split-list);
+ direction_ = get_music ()-get_property (direction);
+ if (is_direction (direction_))
+{
+ directionOne_ = direction_;
+ directionTwo_ = direction_;
+ if (scm_is_true (scm_negative_p (direction_)))
+{
+ horizontalShiftOne_ = scm_from_int (1);
+ horizontalShiftTwo_ = scm_from_int (0);
+}
+}
Context *c = get_outlet ();
@@ -369,6 +390,8 @@ Part_combine_iterator::construct_children ()
Context *two = handles_[CONTEXT_TWO].get_context ();
set_context (two);
second_iter_ = unsmob_iterator (get_iterator (unsmob_music (scm_cadr
(lst;
+ Context *shared = handles_[CONTEXT_SHARED].get_context ();
+ set_context (shared);
/* Mimic all settings of voiceOne/voiceTwo for the two separate
voices...*/
/* FIXME: Is there any way to use the definition of \voiceOne/\voiceTwo
@@ -391,16 +414,20 @@ Part_combine_iterator::construct_children ()
{
SCM sym = ly_symbol2scm (*p);
execute_pushpop_property (one, sym,
-ly_symbol2scm (direction), scm_from_int
(1));
+ly_symbol2scm (direction),
directionOne_);
execute_pushpop_property (two, sym,
-ly_symbol2scm (direction), scm_from_int
(-1));
+ly_symbol2scm (direction),
directionTwo_);
+
+ if (scm_is_number (direction_))
+execute_pushpop_property (shared, sym,
+ ly_symbol2scm (direction), direction_);
}
/* Handle horizontal shifts for crossing notes */
execute_pushpop_property (one, ly_symbol2scm (NoteColumn),
-ly_symbol2scm (horizontal-shift),
scm_from_int (0));
+ly_symbol2scm (horizontal-shift),
horizontalShiftOne_);
execute_pushpop_property (two, ly_symbol2scm (NoteColumn),
-ly_symbol2scm (horizontal-shift),
scm_from_int (1));
+ly_symbol2scm (horizontal-shift),
horizontalShiftTwo_);
/* Also handle MultiMeasureRest positions for voice 1/2 */
execute_pushpop_property (one, ly_symbol2scm (MultiMeasureRest),
ly_symbol2scm (staff-position), scm_from_int
(4));
Index: ly/music-functions-init.ly
diff --git a/ly/music-functions-init.ly b/ly/music-functions-init.ly
index
b3a009860753c54d7ab7549617ab4ba6f32d3ae5..1419daa808f7ebf5e65f33cb9116b940d1fdda67
100644
--- a/ly/music-functions-init.ly
+++ b/ly/music-functions-init.ly
@@ -767,7 +767,21 @@ partcombine =
(_i Take the music in @var{part1} and
Re: Add some polyphonically directed grobs (issue 4387046)
Updated. Bertrand http://codereview.appspot.com/4387046/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: include lines in breve X-extent (issue 1814) (issue 4931043)
Just a quick review. Bertrand http://codereview.appspot.com/4931043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: include lines in breve X-extent (issue 1814) (issue 4931043)
Sorry, I forgot to send the comments. http://codereview.appspot.com/4931043/diff/1/mf/feta-noteheads.mf File mf/feta-noteheads.mf (right): http://codereview.appspot.com/4931043/diff/1/mf/feta-noteheads.mf#newcode168 mf/feta-noteheads.mf:168: gap# := (0.95 - 0.008 * design_size) * stemthick#; You should save gap, line 161. http://codereview.appspot.com/4931043/diff/1/mf/feta-noteheads.mf#newcode169 mf/feta-noteheads.mf:169: define_pixels (gap); This should be moved after set_char_box. http://codereview.appspot.com/4931043/diff/1/mf/feta-noteheads.mf#newcode213 mf/feta-noteheads.mf:213: draw_gridline (z1 - (i * (gap + stemthick), 0), Hmmm... Don't you think (i * (gap + stemthick), 0) has to be written one time instead of four ? http://codereview.appspot.com/4931043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
lily-guile updates and CG: Scheme-C interface section. (issue 4917044)
Reviewers: , Message: Hi, Graham asked me to document an small issue. (http://codereview.appspot.com/4875054/) And this became something bigger. He also told me to push it directly, but I made a few changes in the interface as I read the code and wrote the doc. So the review concerns only the C files of this patch, but feel free to tell me if I was totally wrong when writing it. Thanks, Bertrand Description: lily-guile updates and CG: Scheme-C interface section. Please review this at http://codereview.appspot.com/4917044/ Affected files: M Documentation/contributor/programming-work.itexi M lily/general-scheme.cc M lily/include/lily-guile.hh M lily/lily-guile.cc ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes boolean/SCM confusions, part 1. (issue 4875054)
LGTM :) Bertrand http://codereview.appspot.com/4875054/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes boolean/SCM confusions, part 1. (issue 4875054)
(Two minor comments however). http://codereview.appspot.com/4875054/diff/1/lily/auto-beam-engraver.cc File lily/auto-beam-engraver.cc (right): http://codereview.appspot.com/4875054/diff/1/lily/auto-beam-engraver.cc#newcode172 lily/auto-beam-engraver.cc:172: return !scm_is_false (scm_call_4 (get_property (autoBeamCheck), Indent like this: return !scm_is_false (scm_call_4 ( get_property (autoBeamCheck), context ()-self_scm (), [...] http://codereview.appspot.com/4875054/diff/1/lily/bar-check-iterator.cc File lily/bar-check-iterator.cc (right): http://codereview.appspot.com/4875054/diff/1/lily/bar-check-iterator.cc#newcode56 lily/bar-check-iterator.cc:56: SCM sync = tr-get_property (barCheckSynchronize); Maybe you should also merge this line and line 65? But some will find it more convenient to see the properties defined here, so I don't know what is better. http://codereview.appspot.com/4875054/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New engraver for braces (issue 4807053)
That wasn't working 'cause I forgot to add some files to git... This is now fixed. Bertrand http://codereview.appspot.com/4807053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
New short and long lyric ties. (issue 4912041)
Reviewers: , Message: Hi everyone, This follows 8d148ea05fa4b34f8cc3407e112363d715b27ad8 This is fully working, except for a small issue in make doc. The two examples I put in the doc are working alone, but not with make doc: there should be short ties in ~è~, but we mysteriously get medium ties. This works if we change è for an ASCII character. I think this is somehow due to an encoding issue in the make doc process. Cheers, Bertrand Description: New short and long lyric ties. Please review this at http://codereview.appspot.com/4912041/ Affected files: M Documentation/de/notation/vocal.itely M Documentation/es/notation/vocal.itely M Documentation/fr/notation/vocal.itely M Documentation/notation/vocal.itely M mf/feta-ties.mf M scm/define-markup-commands.scm ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a glyph for tied lyrics. (issue 4808074)
use separate binding for (/ word-space 2) Done. http://codereview.appspot.com/4808074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a glyph for tied lyrics. (issue 4808074)
Pushed as 8d148ea05fa4b34f8cc3407e112363d715b27ad8 Bertrand http://codereview.appspot.com/4808074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes heights and pure heights of stems. (issue 4898044)
Benchmark done on a book with many beams: without the patch: 51.8s and 80 pages with the patch: 52.2s and 83 pages I also noticed some ugliness's between beams and figured bass. The figures stay exactly where they were, but the stems are longer and collide a bit with them. Bertrand http://codereview.appspot.com/4898044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes heights and pure heights of stems. (issue 4898044)
Same remark for fingerings. See input/regression/fingering-cross-staff.ly Consecutive ties look better in input/regression/tie-single.ly http://codereview.appspot.com/4898044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a glyph for tied lyrics. (issue 4808074)
Here's how a better solution should work: * create a smaller tie in addition to this one * use a scm regexp to find this special case; something like: #(use-modules (ice-9 regex)) #(display (regexp-substitute #f (string-match ([^:])~([^:])~([^:]) questa~è~in) 'pre 1 2 3 'post)) But I don't have time to do it right now. I'll push this patch when the countdown will be over and make another patch later for this special case. http://codereview.appspot.com/4808074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Fixes figuredBassCenterContinuations. (issue4868046)
Reviewers: ,
Message:
figuredBassCenterContinuations fails to merge continuation lines on if a
figure is composed of three or more figures.
This tiny patch fixes that.
Bertrand
Description:
Fixes figuredBassCenterContinuations.
Please review this at http://codereview.appspot.com/4868046/
Affected files:
M lily/figured-bass-engraver.cc
Index: lily/figured-bass-engraver.cc
diff --git a/lily/figured-bass-engraver.cc b/lily/figured-bass-engraver.cc
index
18b895660fa079c07523a31a1d4ea45802f3f0b3..24bf68db363eb88ae7ccd37a430f7759ddfbe5cf
100644
--- a/lily/figured-bass-engraver.cc
+++ b/lily/figured-bass-engraver.cc
@@ -217,19 +217,16 @@ Figured_bass_engraver::listen_bass_figure
(Stream_event *ev)
void
Figured_bass_engraver::center_continuations (vectorSpanner * const
consecutive_lines)
{
- if (consecutive_lines.size () == 2)
-{
- vectorGrob * left_figs;
- for (vsize j = consecutive_lines.size (); j--;)
-left_figs.push_back (consecutive_lines[j]-get_bound (LEFT));
+ vectorGrob * left_figs;
+ for (vsize j = consecutive_lines.size (); j--;)
+left_figs.push_back (consecutive_lines[j]-get_bound (LEFT));
- SCM ga = Grob_array::make_array ();
- unsmob_grob_array (ga)-set_array (left_figs);
+ SCM ga = Grob_array::make_array ();
+ unsmob_grob_array (ga)-set_array (left_figs);
- for (vsize j = consecutive_lines.size (); j--;)
-consecutive_lines[j]-set_object (figures,
- unsmob_grob_array
(ga)-smobbed_copy ());
-}
+ for (vsize j = consecutive_lines.size (); j--;)
+consecutive_lines[j]-set_object (figures,
+ unsmob_grob_array (ga)-smobbed_copy
());
}
void
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a glyph for tied lyrics. (issue4808074)
I forgot these cases (o~è~in; a~è~in; o~è~an...). They are often elided (quest'in instead of questa~è~in) by editors and composers. So your view is that lyric ties are not used in the real world? I still feel them as a pedagogy resource for young musicians or something. Or maybe old scores did not use them and they are now more often used. I don't know. Several famous editors use them, with no particular rule. Let's take the example of Ricordi Milano. In this score, published around 1982, there's no ties (we can see a na è in in the beginning): http://petrucci.mus.auth.gr/imglnks/usimg/2/20/IMSLP81325-PMLP165637-la_danza.pdf In this one, published around 1925, we can find ties (and it's not a beginner score!): http://216.129.110.22/files/imglnks/usimg/2/29/IMSLP80146-PMLP162554-Vivaldi_-_Agitata_da_due_venti__Griselda_.pdf In every recent score I have from them, they always use ties. Those which were done using traditional engraving as those using Finale. I will have to do a better patch... http://codereview.appspot.com/4808074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a glyph for tied lyrics. (issue4808074)
Secondly, it does not follow the policy for syllable separator, which is '[space]--[space]', not '-[space]'. I already fixed it in this patch (for every language). Finally, 2nd and 3rd stanzas look _very_ improbable to me in that it has three syllables on a single note, which requires two lyric ties. o~y is a synalepha and it could be a in-word diphthong, y~ho is another synalepha, but o~y~ho is not a triphthong and can not be a three-vowel synalepha. If nobody finds a real example in literature, I suggest to remove the problematic case, it is too artificial. I agree, I never saw such a case. http://codereview.appspot.com/4808074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Progress on loose columns. (issue4841052)
What if we want a larger padding? In your regtest, the clef collides with the third notehead when we set the padding with: \override Score.SpacingSpanner #'left-loose-column-padding = #2 There's also two trailing whitespaces. Regards, Bertrand http://codereview.appspot.com/4841052/diff/6001/input/regression/loose-column-spacing.ly File input/regression/loose-column-spacing.ly (right): http://codereview.appspot.com/4841052/diff/6001/input/regression/loose-column-spacing.ly#newcode13 input/regression/loose-column-spacing.ly:13: \clef treble g'' fis, trailing whitespace. http://codereview.appspot.com/4841052/diff/6001/lily/spacing-loose-columns.cc File lily/spacing-loose-columns.cc (right): http://codereview.appspot.com/4841052/diff/6001/lily/spacing-loose-columns.cc#newcode140 lily/spacing-loose-columns.cc:140: trailing whitespace. http://codereview.appspot.com/4841052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Progress on loose columns. (issue4841052)
LGTM (still one trailing whitespace in the regtest). Bertrand http://codereview.appspot.com/4841052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Sketch for fix of issue 307 (issue4813048)
This needs an update to be applied. Cheers, Bertrand http://codereview.appspot.com/4813048/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds a glyph for tied lyrics. (issue4808074)
Changes done. Passes make, regtest comparison and make doc. Bertrand http://codereview.appspot.com/4808074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Adds a glyph for tied lyrics. (issue4808074)
Reviewers: , Message: Better handling of lyric ties: * The space between two tied words is no more fixed. Its value is word-space. The default space is therefore decreased. * New glyph, so that we won't need an external font. * Increased vertical space between words and tie. It is therefore now possible to have commas in tied lyrics without any collision (example: verte,~et). How to build it: rm mf/out/feta* make Regards, Bertrand Description: Adds a glyph for tied lyrics. Please review this at http://codereview.appspot.com/4808074/ Affected files: M Documentation/de/notation/vocal.itely M Documentation/es/notation/vocal.itely M Documentation/fr/notation/vocal.itely M Documentation/notation/vocal.itely M mf/feta-generic.mf M mf/feta-test-generic.mf A mf/feta-ties.mf M scm/define-markup-commands.scm ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New engraver for braces (issue4807053)
This should work now. http://codereview.appspot.com/4807053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue4553056)
lily/text-interface.cc updated. http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds longas, maximas and non-standard tweaks to MultiMeasureRest (issue4536068)
Hi, I think I found a proper way to calculate church rests. I also updated so that it applies on the latest git HEAD. Bertrand. http://codereview.appspot.com/4536068/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds longas, maximas and non-standard tweaks to MultiMeasureRest (issue4536068)
Hi Pál (besides, are you Pál Benkő the chess master?) Thanks for this nice review. 1. about the very existence of usable-duration-logs - ok, it's generic, but who uses this genericity? is it not always (0 -1 -2 -3)? is it not always a range with lower end -3? is it not always a range? It can be something else. Some recent editors are also using half and quarter rests. One may also want to only have whole rests, so I guess we don't have to hard-code -3. I don't honestly know if this has to be range or if bounds would be enough. I solved the issue I mentioned in the last comment, so that church rests only pick duration logs from usable-duration-logs. I applied the other changes. Thanks again, Bertrand http://codereview.appspot.com/4536068/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue4553056)
I updated the patch. There's now a list of special characters and a \replace command for markups. The escape character is now '§'. It's the only one that works great with lyrics. And it isn't used elsewhere in LilyPond's syntax. The syntax for using the list of special characters #(include-special-characters) is a bit ackward but I didn't found anything better. Regards, Bertrand http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue4553056)
Ouch...
The problem with '' is that it fails on lyrics:
this works:
\new Lyrics \lyricmode { as; }
but this doesn't:
\new Lyrics \lyricmode { s; }
There is the same kind of issues with almost every easy-to-type special
character:
@ % $ # \ / ^ ~ + = * ; ( ) [ ] { }
This is the exact list of every possible escape characters for a french
keyboard under linux:
§ £ µ € ¤ ¹ ² ' ` © ¨ ø Ø ≤ ≥ « » “ ” ↓ ¬ × ÷ ¿ ¡ ∕ ⋅ …
Of course, only a few of them are available under windows:
§ £ µ € ¤ ' `
If we consider US, british, german and spanish keyboards, this only
gives these two very bad characters:
' `
The best solution may be to solve this lyric problem first :\
Bertrand
http://codereview.appspot.com/4553056/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
New engraver for braces (issue4807053)
Reviewers: , Message: Hi! This patchs allows to add braces the same way as arpeggios. Very useful for complex organ music. Check this out, there is several examples: http://imslp.org/wiki/12_Pi%C3%A8ces_pour_Orgue_(Gigout,_Eug%C3%A8ne) For the moment, the new engraver is a clone of Arpeggio_engraver. But I would like to add a command to attach text directly to braces (see IMSLP's score). Since there's many cases for the text attachment points, I think a new engraver is required. Done with Mike's help. The regression test is temporary. Regards, Bertrand PS: Li Jie Wong requested this last year : http://lists.gnu.org/archive/html/lilypond-user/2010-11/msg00361.html Please review this at http://codereview.appspot.com/4807053/ Affected files: A input/regression/braces.ly M lily/arpeggio.cc A lily/brace-engraver.cc M lily/include/arpeggio.hh M lily/span-arpeggio-engraver.cc A lily/span-brace-engraver.cc M ly/engraver-init.ly M ly/property-init.ly M scm/define-context-properties.scm M scm/define-event-classes.scm M scm/define-grob-interfaces.scm M scm/define-grob-properties.scm M scm/define-grobs.scm M scm/define-music-display-methods.scm M scm/define-music-types.scm M scm/safe-lily.scm ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds longas, maximas and non-standard tweaks to MultiMeasureRest (issue4536068)
http://codereview.appspot.com/4536068/diff/37002/lily/multi-measure-rest.cc#newcode241 lily/multi-measure-rest.cc:241: length = (2 -i) / 2; The division by 2 changes the result. Not that I understand too well what it is supposed to be doing. To be honest, I never understood well how this bitset operator works. What I see is that 2 -i gives the same result than 2^(-i+1). I should have write 2 (-i + 1) instead of /2... Besides this, I would like to remove these ugly ifs I added yesterday, but I don't see a better human-readable solution. Bertrand http://codereview.appspot.com/4536068/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds longas, maximas and non-standard tweaks to MultiMeasureRest (issue4536068)
I should have write 2 (-i + 1) instead of /2... the +1 multiplies by 2, not divides. Whoops, I meant -(i + 1)... perhaps the best would be 1 -i. Thanks. When I told you I never understood , I wasn't kidding! An update will follow this comment. Bertrand http://codereview.appspot.com/4536068/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds longas, maximas and non-standard tweaks to MultiMeasureRest (issue4536068)
It doesn't need to be ordered. It can have holes, but there's a small issue with this for now: Church rests are only looking for maximum and minimum values. You can therefore find longas in a church rest if you set usable-duration-logs to '(0 -1 -3). I don't know whether it's better to keep this system or to stick to usable-duration-logs. This would be logical to stick to the list, but church rests are something special that doesn't behave the same way than uncompressed multi-measure rests. Maybe a separated list for church rests would be the best solution... Thanks, Bertrand http://codereview.appspot.com/4536068/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New engraver for braces (issue4807053)
I updated the patch to work on the latest git HEAD. After doing a clean regtest comparison (git clean -fxd, config, make, make test-baseline, make check), I don't see anything. The logs' names show this is due to Mike's last commits. Regards, Bertrand http://codereview.appspot.com/4807053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds longas, maximas and non-standard tweaks to MultiMeasureRest (issue4536068)
This is really dirty, but here's an update that does what you want (I hope). If you have ideas to clean a bit this mess... Thanks, Bertrand http://codereview.appspot.com/4536068/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Adds longas, maximas and non-standard tweaks to MultiMeasureRest (issue4536068)
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode130 lily/multi-measure-rest.cc:130: double duration_log = -log2 (ml.Rational::to_double ()); log2 is a floating point operation not guaranteed to be exact. One usually uses a loop for figuring out a proper integer value. It is also not clear to me that this will pick out the right kind of rest always. For example, what to do about 3/2? We either get 2/1 rests, or 1/1 rests, and it is not clear to me that this rounding choice is necessarily the same as that for 3/4. I think it might be saner to just have an overrideable lookup list for exact meters (and 4/4 is not necessarily the same as 2/2 here), and revert to a separately configurable default otherwise, likely a whole rest. This sounds complex... I agree with you, but I don't know if others will since it adds a heavy autobeaming-like system. I need another day (or couple of days) to implement this feature. Anyway, I made all the other changes. Feel free to critisize, I still need to learn much about C/Scheme/LilyPond. Bertrand http://codereview.appspot.com/4536068/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue4553056)
Patch updated. I abandonned the idea of including a special characters list. http://codereview.appspot.com/4553056/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New alist to replace special characters. (issue4553056)
A new file in ly with definitions using add-text-replacements!?
Like this?
germanHTML = #(add-text-replacements! '((ss; . ß)
[...])
englishLaTeX = [...]
This way, users can easily stack special characters with this syntax:
\paper {
\englishHTML
\frenchLigatures
}
Besides, I made some performance tests.
With a big replacement alist, building Nicolas Sceaux's huge Atys takes
0.7s more than without it.
Thanks,
Bertrand
http://codereview.appspot.com/4553056/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New breve rest with ledger lines (issue4650052)
Yes, sorry for that. Just for information, ancient fonts are stored in the parmesan-* files. For the ancient fonts, we really need to think about a less hard-coded solution. http://codereview.appspot.com/4650052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New breve rest with ledger lines (issue4650052)
Updated with the good bounds. The problem with mensural/neomensural/Petrucci styles is that they often have different ledger line thicknesses. The best solution is to get these ledger lines out of metafont. But this requires much more work. For the moment, the exception is handled by the next if, line 100. Thanks! Bertrand http://codereview.appspot.com/4650052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New breve rest with ledger lines (issue4650052)
Maybe I should have been more clear in the description. The glyph I added in feta-rests.mf is for the default style. You have to remove mf/out/feta* before compiling. There's no problem when I try this patch with your example. But again, maybe I don't understand what you mean. Thanks http://codereview.appspot.com/4650052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
New breve rest with ledger lines (issue4650052)
Reviewers: ,
Message:
Minor patch that adds a useful glyph for baroque music.
Description:
New breve rest with ledger lines
Please review this at http://codereview.appspot.com/4650052/
Affected files:
M lily/rest.cc
M mf/feta-rests.mf
Index: lily/rest.cc
diff --git a/lily/rest.cc b/lily/rest.cc
index
9b85523f80ae2840ad273f8b1626891ef8f7493c..dcc7bdc2e643f9f05f1036b8ae6de91de7347470
100644
--- a/lily/rest.cc
+++ b/lily/rest.cc
@@ -80,7 +80,7 @@ string
Rest::glyph_name (Grob *me, int balltype, string style, bool try_ledgers)
{
bool is_ledgered = false;
- if (try_ledgers (balltype == 0 || balltype == 1))
+ if (try_ledgers (balltype == 0 || balltype == 1 || balltype == -1))
{
Real rad = Staff_symbol_referencer::staff_radius (me) * 2.0;
Real pos = Staff_symbol_referencer::get_position (me);
@@ -92,6 +92,7 @@ Rest::glyph_name (Grob *me, int balltype, string style,
bool try_ledgers)
*/
is_ledgered |= (balltype == 0) (pos = +rad + 2 || pos -rad);
is_ledgered |= (balltype == 1) (pos = -rad - 2 || pos +rad);
+ is_ledgered |= (balltype == -1) (pos = -rad - 3 || pos = +rad +
2);
}
string actual_style (style.c_str ());
Index: mf/feta-rests.mf
diff --git a/mf/feta-rests.mf b/mf/feta-rests.mf
index
4f270801793997ead28f603db6703fc950eaf107..1e416d36d37f8b40a861dd72c39ef523afba3a95
100644
--- a/mf/feta-rests.mf
+++ b/mf/feta-rests.mf
@@ -125,6 +125,24 @@ fet_beginchar (breve rest, M1);
draw_staff (-2, 2, 0);
fet_endchar;
+fet_beginchar (breve rest (outside staff), M1o);
+ set_char_box (0, breve_rest_x#,
+ ledgerlinethickness# / 2, breve_rest_y#);
+
+ draw_block ((0, 0), (breve_rest_x, breve_rest_y));
+
+ pickup pencircle scaled ledgerlinethickness;
+
+ y5 = y6 = breve_rest_y;
+ lft x5 = -b - breve_rest_y / 2;
+ rt x6 = w + breve_rest_y / 2;
+
+ draw_gridline (z5, z6, ledgerlinethickness_rounded);
+ draw_gridline ((x5, 0), (x6, 0), ledgerlinethickness_rounded);
+
+ draw_staff (-2, 2, 3);
+fet_endchar;
+
fet_beginchar (Quarter rest, 2);
save alpha, yshift, height;
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Current state of automatic footnotes. (issue4580041)
Hi Mike! There is a problem with your patch, I can not compile until the end, even after git clean -fxd. This crashes when building internals.texi And there is still the same spacing issues with number_raise and notes higher than 9... Thanks, Bertrand http://codereview.appspot.com/4580041/diff/2001/lily/page-layout-problem.cc File lily/page-layout-problem.cc (right): http://codereview.appspot.com/4580041/diff/2001/lily/page-layout-problem.cc#newcode209 lily/page-layout-problem.cc:209: annotation-translate_axis (footnote_stencil-extent (Y_AXIS)[UP] + number_raise - annotation-extent(Y_AXIS)[UP], Y_AXIS); Looks like the vertical spacing problem comes from here. Why not just : annotation-translate_axis (number_raise, Y_AXIS); ? Or with a Scheme interface that allows users to chose between fixed and variable height ? Same comment for line 239. http://codereview.appspot.com/4580041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
