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: Unifies mensural ligatures with blot-diameter. (issue 5030053)
On 2011/09/18 19:55:47, Bertrand Bordage wrote: Another update that fixes some variable errors. It now passes make. thanks Bertrand, this is great work; I can now print flexae just the default way! (so long I had to use non-default viewers and lpr commands.) p 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)
http://codereview.appspot.com/5030053/diff/2004/lily/mensural-ligature.cc File lily/mensural-ligature.cc (right): http://codereview.appspot.com/5030053/diff/2004/lily/mensural-ligature.cc#newcode74 lily/mensural-ligature.cc:74: = (me-layout ()-get_dimension (ly_symbol2scm (blot-diameter))); remove extra parens 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)
2011/9/19 bordage.bertr...@gmail.com: 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. True. However, your changes are very subtle and extremely easy to overlook. I'm sure that if someone looked at the regtest comparison without knowing what to look for, he would miss it entirely and think that nothing changed. 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... I seriously consider adding dedicated regtests for all such tiny graphical differencies. Please see this thread for a more detailed discussion about it: http://lists.gnu.org/archive/html/lilypond-devel/2011-09/msg00308.html 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). Ah, so Lookup::beam () prints a beam sybol! Nice. 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. ok, i understand now. thanks, Janek ___ 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 Bertrand, The new mensural style introduced with commit 0dcc93c0a5a97d048db2f7631966f41ae0059ab5 created some ugliness's in mensural ligatures. which commit is the patch supposed to apply to? I couldn't apply it to my HEAD f8a4491c571dc57c87aec33dc8e34c436e222537; having applied it to 0dcc93 it produced empty longae in ligatures, after removing the build directory even make failed. p ___ 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)
hi Bertrand, Another update that fixes some variable errors. It now passes make. this is ok, passes apply, make and all my test; thanks! p 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)
I think LGTM, but it would be great if you'd add a regtest to demonstrate what this patch is fixing. (i was going to write before/after pdfs attached to tracker issue would be priceless! but i've just saw that you added them - perfect!) thanks, Janek http://codereview.appspot.com/5030053/diff/9001/lily/mensural-ligature.cc File lily/mensural-ligature.cc (right): 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) 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? 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
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: Unifies mensural ligatures with blot-diameter. (issue 5030053)
Passes make and reg tests - I get differences but they look ok see: http://code.google.com/p/lilypond/issues/detail?id=1898#c1 http://codereview.appspot.com/5030053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
