Re: Fattens the 256 first braces. (issue4518052)
If you say you fatten the first 256 braces, does that mean that there is a jump in thickness when going from the 256th to the 257th brace? In that case, would it not be better to have a smoother transition rather than a jump? Metafont is good at curves, after all... http://codereview.appspot.com/4518052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fattens the 256 first braces. (issue4518052)
2011/5/11 : > If you say you fatten the first 256 braces, does that mean that there is > a jump in thickness when going from the 256th to the 257th brace? Is it a joke? -- Francisco Vila. Badajoz (Spain) www.paconet.org , www.csmbadajoz.com ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fattens the 256 first braces. (issue4518052)
2011/5/11 Francisco Vila : > 2011/5/11 : >> If you say you fatten the first 256 braces, does that mean that there is >> a jump in thickness when going from the 256th to the 257th brace? > > Is it a joke? Oh, of course it is. Sorry :*) -- Francisco Vila. Badajoz (Spain) www.paconet.org , www.csmbadajoz.com ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fattens the 256 first braces. (issue4518052)
I don't know if this is a joke, so I answer to David's question : of course there is no jump between the 256th and the 257th ! Refer to this message to see before/after : http://lists.gnu.org/archive/html/lilypond-devel/2011-05/msg00150.html http://codereview.appspot.com/4518052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fattens the 256 first braces. (issue4518052)
http://codereview.appspot.com/4518052/diff/2002/mf/feta-braces.mf File mf/feta-braces.mf (right): http://codereview.appspot.com/4518052/diff/2002/mf/feta-braces.mf#newcode151 mf/feta-braces.mf:151: fatten := 1 + fatten_factor - min ( number * fatten_factor / 256, fatten_factor); This is a bit obscure to me. How about fatten := min(number/256,1)[1+fatten_factor,1]; That makes it more obvious that the factor is running from 1+fatten_factor to 1. There does not seem to be much point in using min, either, regarding clarity. So maybe the whole thing can be written as if number<256: x := x*(number/256[1+fatten_factor,1]); fi However, "fatten_factor" sounds like a _factor_, so it would make more sense to change it to 2.5 (is that really the intended value? Seems really large!) and write this as if number<256: x := (number/256)[x*fatten_factor,x]; fi I think that expresses the meaning better, even without requiring an extra variable "fatten". http://codereview.appspot.com/4518052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fattens the 256 first braces. (issue4518052)
http://codereview.appspot.com/4518052/diff/2002/mf/feta-braces.mf File mf/feta-braces.mf (right): http://codereview.appspot.com/4518052/diff/2002/mf/feta-braces.mf#newcode151 mf/feta-braces.mf:151: fatten := 1 + fatten_factor - min ( number * fatten_factor / 256, fatten_factor); Thanks ! Hum... I agree with your idea of making this clearer. I also agree that my factor isn't really a factor since it's neutral value is 0. But the functions you gave do not produce the correct output. And I can't manage to find something cleaner than what I wrote. Also : we can't get rid of "fatten", as it is also used to thicken the ends of the braces. http://codereview.appspot.com/4518052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fattens the 256 first braces. (issue4518052)
On 2011/05/11 09:22:14, Bertrand Bordage wrote: http://codereview.appspot.com/4518052/diff/2002/mf/feta-braces.mf File mf/feta-braces.mf (right): http://codereview.appspot.com/4518052/diff/2002/mf/feta-braces.mf#newcode151 mf/feta-braces.mf:151: fatten := 1 + fatten_factor - min ( number * fatten_factor / 256, fatten_factor); Thanks ! Hum... I agree with your idea of making this clearer. I also agree that my factor isn't really a factor since it's neutral value is 0. But the functions you gave do not produce the correct output. And I can't manage to find something cleaner than what I wrote. Also : we can't get rid of "fatten", as it is also used to thicken the ends of the braces. Well, the middle formula was missing parens around number/256. Other than that, it should have worked fine. Something like fatten := min(number/256,1)[1+fatten_factor,1]; should do the trick then. http://codereview.appspot.com/4518052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fattens the 256 first braces. (issue4518052)
On 2011/05/11 09:54:36, dak wrote: Something like fatten := min(number/256,1)[1+fatten_factor,1]; should do the trick then. Well, I repeat the suggestion to make fatten_factor neutral at 1 instead of 0 (resulting in omitting 1+ in the above formula), and perhaps instead of hardcoding 256, one could make it a parameter fatten_end or similar. http://codereview.appspot.com/4518052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fattens the 256 first braces. (issue4518052)
:) Thanks ! This works great. Updated. http://codereview.appspot.com/4518052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fattens the 256 first braces. (issue4518052)
LGTM. I like the results! http://codereview.appspot.com/4518052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
MIDI: intelligently combine overlapping notes. (issue4520050)
Small variable nit upon rereading this patch. No change in semantics. http://codereview.appspot.com/4520050/diff/1/flower/include/pqueue.hh File flower/include/pqueue.hh (right): http://codereview.appspot.com/4520050/diff/1/flower/include/pqueue.hh#newcode121 flower/include/pqueue.hh:121: vsize next = i / 2; This should really be tgt / 2, for consistency. http://codereview.appspot.com/4520050/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Better width functions for the current arpeggio print functions. (issue4517051)
Seems reasonable to me. I couldn't think of any way to generalize the internal call. Carl http://codereview.appspot.com/4517051/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Better width functions for the current arpeggio print functions. (issue4517051)
On May 11, 2011, at 9:51 AM, carl.d.soren...@gmail.com wrote: > Seems reasonable to me. > > I couldn't think of any way to generalize the internal call. > > Carl > > > http://codereview.appspot.com/4517051/ Thanks! There is a note in arpeggio.cc saying that width cannot be gleaned from the print function because it triggers a vertical alignment when arpeggios are cross-staff. By turning the function into an internal function and calling it form outside functions, I'm assuming that this avoids triggering the vertical alignment, but I'm not sure this is the case. Does anyone know how/where/when/why the vertical alignment is triggered? Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Better width functions for the current arpeggio print functions. (issue4517051)
On 11 May 2011 15:18, m...@apollinemike.com wrote: > There is a note in arpeggio.cc saying that width cannot be gleaned from the > print function because it triggers a vertical alignment when arpeggios are > cross-staff. By turning the function into an internal function and calling > it form outside functions, I'm assuming that this avoids triggering the > vertical alignment, but I'm not sure this is the case. Does anyone know > how/where/when/why the vertical alignment is triggered? Probably when the print function reads 'positions: ly:arpeggio::calc-positions requests the common refpoint in the Y-axis, which will be a VerticalAlignment for cross-staff arpeggios. Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Better width functions for the current arpeggio print functions. (issue4517051)
http://codereview.appspot.com/4517051/diff/1/lily/arpeggio.cc File lily/arpeggio.cc (right): http://codereview.appspot.com/4517051/diff/1/lily/arpeggio.cc#newcode98 lily/arpeggio.cc:98: MAKE_SCHEME_CALLBACK (Arpeggio, internal_print, 1); Why are you exporting these internal functions? http://codereview.appspot.com/4517051/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: NR 1.6.3 - edit eg.s in quoting other voices (issue4518053)
Reviewers: Graham Percival, Keith,
Message:
Second Draft
http://codereview.appspot.com/4518053/diff/1/Documentation/notation/staff.itely
File Documentation/notation/staff.itely (right):
http://codereview.appspot.com/4518053/diff/1/Documentation/notation/staff.itely#newcode1022
Documentation/notation/staff.itely:1022: @cindex quote, voices
On 2011/05/10 19:59:35, Graham Percival wrote:
could we keep the old @cindex as well? I'm fine with adding the new
ones; just
don't remove the old ones.
I didn't remove as much as reword for clarity. Else we get into
semantics about (for example) cues vs cue notes (cues means nothing -
snooker cues? 'cue notes' is what they are properly called). Again
'Fragments' is meaningless in an index without some context - which I
added (Fragments, quoting).
http://codereview.appspot.com/4518053/diff/1/Documentation/notation/staff.itely#newcode1045
Documentation/notation/staff.itely:1045: \addQuote "flute" @{
\fluteNotes @}
On 2011/05/11 03:26:40, Keith wrote:
> remove the @example and just go directly into the @lilypond.
That would be better.
Done.
http://codereview.appspot.com/4518053/diff/1/Documentation/notation/staff.itely#newcode1045
Documentation/notation/staff.itely:1045: \addQuote "flute" @{
\fluteNotes @}
On 2011/05/10 19:59:35, Graham Percival wrote:
could we keep a definition for \fluteNotes ? again, I'm happy to have
the name
change, but I'd like to see some notes actually being defined.
alternately, remove the @example and just go directly into the
@lilypond. That
would be my first instinct, actually, although I can't claim to have
looked at
this in detail.
Done.
http://codereview.appspot.com/4518053/diff/1/Documentation/notation/staff.itely#newcode1072
Documentation/notation/staff.itely:1072: If the music expression used in
@code{\quoteDuring} contains notes
On 2011/05/11 03:26:40, Keith wrote:
I suggest removing this whole paragraph and @lilypond.
1. The original text was wrong: \quoteDuring does not create another
Voice for
polyphony; instead it adds notes to the existing Voice to make chords.
2. Using \cueDuring, by contrast, *does* add a voice and create a
polyphonic
situation, and in fact I have found that to be useful, but was
discouraged from
trying it because of the old text.
I think we need something, somewhere about this, as it would appear that
this is a nice way to add polyphony - even if it isn't and to be honest
I don't know what the difference is, or the subtlety is lost on me. It
LOOKS like polyphony.
Maybe an appropriate @KNOWNISSUE?
http://codereview.appspot.com/4518053/diff/1/Documentation/notation/staff.itely#newcode1082
Documentation/notation/staff.itely:1082: c4 cis c b \quoteDuring
#"flute" { e4 dis b a }
On 2011/05/11 03:26:40, Keith wrote:
c4 cis c b \quoteDuring #"flute" { e4 r8 ais b4 a }
If you do decide to keep this @lilypond, at least make it output
something
sensible.
Kept it in for now until we decide on the last comment.
http://codereview.appspot.com/4518053/diff/1/Documentation/notation/staff.itely#newcode1101
Documentation/notation/staff.itely:1101: a4 gis g gis | b4^"quoted" r8
ais\p a4( f)
On 2011/05/11 03:26:40, Keith wrote:
\key d\major
b4 ais a ais | cis4^"quoted" r8 bis\p b4( f)
so that this example realistically shows the same concert-pitch notes
as the
earlier examples.
(I should have caught this error months ago.)
Done.
http://codereview.appspot.com/4518053/diff/1/Documentation/notation/staff.itely#newcode1120
Documentation/notation/staff.itely:1120: markups, etc. and is inserted
into the music expression. It is possible
On 2011/05/11 03:26:40, Keith wrote:
markups, etc. --[and is inserted into the music expression]--
Not sure what you wanted here, but I have improved this awkward sentence
anyway.
Description:
Doc: NR 1.6.3 - edit eg.s in quoting other voices
Suggestion from Keith O'Hara.
An attempt to give a more realistic example than what was previously
there.
It also helps emphasise that it must be made at the top level but also
that the
order of the \quoteDurings and \addQuotes is not prescribed.
Minor improvements of syntax and I moved around some of the information
to make it flow more easily. Removed unnecessary text (either redundant,
repeated or shown in the example).
Please review this at http://codereview.appspot.com/4518053/
Affected files:
M Documentation/notation/staff.itely
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: NR 1.6.3 - edit eg.s in quoting other voices (issue4518053)
LGTM
http://codereview.appspot.com/4518053/diff/5001/Documentation/notation/staff.itely
File Documentation/notation/staff.itely (right):
http://codereview.appspot.com/4518053/diff/5001/Documentation/notation/staff.itely#newcode1090
Documentation/notation/staff.itely:1090: The @code{\transposition}
command can also be used with quotes. See
I added the former text at this point only a couple months ago, so I
think we need a bit more explanation. Maybe :
"The @code{\quoteDuring} command uses the @\code{\transposition}
settings of both quoted and quoting parts to produce notes for the
quoting part that have the same sounding pitch as those in the quoted
part."
Section @ref{instrument transpositions} does not explain this point very
well, because an example of quoting is required to really explain it.
http://codereview.appspot.com/4518053/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Better width functions for the current arpeggio print functions. (issue4517051)
On May 11, 2011, at 12:02 PM, n.putt...@gmail.com wrote: > > http://codereview.appspot.com/4517051/diff/1/lily/arpeggio.cc > File lily/arpeggio.cc (right): > > http://codereview.appspot.com/4517051/diff/1/lily/arpeggio.cc#newcode98 > lily/arpeggio.cc:98: MAKE_SCHEME_CALLBACK (Arpeggio, internal_print, 1); > Why are you exporting these internal functions? > > http://codereview.appspot.com/4517051/ The internal function idea is now bunk - you're right about the positions thing. The issue is that, for the chord bracket and chord slur (and Bertrand's eventual chord brace, which hypothetically varies significantly in its X dimension as it gets larger), the width of the grob is dependent on knowing the extremal note head positions, which triggers a vertical alignment. Any suggestions on how to get this information without triggering the vertical alignment? Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Better width functions for the current arpeggio print functions. (issue4517051)
On 11 May 2011 19:11, wrote: > The issue is that, for the chord bracket and chord slur (and Bertrand's > eventual chord brace, which hypothetically varies significantly in its X > dimension as it gets larger), the width of the grob is dependent on knowing > the extremal note head positions, which triggers a vertical alignment. Surely not the chord bracket? It has a fixed width (line 171 of arpeggio.cc), though perhaps it should be tweakable. > Any suggestions on how to get this information without triggering the > vertical alignment? Sorry, no. Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix calculation of vertical offset when 'staff-padding is set (issue4489042)
LGTM.
http://codereview.appspot.com/4489042/diff/1/input/regression/clef-octavation.ly
File input/regression/clef-octavation.ly (right):
http://codereview.appspot.com/4489042/diff/1/input/regression/clef-octavation.ly#newcode3
input/regression/clef-octavation.ly:3: \header{
\header {
http://codereview.appspot.com/4489042/diff/1/input/regression/clef-octavation.ly#newcode5
input/regression/clef-octavation.ly:5: texidoc=" Octavate symbols should
be correctly positioned close to
texidoc = "Octavate
http://codereview.appspot.com/4489042/diff/1/input/regression/clef-octavation.ly#newcode11
input/regression/clef-octavation.ly:11: \new Staff { \clef "G^8" g''1 }
indent
http://codereview.appspot.com/4489042/diff/1/input/regression/clef-octavation.ly#newcode18
input/regression/clef-octavation.ly:18: \layout { ragged-right = ##t }
remove
http://codereview.appspot.com/4489042/diff/1/lily/side-position-interface.cc
File lily/side-position-interface.cc (right):
http://codereview.appspot.com/4489042/diff/1/lily/side-position-interface.cc#newcode280
lily/side-position-interface.cc:280: Real diff = dir * staff_extent[dir]
+ staff_padding
add parentheses to enforce indent
http://codereview.appspot.com/4489042/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Better width functions for the current arpeggio print functions. (issue4517051)
On May 11, 2011, at 2:19 PM, Neil Puttock wrote: > On 11 May 2011 19:11, wrote: > >> The issue is that, for the chord bracket and chord slur (and Bertrand's >> eventual chord brace, which hypothetically varies significantly in its X >> dimension as it gets larger), the width of the grob is dependent on knowing >> the extremal note head positions, which triggers a vertical alignment. > > Surely not the chord bracket? It has a fixed width (line 171 of > arpeggio.cc), though perhaps it should be tweakable. > Sorry - yes, you're right. I amend my comment to chord slurs and braces. >> Any suggestions on how to get this information without triggering the >> vertical alignment? > > Sorry, no. > I'll try to think of something... ~Mike ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Making website build almost completely quiet (issue4515042)
looks basically good. http://codereview.appspot.com/4515042/diff/1/make/website.make File make/website.make (right): http://codereview.appspot.com/4515042/diff/1/make/website.make#newcode8 make/website.make:8: quiet-run = $(findstring s, $(MAKEFLAGS)) I think I'd still rather have this be: quiet-run = TRUE or True or true or however you do this in make http://codereview.appspot.com/4515042/diff/1/make/website.make#newcode10 make/website.make:10: quiet-string=-q can this name be more descriptive? like quiet-option ? (I'm not overly fond of that suggestion; I'm just trying to spark your imagination) http://codereview.appspot.com/4515042/diff/1/scripts/build/bib2texi.py File scripts/build/bib2texi.py (right): http://codereview.appspot.com/4515042/diff/1/scripts/build/bib2texi.py#newcode16 scripts/build/bib2texi.py:16: suppress_output = '' could this be a boolean instead? http://codereview.appspot.com/4515042/diff/1/scripts/build/bib2texi.py#newcode27 scripts/build/bib2texi.py:27: suppress_output = ' -terse ' set the boolean here http://codereview.appspot.com/4515042/diff/1/scripts/build/bib2texi.py#newcode63 scripts/build/bib2texi.py:63: cmd = "TEXMFOUTPUT=%s bibtex %s %s" % (tmpdir, suppress_output, tmpfile) maybe make an extra var here: suppress_output_command = '' if suppress_output: suppress_output_command = ' -terse ' http://codereview.appspot.com/4515042/diff/1/scripts/build/bib2texi.py#newcode65 scripts/build/bib2texi.py:65: if (suppress_output != ' -terse '): if suppress_output: the reason for all this is that I don't like seeing stuff like the line above; it feels "fragile" to me. Not that I ever expect the command-line option of bibtex to change, but still... http://codereview.appspot.com/4515042/diff/1/scripts/build/mass-link.py File scripts/build/mass-link.py (right): http://codereview.appspot.com/4515042/diff/1/scripts/build/mass-link.py#newcode16 scripts/build/mass-link.py:16: # PH note - I've just commented this out, since it's so not-helpful I suggest removing it entirely. http://codereview.appspot.com/4515042/diff/1/scripts/build/website-known-missing-files.txt File scripts/build/website-known-missing-files.txt (right): http://codereview.appspot.com/4515042/diff/1/scripts/build/website-known-missing-files.txt#newcode1 scripts/build/website-known-missing-files.txt:1: ancient-notation.itely What's this file? Didn't we already have this file in the previous commit? http://codereview.appspot.com/4515042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: MIDI: intelligently combine overlapping notes. (issue4520050)
Reviewers: md5i, Message: This looks good overall. I will test out my own suggestions, because if we can make a smaller change, it will be easier to follow through this complicated stretch of history in the MIDI-output code. http://codereview.appspot.com/4520050/diff/1/flower/include/pqueue.hh File flower/include/pqueue.hh (right): http://codereview.appspot.com/4520050/diff/1/flower/include/pqueue.hh#newcode121 flower/include/pqueue.hh:121: vsize next = i / 2; On 2011/05/11 13:41:04, md5i wrote: This should really be tgt / 2, for consistency. Done http://codereview.appspot.com/4520050/diff/9001/flower/include/pqueue.hh#newcode110 flower/include/pqueue.hh:110: void del (vsize i) Rather than change the 'flower' library to support deletion from the heap, I suggest we continue to use the ignore_ flag on items we would delete. The formerly-used "lazy deletion" technique seems appropriate for the note-of queue, because deletions are rare. http://codereview.appspot.com/4520050/diff/9001/lily/midi-walker.cc File lily/midi-walker.cc (right): http://codereview.appspot.com/4520050/diff/9001/lily/midi-walker.cc#newcode94 lily/midi-walker.cc:94: if (now_ticks == stop_note_queue[i].val.first The use of the .first field to cache the note-on-time was difficult for me to understand. I suggest following pointers as was done to find now_ticks: int queued_ticks = stop_note_queue[i].val.second->audio_->audio_column_->ticks () Description: MIDI: intelligently combine overlapping notes. Fix issue 1647 A patch from Micheal Duggan Please review this at http://codereview.appspot.com/4520050/ Affected files: M flower/include/pqueue.hh A input/regression/midi-unisons.ly M lily/audio-staff.cc M lily/include/audio-staff.hh M lily/include/midi-walker.hh M lily/midi-walker.cc M lily/staff-performer.cc M scm/define-context-properties.scm ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: MIDI: intelligently combine overlapping notes. (issue4520050)
On 2011/05/11 23:12:08, Keith wrote: http://codereview.appspot.com/4520050/diff/9001/flower/include/pqueue.hh#newcode110 flower/include/pqueue.hh:110: void del (vsize i) Rather than change the 'flower' library to support deletion from the heap, I suggest we continue to use the ignore_ flag on items we would delete. The formerly-used "lazy deletion" technique seems appropriate for the note-of queue, because deletions are rare. Makes sense to me. I don't recall if "_ignore" existed when I originally wrote the patch, several years ago now. http://codereview.appspot.com/4520050/diff/9001/lily/midi-walker.cc File lily/midi-walker.cc (right): http://codereview.appspot.com/4520050/diff/9001/lily/midi-walker.cc#newcode94 lily/midi-walker.cc:94: if (now_ticks == stop_note_queue[i].val.first The use of the .first field to cache the note-on-time was difficult for me to understand. I suggest following pointers as was done to find now_ticks: int queued_ticks = stop_note_queue[i].val.second->audio_->audio_column_->ticks () Good call, in my opinion. I was never happy with the Pair. http://codereview.appspot.com/4520050/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
