Re: include lines in breve X-extent (issue 1814) (issue 4931043)
On Wed, May 9, 2012 at 5:01 PM, Janek Warchoł wrote: > On Wed, May 9, 2012 at 8:03 AM, wrote: >> >> 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#newcode183 >> mf/feta-noteheads.mf:183: set_char_box (stemthick# * linecount + gap# * >> (linecount - 1), >> Jan, the breves seem to be the only note-heads that have an extent to >> the left of the reference point. Presumably this is to make them line >> up properly with whole-notes in note-columns. >> It complicates chord-collisions a bit, so take a look at the issue 1713 >> patch if you can. > > I'll try - hopefully when i come back home today evening. I've compiled it and checked the output of a test file; i see the problem. Unfortunately i'm too tired to continue testing today :( thanks, Janek ___ 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 Wed, May 9, 2012 at 8:03 AM, wrote: > > 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#newcode183 > mf/feta-noteheads.mf:183: set_char_box (stemthick# * linecount + gap# * > (linecount - 1), > Jan, the breves seem to be the only note-heads that have an extent to > the left of the reference point. Presumably this is to make them line > up properly with whole-notes in note-columns. > It complicates chord-collisions a bit, so take a look at the issue 1713 > patch if you can. I'll try - hopefully when i come back home today evening. cheers, Janek ___ 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)
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#newcode183 mf/feta-noteheads.mf:183: set_char_box (stemthick# * linecount + gap# * (linecount - 1), Jan, the breves seem to be the only note-heads that have an extent to the left of the reference point. Presumably this is to make them line up properly with whole-notes in note-columns. It complicates chord-collisions a bit, so take a look at the issue 1713 patch if you can. 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)
Reviewers: janek, Bertrand Bordage, MikeSol, J_lowe, Ian Hulin (gmail), graham_percival-music.ca, Message: On 2011/09/06 16:26:40, Bertrand Bordage wrote: 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. Done (http://codereview.appspot.com/4986042/) Description: include lines in breve X-extent (issue 1814) char box of breve glyphs is widened to include the lines, not only notehead. This will allow Lily to calculate collisions with breves correctly. It shouldn't change how breves are aligned in note columns. Please review this at http://codereview.appspot.com/4931043/ Affected files: M mf/feta-noteheads.mf Index: mf/feta-noteheads.mf diff --git a/mf/feta-noteheads.mf b/mf/feta-noteheads.mf index a58e4bc89a7a6c607a3da1dea2478ecce9b8ba0f..1c208b651d9a248bb48b10deb05c3ff63d4c11d5 100644 --- a/mf/feta-noteheads.mf +++ b/mf/feta-noteheads.mf @@ -164,6 +164,11 @@ def draw_brevis (expr linecount, line_thickness_multiplier) = define_whole_blacker_pixels (stemthick); % Breves of smaller design sizes should have their lines + % farther apart. + gap# := (0.95 - 0.008 * design_size) * stemthick#; + define_pixels (gap); + + % Breves of smaller design sizes should have their lines % farther apart (the overlap should be smaller). fudge = hround (blot_diameter * min (max (-0.15, @@ -175,6 +180,11 @@ def draw_brevis (expr linecount, line_thickness_multiplier) = draw_outside_ellipse (1.80, 0, 0.707, 0); undraw_inside_ellipse (1.30, 125, 0.68, 2 stafflinethickness#); + set_char_box (stemthick# * linecount + gap# * (linecount - 1), + width# + stemthick# * linecount + gap# * (linecount - 1), + noteheight# / 2, + noteheight# / 2); + pickup pencircle scaled stemthick; % Breves of smaller design sizes should have their lines longer. @@ -194,20 +204,17 @@ def draw_brevis (expr linecount, line_thickness_multiplier) = rt x1 - fudge = 0; x1 = x2; - fudge + lft x3 = w; + fudge + lft x3 = width; x4 = x3; y4 = y2; y3 = y1; - % Breves of smaller design sizes should have their lines - % farther apart. - line_distance := (1.95 - 0.008 * design_size) * stemthick; for i := 0 step 1 until linecount - 1: - draw_gridline (z1 - (i * line_distance, 0), - z2 - (i * line_distance, 0), + draw_gridline (z1 - (i * (gap + stemthick), 0), + z2 - (i * (gap + stemthick), 0), stemthick); - draw_gridline (z3 + (i * line_distance, 0), - z4 + (i * line_distance, 0), + draw_gridline (z3 + (i * (gap + stemthick), 0), + z4 + (i * (gap + stemthick), 0), stemthick); endfor; enddef; ___ 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: include lines in breve X-extent (issue 1814) (issue 4931043)
On Tue, Sep 06, 2011 at 08:54:40AM +, janek.lilyp...@gmail.com wrote: > I have new patch set ready, but i have trouble logging as another user > in git-cl (this issue was created by my previous e-mail account). Can > you give me some clue? Just make a new issue. Follow the instructions in the CG to "reset git-cl". Cheers, - Graham ___ 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)
Ian, On 2011/08/30 10:29:42, Ian Hulin (gmail) wrote: Janek, Bertrand posted some review comments here. I think it would be polite in the case of a newer contributor like Bertrand to post some responses one way or another (either "don't worry about it, because. . ." or "nice catch, I'll upload an updated patch set".) You are absolutely right! I had limited access to my e-mail and missed this. Sorry, Bertrand! I have new patch set ready, but i have trouble logging as another user in git-cl (this issue was created by my previous e-mail account). Can you give me some clue? 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#; On 2011/08/25 15:03:04, Bertrand Bordage wrote: You should save gap, line 161. Done. http://codereview.appspot.com/4931043/diff/1/mf/feta-noteheads.mf#newcode169 mf/feta-noteheads.mf:169: define_pixels (gap); On 2011/08/25 15:03:04, Bertrand Bordage wrote: This should be moved after set_char_box. Done. http://codereview.appspot.com/4931043/diff/1/mf/feta-noteheads.mf#newcode213 mf/feta-noteheads.mf:213: draw_gridline (z1 - (i * (gap + stemthick), 0), On 2011/08/25 15:03:04, Bertrand Bordage wrote: Hmmm... Don't you think (i * (gap + stemthick), 0) has to be written one time instead of four ? 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; ? 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)
Janek, Bertrand posted some review comments here. I think it would be polite in the case of a newer contributor like Bertrand to post some responses one way or another (either "don't worry about it, because. . ." or "nice catch, I'll upload an updated patch set".) Cheers, Ian 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)
LGTM Patch applied and it fixes documents originally showing collisions between double-lined breves, bar-lines and accidentals. Cheers, Ian 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)
passes make and reg tests 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
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
include lines in breve X-extent (issue 1814) (issue 4931043)
http://codereview.appspot.com/4931043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
