Re: Add tab-tie-follow-engraver (issue2723043)
Am 07.12.2010 00:56, schrieb Carl Sorensen: [...] I'm happy to go with 3/5, as you recommend. I've added the head-offset property to TabNoteHead 'details, set its default value to 3/5, and pushed the changes. Thanks! It's great to see how older lilypond input files are now compiled and the resulting scores look much better! Regards, Marc ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 12/6/10 3:37 AM, "Marc Hohl" wrote: > Am 05.12.2010 23:12, schrieb Carl Sorensen: >> >> >> [...] >>> Perhaps the left edge of a number should be aligned to the left edge of >>> a note head >>> when the fret number is a single digit, whereas fret numbers> 9 should >>> be centered >>> according to the center of the left-aligned single digits (I don't know how >>> to describe this in a simpler way...). >>> >>> >> Marc, >> >> If you get the most recent patch from Rietveld, and look at line 276 of >> scm/tablatures.scm, you'll see the constant that adjusts things -- the 2/3. >> > Hello Carl, > > I fiddled around with the value, and it seems that 3/5 is the best IMHO. > 1/2 is exactly what I described, but it doesn't look right. I didn't like 1/2 either. That's why I went with 2/3, but that was just because it seemed to be centered on the intersection between the regular note heads and the staff lines for notes in staff spaces. I'm happy to go with 3/5, as you recommend. I've added the head-offset property to TabNoteHead 'details, set its default value to 3/5, and pushed the changes. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Am 05.12.2010 23:12, schrieb Carl Sorensen: [...] Perhaps the left edge of a number should be aligned to the left edge of a note head when the fret number is a single digit, whereas fret numbers> 9 should be centered according to the center of the left-aligned single digits (I don't know how to describe this in a simpler way...). Marc, If you get the most recent patch from Rietveld, and look at line 276 of scm/tablatures.scm, you'll see the constant that adjusts things -- the 2/3. Hello Carl, I fiddled around with the value, and it seems that 3/5 is the best IMHO. 1/2 is exactly what I described, but it doesn't look right. Would you please test this with multiple values of that constant, and come up with a value that you like? I think a value of 1/2 would give the behavior you described above. I'd like your opinion on what would be a preferred value. Once you have a good value, I think I'll modify the patch so that constant is part of the TabNoteHead 'details, which will give the user control over it. Sounds good! So anyone who feels unhappy with 3/5 could change this easily ;-) Regards, Marc ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 12/5/10 12:22 PM, "Marc Hohl" wrote: > Am 05.12.2010 02:19, schrieb carl.d.soren...@gmail.com: >> On 2010/12/05 00:58:13, Neil Puttock wrote: >>> I don't know, but they're consistently shifted to the right unless >>> there are double-digit notes present. >> >> I wanted your opinion on the spacing in the .png I sent. I adjusted the >> offset, but it wasn't part of this patch. >> >> I have deliberately shifted the tab note heads to the right, because >> they were slightly too far to the left, in my opinion. > I don't know why, but in the .png it looks as if they were almost > right-aligned to the > note heads, but in fact, they are centered. > > Perhaps the left edge of a number should be aligned to the left edge of > a note head > when the fret number is a single digit, whereas fret numbers > 9 should > be centered > according to the center of the left-aligned single digits (I don't know how > to describe this in a simpler way...). > Marc, If you get the most recent patch from Rietveld, and look at line 276 of scm/tablatures.scm, you'll see the constant that adjusts things -- the 2/3. Would you please test this with multiple values of that constant, and come up with a value that you like? I think a value of 1/2 would give the behavior you described above. I'd like your opinion on what would be a preferred value. Once you have a good value, I think I'll modify the patch so that constant is part of the TabNoteHead 'details, which will give the user control over it. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Am 05.12.2010 02:19, schrieb carl.d.soren...@gmail.com: On 2010/12/05 00:58:13, Neil Puttock wrote: I don't know, but they're consistently shifted to the right unless there are double-digit notes present. I wanted your opinion on the spacing in the .png I sent. I adjusted the offset, but it wasn't part of this patch. I have deliberately shifted the tab note heads to the right, because they were slightly too far to the left, in my opinion. I don't know why, but in the .png it looks as if they were almost right-aligned to the note heads, but in fact, they are centered. Perhaps the left edge of a number should be aligned to the left edge of a note head when the fret number is a single digit, whereas fret numbers > 9 should be centered according to the center of the left-aligned single digits (I don't know how to describe this in a simpler way...). Regards, Marc ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/12/05 00:58:13, Neil Puttock wrote: I don't know, but they're consistently shifted to the right unless there are double-digit notes present. I wanted your opinion on the spacing in the .png I sent. I adjusted the offset, but it wasn't part of this patch. I have deliberately shifted the tab note heads to the right, because they were slightly too far to the left, in my opinion. I've now put the new offsets in the latest patch sets. Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/12/05 00:58:13, Neil Puttock wrote: I don't know, but they're consistently shifted to the right unless there are double-digit notes present. I wanted your opinion on the spacing in the .png I sent. I adjusted the offset, but it wasn't part of this patch. I have deliberately shifted the tab note heads to the right, because they were slightly too far to the left, in my opinion. I've now put the new offsets in the latest patch sets. Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 5 December 2010 00:35, Carl Sorensen wrote: > How's this? To my eye it appears that the tab heads are centered on the > regular notes now. I don't know, but they're consistently shifted to the right unless there are double-digit notes present. Cheers, Neil <>___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/12/03 16:32:07, Carl wrote: Added an offset to get the column centered one character-width to the right of the paper column. I think it's even better than it used to be. They look too far to the right to me. http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Thanks for the feedback. I've responded to the comments and posted a
new patch set.
http://codereview.appspot.com/2723043/diff/109001/lily/tab-harmonic-engraver.cc
File lily/tab-harmonic-engraver.cc (left):
http://codereview.appspot.com/2723043/diff/109001/lily/tab-harmonic-engraver.cc#oldcode83
lily/tab-harmonic-engraver.cc:83: "HarmonicParenthesesItem ",
On 2010/11/28 22:24:05, Neil Puttock wrote:
This is never created, even for \tabFullNotation.
Deleting the engraver solved this issue
http://codereview.appspot.com/2723043/diff/109001/lily/tab-harmonic-engraver.cc
File lily/tab-harmonic-engraver.cc (right):
http://codereview.appspot.com/2723043/diff/109001/lily/tab-harmonic-engraver.cc#newcode59
lily/tab-harmonic-engraver.cc:59: victim->set_property ("style",
ly_symbol2scm ("harmonic"));
On 2010/11/28 22:24:05, Neil Puttock wrote:
This makes it difficult for users to tweak the appearance of the
harmonic
brackets, though if you still think it's a price worth paying, you
might as well
junk the engraver and read 'articulations directly inside the notehead
callback.
I junked the engraver and added properties to the tab-note-head 'details
for cautionary and harmonic brackets.
http://codereview.appspot.com/2723043/diff/109001/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):
http://codereview.appspot.com/2723043/diff/109001/scm/define-grob-properties.scm#newcode965
scm/define-grob-properties.scm:965: (display-cautionary ,boolean?
"Display as cautionary.")
On 2010/11/28 22:24:05, Neil Puttock wrote:
Needs a more descriptive docstring.
Tried to make it more descriptive, but I didn't want it to be limited to
tab-note-head.
http://codereview.appspot.com/2723043/diff/109001/scm/tablature.scm
File scm/tablature.scm (right):
http://codereview.appspot.com/2723043/diff/109001/scm/tablature.scm#newcode297
scm/tablature.scm:297: (whiteout (ly:grob-property grob 'whiteout #t))
On 2010/11/28 22:24:05, Neil Puttock wrote:
If 'whiteout is set, it's applied to the final stencil in grob.cc.
Removed it from the callback
http://codereview.appspot.com/2723043/diff/109001/scm/tablature.scm#newcode321
scm/tablature.scm:321: (centered-stencil output-grob)))
On 2010/11/28 22:24:05, Neil Puttock wrote:
This has a nasty side-effect: it centres the noteheads on the
PaperColumn,
shifting them to the left of heads in other staves.
Added an offset to get the column centered one character-width to the
right of the paper column. I think it's even better than it used to be.
http://codereview.appspot.com/2723043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/11/28 15:42:47, marc wrote: Just some remarks about the harmonic detection. Thanks, Marc. I wrote a slightly different version of this (using filter). It works great! http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/11/28 22:24:05, Neil Puttock wrote: http://codereview.appspot.com/2723043/diff/109001/scm/tablature.scm#newcode321 scm/tablature.scm:321: (centered-stencil output-grob))) This has a nasty side-effect: it centres the noteheads on the PaperColumn, shifting them to the left of heads in other staves. I've now added code that sets an offset the size of an "8" in the current tab-note-head grob context. This actually lines things up better than it used to be lined up, in my opinion. Is it too much of a hack? http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/11/28 22:24:05, Neil Puttock wrote:
http://codereview.appspot.com/2723043/diff/109001/lily/tab-harmonic-engraver.cc#newcode59
lily/tab-harmonic-engraver.cc:59: victim->set_property ("style",
ly_symbol2scm
("harmonic"));
This makes it difficult for users to tweak the appearance of the
harmonic
brackets, though if you still think it's a price worth paying, you
might as well
junk the engraver and read 'articulations directly inside the notehead
callback.
Ahh, now I understand why there's a HarmonicParenthesisItem. I'm not
sure what I think the best solution is. I'd welcome some discussion on
it.
I think that the harmonic parentheses should become part of the notehead
grob, so that when we parenthesize the notehead it will include the
harmonic parentheses.
I can see that we don't have any way to adjust the properties if it's
the part of the same grob, so we re really would like it to be two
separate grobs.
Is there any way you can think of to combine two grobs into one?
http://codereview.appspot.com/2723043/diff/109001/scm/tablature.scm#newcode321
scm/tablature.scm:321: (centered-stencil output-grob)))
This has a nasty side-effect: it centres the noteheads on the
PaperColumn,
shifting them to the left of heads in other staves.
So what I'd really like to do is center the tab-note-heads on the center
of the notes in the regular music column. I can do that by centering,
then offsetting. But how can I get the amount for the offset?
Thanks,
Carl
http://codereview.appspot.com/2723043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
http://codereview.appspot.com/2723043/diff/109001/lily/tab-harmonic-engraver.cc
File lily/tab-harmonic-engraver.cc (left):
http://codereview.appspot.com/2723043/diff/109001/lily/tab-harmonic-engraver.cc#oldcode83
lily/tab-harmonic-engraver.cc:83: "HarmonicParenthesesItem ",
This is never created, even for \tabFullNotation.
http://codereview.appspot.com/2723043/diff/109001/lily/tab-harmonic-engraver.cc
File lily/tab-harmonic-engraver.cc (right):
http://codereview.appspot.com/2723043/diff/109001/lily/tab-harmonic-engraver.cc#newcode59
lily/tab-harmonic-engraver.cc:59: victim->set_property ("style",
ly_symbol2scm ("harmonic"));
This makes it difficult for users to tweak the appearance of the
harmonic brackets, though if you still think it's a price worth paying,
you might as well junk the engraver and read 'articulations directly
inside the notehead callback.
http://codereview.appspot.com/2723043/diff/109001/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):
http://codereview.appspot.com/2723043/diff/109001/scm/define-grob-properties.scm#newcode965
scm/define-grob-properties.scm:965: (display-cautionary ,boolean?
"Display as cautionary.")
Needs a more descriptive docstring.
http://codereview.appspot.com/2723043/diff/109001/scm/tablature.scm
File scm/tablature.scm (right):
http://codereview.appspot.com/2723043/diff/109001/scm/tablature.scm#newcode297
scm/tablature.scm:297: (whiteout (ly:grob-property grob 'whiteout #t))
If 'whiteout is set, it's applied to the final stencil in grob.cc.
http://codereview.appspot.com/2723043/diff/109001/scm/tablature.scm#newcode321
scm/tablature.scm:321: (centered-stencil output-grob)))
This has a nasty side-effect: it centres the noteheads on the
PaperColumn, shifting them to the left of heads in other staves.
http://codereview.appspot.com/2723043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Just some remarks about the harmonic detection. Regards, Marc http://codereview.appspot.com/2723043/diff/109001/scm/tablature.scm File scm/tablature.scm (right): http://codereview.appspot.com/2723043/diff/109001/scm/tablature.scm#newcode296 scm/tablature.scm:296: (harmonic (eq? (ly:grob-property grob 'style #f) 'harmonic)) I am not sure whether this approach is sane enough; I would at least include mixed-harmonic, too. When I worked on this, I used a different function: (define (is-harmonic? grob) (let ((articulations (ly:event-property (event-cause grob) 'articulations)) (harmonic-found #f)) (for-each (lambda (art) (if (ly:in-event-class? art 'harmonic-event) (set! harmonic-found #t))) articulations) harmonic-found)) This seems to work regardless of the user's choice of harmonic display style. http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Thanks, Marc. That was a mistake I made in resolving merge conflicts. http://codereview.appspot.com/2723043/diff/103001/scm/tablature.scm File scm/tablature.scm (right): http://codereview.appspot.com/2723043/diff/103001/scm/tablature.scm#newcode291 scm/tablature.scm:291: (grob-interpret-markup grob (make-customFretLabel-markup fret))) On 2010/11/27 09:05:58, marc wrote: I think you reverted my corrected patch here, at least partly. customFretLabel-markup isn't defined any more. Oops, I'm sorry. I handled the merge conflict inappropriately. I've now changed it to redefine tab-note-head::print-custom-fret-label so that it sets the 'text property of the grob and then calls tab-note-head::print. http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Hello Carl, your code looks great (at least at a quick glance), but it looks as you didn't rebase after my patch correction concerning the custom fret label was applied. http://codereview.appspot.com/2723043/diff/103001/scm/tablature.scm File scm/tablature.scm (right): http://codereview.appspot.com/2723043/diff/103001/scm/tablature.scm#newcode291 scm/tablature.scm:291: (grob-interpret-markup grob (make-customFretLabel-markup fret))) I think you reverted my corrected patch here, at least partly. customFretLabel-markup isn't defined any more. http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Thanks for the review, Neil. I've responded to all your comments. I've also defined a new print function for TabNoteHeads in Scheme. It will take care of all of the necessary parentheses and harmonic brackets, based on the settings of 'display-cautionary and 'style. Thanks, Carl http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc File lily/tab-tie-follow-engraver.cc (right): http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode52 lily/tab-tie-follow-engraver.cc:52: void process_acknowledged (); On 2010/11/16 23:30:42, Neil Puttock wrote: remove Done. http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode64 lily/tab-tie-follow-engraver.cc:64: ties_.push_back (dynamic_cast (info.grob ())); On 2010/11/16 23:30:42, Neil Puttock wrote: push_back (info.spanner ()) Done. http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode70 lily/tab-tie-follow-engraver.cc:70: glissandi_.push_back (dynamic_cast (info.grob ())); On 2010/11/16 23:30:42, Neil Puttock wrote: push_back (info.spanner ()) Done. Thanks for teaching me about these calls. http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode76 lily/tab-tie-follow-engraver.cc:76: note_heads_.push_back (dynamic_cast (info.grob ())); On 2010/11/16 23:30:42, Neil Puttock wrote: push_back (info.item ()) Done. http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode82 lily/tab-tie-follow-engraver.cc:82: slurs_.push_back (dynamic_cast (info.grob ())); On 2010/11/16 23:30:42, Neil Puttock wrote: push_back (info.spanner ()) Done. http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode86 lily/tab-tie-follow-engraver.cc:86: Tab_tie_follow_engraver::process_acknowledged () On 2010/11/16 23:30:42, Neil Puttock wrote: remove Done. http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode108 lily/tab-tie-follow-engraver.cc:108: Item *slur_cause = dynamic_cast (unsmob_grob (left_cause)); On 2010/11/16 23:30:42, Neil Puttock wrote: unsmob_item (left_cause) Done. http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode121 lily/tab-tie-follow-engraver.cc:121: if ((left_bound == note_heads_[k])) On 2010/11/16 23:30:42, Neil Puttock wrote: remove extra parentheses Done. http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode154 lily/tab-tie-follow-engraver.cc:154: On 2010/11/16 23:30:42, Neil Puttock wrote: gratuitous newline Done. http://codereview.appspot.com/2723043/diff/70001/scm/define-grob-interfaces.scm File scm/define-grob-interfaces.scm (right): http://codereview.appspot.com/2723043/diff/70001/scm/define-grob-interfaces.scm#newcode214 scm/define-grob-interfaces.scm:214: '(details display-cautionary)) On 2010/11/16 23:30:42, Neil Puttock wrote: New structure is span-start and display-cautionary. tie-follow is implied in the callback that is used. display-cautionary is used by glissando callback and in new tab-note-head::print function. http://codereview.appspot.com/2723043/diff/70001/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/2723043/diff/70001/scm/define-grob-properties.scm#newcode1016 scm/define-grob-properties.scm:1016: (span-start ,boolean? "Is the note at the start of a spanner?") On 2010/11/16 23:30:42, Neil Puttock wrote: note head Done. http://codereview.appspot.com/2723043/diff/70001/scm/define-grob-properties.scm#newcode1023 scm/define-grob-properties.scm:1023: (tie-follow ,boolean? "Is the note at the end of a tie?") On 2010/11/16 23:30:42, Neil Puttock wrote: note head Eliminated http://codereview.appspot.com/2723043/diff/70001/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/2723043/diff/70001/scm/define-grobs.scm#newcode822 scm/define-grobs.scm:822: line-interface On 2010/11/16 23:30:42, Neil Puttock wrote: indent I assume you mean to use tabs. Done. http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc File lily/tab-tie-follow-engraver.cc (right): http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode52 lily/tab-tie-follow-engraver.cc:52: void process_acknowledged (); remove http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode64 lily/tab-tie-follow-engraver.cc:64: ties_.push_back (dynamic_cast (info.grob ())); push_back (info.spanner ()) http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode70 lily/tab-tie-follow-engraver.cc:70: glissandi_.push_back (dynamic_cast (info.grob ())); push_back (info.spanner ()) http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode76 lily/tab-tie-follow-engraver.cc:76: note_heads_.push_back (dynamic_cast (info.grob ())); push_back (info.item ()) http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode82 lily/tab-tie-follow-engraver.cc:82: slurs_.push_back (dynamic_cast (info.grob ())); push_back (info.spanner ()) http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode86 lily/tab-tie-follow-engraver.cc:86: Tab_tie_follow_engraver::process_acknowledged () remove http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode108 lily/tab-tie-follow-engraver.cc:108: Item *slur_cause = dynamic_cast (unsmob_grob (left_cause)); unsmob_item (left_cause) http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode121 lily/tab-tie-follow-engraver.cc:121: if ((left_bound == note_heads_[k])) remove extra parentheses http://codereview.appspot.com/2723043/diff/70001/lily/tab-tie-follow-engraver.cc#newcode154 lily/tab-tie-follow-engraver.cc:154: gratuitous newline http://codereview.appspot.com/2723043/diff/70001/scm/define-grob-interfaces.scm File scm/define-grob-interfaces.scm (right): http://codereview.appspot.com/2723043/diff/70001/scm/define-grob-interfaces.scm#newcode214 scm/define-grob-interfaces.scm:214: '(details display-cautionary)) ? tie-follow span-start http://codereview.appspot.com/2723043/diff/70001/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/2723043/diff/70001/scm/define-grob-properties.scm#newcode1016 scm/define-grob-properties.scm:1016: (span-start ,boolean? "Is the note at the start of a spanner?") note head http://codereview.appspot.com/2723043/diff/70001/scm/define-grob-properties.scm#newcode1023 scm/define-grob-properties.scm:1023: (tie-follow ,boolean? "Is the note at the end of a tie?") note head http://codereview.appspot.com/2723043/diff/70001/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/2723043/diff/70001/scm/define-grobs.scm#newcode822 scm/define-grobs.scm:822: line-interface indent http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Carl Sorensen writes: > On 11/13/10 3:23 AM, "David Kastrup" wrote: > >> Carl Sorensen writes: >> >>> On 11/13/10 2:01 AM, "David Kastrup" wrote: >>> carl.d.soren...@gmail.com writes: > >>> You sort the noteheads according to some criterion, and use the same criterion for sorting the other data structures according to their notehead anchors. Then you have three lists, and you work off the notehead list, checking its current head with the other current heads, advancing the other lists when their heads get behind, and treating the association when the heads match. >>> >>> I'd thought about doing this. But I'm not aware of any criteria that >>> one could use for sorting in this particular case. I'm not aware of >>> any index or property the grobs have that is sortable. >> >> With non-compacting garbage collection, memory address of grob is a >> perfectly fine, if arbitrary, criterion. > > Ahh, OK. I see how that might work now. > > Because this is not a simplification, but might be an optimization, > I'll not implement it now. If we see the current code as a big time > sink, and if we see that implementing the sorting will reduce the time > significantly, then I'll consider it. To be honest, I'd consider it saner to create an extra bucket in the grob data structure where one can chain up related grobs like ties and stuff. Then you basically iterate once through the ties and slurs and chain them up in their respective attaching grob, and are done. O(n) at the cost of another data field. But it is not unlikely that other engravers could also make use of something like that. > But I don't think this is a big time sink. I think that enough care > has been taken to keep the loops short through the use of break, and > the vectors are small enough, that the speed should be acceptable. As long as the break is correct and does not cause missing pieces. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Am 13.11.2010 15:29, schrieb Carl Sorensen: On 11/13/10 3:18 AM, "Marc Hohl" wrote: Am 13.11.2010 06:21, schrieb carl.d.soren...@gmail.com: I can think of no way to simplify this code further. If you have any ideas I'd be happy to hear them. There was the idea to include this into the Tab_note_heads_engraver, and if it were possible to include the parenthesize stuff into that engraver as well, it would a) simplify the callbacks b) solve the problem about the harmonic brackets Is it possible to detect the tie/slur/glissando cases within the engraver and call a routine to parenthesize the fret number? Then the resulting grob is a compound object that gets its angle brackets displayed correctly without fiddling with padding and stuff. It may be possible to include the tie/slur/glissando in the tab_note_heads_engraver, but I haven't yet been able to get it to work properly. However, we can't do the whole display-cautionary thing in the engraver, because the engraver doesn't know about such things as split ties. That has to be dealt with in the tab note head callback. Yes, I know, because the line breaking happens at a time when the engravers have already done their job, IIUC. The problem with the harmonic brackets is orthogonal to this problem. I started work on it, but I have to sort out the various parenthesize functions before I know how to fully deal with it. It's on my list, but I want to get this one resolved first. Yes, of course. I am not sure whether it is possible at all to include all the stuff into the tab note head engraver, but I think it would be more homogenic to handle the tab note head appearance by engravers as far as possible without callback patchwork. Marc Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 11/13/10 3:23 AM, "David Kastrup" wrote: > Carl Sorensen writes: > >> On 11/13/10 2:01 AM, "David Kastrup" wrote: >> >>> carl.d.soren...@gmail.com writes: >>> >> >>> You sort the noteheads according to some criterion, and use the same >>> criterion for sorting the other data structures according to their >>> notehead anchors. Then you have three lists, and you work off the >>> notehead list, checking its current head with the other current heads, >>> advancing the other lists when their heads get behind, and treating the >>> association when the heads match. >> >> I'd thought about doing this. But I'm not aware of any criteria that >> one could use for sorting in this particular case. I'm not aware of >> any index or property the grobs have that is sortable. > > With non-compacting garbage collection, memory address of grob is a > perfectly fine, if arbitrary, criterion. Ahh, OK. I see how that might work now. Because this is not a simplification, but might be an optimization, I'll not implement it now. If we see the current code as a big time sink, and if we see that implementing the sorting will reduce the time significantly, then I'll consider it. But I don't think this is a big time sink. I think that enough care has been taken to keep the loops short through the use of break, and the vectors are small enough, that the speed should be acceptable. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 11/13/10 3:18 AM, "Marc Hohl" wrote: > Am 13.11.2010 06:21, schrieb carl.d.soren...@gmail.com: >> I can think of no way to simplify this code further. If you have any >> ideas I'd be happy to hear them. > There was the idea to include this into the Tab_note_heads_engraver, and > if it were > possible to include the parenthesize stuff into that engraver as well, > it would > > a) simplify the callbacks > b) solve the problem about the harmonic brackets > > Is it possible to detect the tie/slur/glissando cases within the > engraver and call a > routine to parenthesize the fret number? Then the resulting grob is a > compound object that > gets its angle brackets displayed correctly without fiddling with > padding and stuff. It may be possible to include the tie/slur/glissando in the tab_note_heads_engraver, but I haven't yet been able to get it to work properly. However, we can't do the whole display-cautionary thing in the engraver, because the engraver doesn't know about such things as split ties. That has to be dealt with in the tab note head callback. The problem with the harmonic brackets is orthogonal to this problem. I started work on it, but I have to sort out the various parenthesize functions before I know how to fully deal with it. It's on my list, but I want to get this one resolved first. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Am 13.11.2010 06:21, schrieb carl.d.soren...@gmail.com: Thanks for the help on the null pointer. I was thinking that it was some *other* kind of variable than a Grob, and that I was casting it wrong. Got that all taken care of -- no scheme calls at all. On 2010/11/12 17:40:42, Neil Puttock wrote: It's back to square one though, isn't it? The triple nested loop is horrible, and should be avoided at all costs; this is partly why I suggested only acknowledging the ties and heads. OK, so now I've eliminated the triple nested loop. There is what appears to me to be a required nested loop. One loop to loop through the note-heads. Then an inner loop (with a break) through the ties looking for a tie on a note head. Followed by a second inner loop (with a break) through the slurs looking for a slur on the note head. Followed by a third inner loop (if we didn't find a slur) through the glissandos looking for a glissando on the note head. I can think of no way to simplify this code further. If you have any ideas I'd be happy to hear them. There was the idea to include this into the Tab_note_heads_engraver, and if it were possible to include the parenthesize stuff into that engraver as well, it would a) simplify the callbacks b) solve the problem about the harmonic brackets Is it possible to detect the tie/slur/glissando cases within the engraver and call a routine to parenthesize the fret number? Then the resulting grob is a compound object that gets its angle brackets displayed correctly without fiddling with padding and stuff. Marc Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Carl Sorensen writes: > On 11/13/10 2:01 AM, "David Kastrup" wrote: > >> carl.d.soren...@gmail.com writes: >> >>> OK, so now I've eliminated the triple nested loop. >>> >>> There is what appears to me to be a required nested loop. >>> >>> One loop to loop through the note-heads. >>> Then an inner loop (with a break) through the ties looking for a tie on >>> a note head. >>> Followed by a second inner loop (with a break) through the slurs looking >>> for a slur on the note head. >>> Followed by a third inner loop (if we didn't find a slur) through the >>> glissandos looking for a glissando on the note head. >>> >>> I can think of no way to simplify this code further. If you have any >>> ideas I'd be happy to hear them. >> >> It's not a simplification, but the usual manner to find multiple >> associations and deal with them is sorting. > > Thanks for the idea. > >> You sort the noteheads according to some criterion, and use the same >> criterion for sorting the other data structures according to their >> notehead anchors. Then you have three lists, and you work off the >> notehead list, checking its current head with the other current heads, >> advancing the other lists when their heads get behind, and treating the >> association when the heads match. > > I'd thought about doing this. But I'm not aware of any criteria that > one could use for sorting in this particular case. I'm not aware of > any index or property the grobs have that is sortable. With non-compacting garbage collection, memory address of grob is a perfectly fine, if arbitrary, criterion. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 11/13/10 2:01 AM, "David Kastrup" wrote: > carl.d.soren...@gmail.com writes: > >> OK, so now I've eliminated the triple nested loop. >> >> There is what appears to me to be a required nested loop. >> >> One loop to loop through the note-heads. >> Then an inner loop (with a break) through the ties looking for a tie on >> a note head. >> Followed by a second inner loop (with a break) through the slurs looking >> for a slur on the note head. >> Followed by a third inner loop (if we didn't find a slur) through the >> glissandos looking for a glissando on the note head. >> >> I can think of no way to simplify this code further. If you have any >> ideas I'd be happy to hear them. > > It's not a simplification, but the usual manner to find multiple > associations and deal with them is sorting. > Thanks for the idea. > You sort the noteheads according to some criterion, and use the same > criterion for sorting the other data structures according to their > notehead anchors. Then you have three lists, and you work off the > notehead list, checking its current head with the other current heads, > advancing the other lists when their heads get behind, and treating the > association when the heads match. I'd thought about doing this. But I'm not aware of any criteria that one could use for sorting in this particular case. I'm not aware of any index or property the grobs have that is sortable. > > Of course, this only makes sense when more than one list has more than > one member (more finicky, if N is the length of the largest list, the > second largest list should have significantly more than lg N members). > Otherwise, the brute-force method will tend to be faster. I think that in this case, the list lengths are often 1. They're rarely larger than 3. It seems that in such a case, brute force performance is fine. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
carl.d.soren...@gmail.com writes: > OK, so now I've eliminated the triple nested loop. > > There is what appears to me to be a required nested loop. > > One loop to loop through the note-heads. > Then an inner loop (with a break) through the ties looking for a tie on > a note head. > Followed by a second inner loop (with a break) through the slurs looking > for a slur on the note head. > Followed by a third inner loop (if we didn't find a slur) through the > glissandos looking for a glissando on the note head. > > I can think of no way to simplify this code further. If you have any > ideas I'd be happy to hear them. It's not a simplification, but the usual manner to find multiple associations and deal with them is sorting. You sort the noteheads according to some criterion, and use the same criterion for sorting the other data structures according to their notehead anchors. Then you have three lists, and you work off the notehead list, checking its current head with the other current heads, advancing the other lists when their heads get behind, and treating the association when the heads match. Of course, this only makes sense when more than one list has more than one member (more finicky, if N is the length of the largest list, the second largest list should have significantly more than lg N members). Otherwise, the brute-force method will tend to be faster. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Thanks for the help on the null pointer. I was thinking that it was some *other* kind of variable than a Grob, and that I was casting it wrong. Got that all taken care of -- no scheme calls at all. On 2010/11/12 17:40:42, Neil Puttock wrote: It's back to square one though, isn't it? The triple nested loop is horrible, and should be avoided at all costs; this is partly why I suggested only acknowledging the ties and heads. OK, so now I've eliminated the triple nested loop. There is what appears to me to be a required nested loop. One loop to loop through the note-heads. Then an inner loop (with a break) through the ties looking for a tie on a note head. Followed by a second inner loop (with a break) through the slurs looking for a slur on the note head. Followed by a third inner loop (if we didn't find a slur) through the glissandos looking for a glissando on the note head. I can think of no way to simplify this code further. If you have any ideas I'd be happy to hear them. Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/11/12 04:12:48, Carl wrote: Here's a new version of the patch that tries to eliminate any scheme calls, and demonstrates my problem. I need to do a check on the left bound to see if it's a grob, because if I take the check out I get a Bus error. I don't know how to do such a check from C++ -- I think it's done by casting, but I'm not sure. Of course you know. It's a pointer, so you check whether it's NULL. if (left_bound) If I self-scm() the C++ Item * variable, then use the scheme routine ly:grob? to check if it's a grob, if I get a Bus error. The slur appears to be acknowledged twice, and the first time it has no left bound (which implies it would be more efficient and safer to do this checking in stop_translation_timestep ()) As it stands, the code works on my machine. It's back to square one though, isn't it? The triple nested loop is horrible, and should be avoided at all costs; this is partly why I suggested only acknowledging the ties and heads. Cheers, Neil http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Am 12.11.2010 05:07, schrieb carl.d.soren...@gmail.com: On 2010/11/07 09:21:45, marc wrote: http://codereview.appspot.com/2723043/diff/57001/scm/define-grob-properties.scm#newcode1021 scm/define-grob-properties.scm:1021: (display-cautionary ,boolean? "Display in cautionary form.") Hey, good choice - I like the name of the new property, it's much better than tie-follow. Thanks. I hoped you wouldn't mind. http://codereview.appspot.com/2723043/diff/57001/scm/tablature.scm#newcode267 scm/tablature.scm:267: ;;(and tie-follow this should be 'cautionary', IIUC. This is commented out and will go away once I'm sure everything is working properly. Ah, I overlooked the ;; - and of course, since the engraver has done its work before the callbacks take place, there is no need for the slur callback to check the 'cautionary property, and the same goes for the glissandos (glissandi? whatever...) Now IUC ;-) Thanks for your work, Marc http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Here's a new version of the patch that tries to eliminate any scheme calls, and demonstrates my problem. I need to do a check on the left bound to see if it's a grob, because if I take the check out I get a Bus error. I don't know how to do such a check from C++ -- I think it's done by casting, but I'm not sure. If I self-scm() the C++ Item * variable, then use the scheme routine ly:grob? to check if it's a grob, if I get a Bus error. Neil, can you give me any insight here? I'd like to use all C++ variables, and eliminate the two remaining scheme calls. As it stands, the code works on my machine. Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/11/07 09:24:31, marc wrote: Am 07.11.2010 08:47, schrieb carl.d.soren...@gmail.com: > In this patch, the glissando, slur, tie, and repeat-tie callbacks have > been modified. The slur callback doesn't even check for the tie status, > since there's no change. But it has to take the value of 'display-cautionary into account, isn't it? As far as I could see from the existing code, there was no difference for the slur based on the style of the note. The slur goes from the top center of the note, which is the same in both cases, I think. > I think that this is the right logic. Because the tie can be split, the > tie (and repeat-tie) callbacks need to decide on the note head display. > The slur and glissando callbacks don't need to set the display, they > just respond to it if need be. +1 Thanks for the confirmation. Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/11/07 09:21:45, marc wrote: http://codereview.appspot.com/2723043/diff/57001/scm/define-grob-properties.scm#newcode1021 scm/define-grob-properties.scm:1021: (display-cautionary ,boolean? "Display in cautionary form.") Hey, good choice - I like the name of the new property, it's much better than tie-follow. Thanks. I hoped you wouldn't mind. http://codereview.appspot.com/2723043/diff/57001/scm/tablature.scm#newcode267 scm/tablature.scm:267: ;;(and tie-follow this should be 'cautionary', IIUC. This is commented out and will go away once I'm sure everything is working properly. http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Am 07.11.2010 08:47, schrieb carl.d.soren...@gmail.com: This patch has revised the callbacks in scm/tablature.scm. It appears to work properly. The tab-tie-follow-engraver is still using SCM calls to analyze the grobs. I will get that worked out soon, I hope (although I seem to have gone backward, not forward, today). In this patch, the glissando, slur, tie, and repeat-tie callbacks have been modified. The slur callback doesn't even check for the tie status, since there's no change. But it has to take the value of 'display-cautionary into account, isn't it? The glissando callback checks to see if the notehead has been forced to display with parentheses, but it does not set the notehead display format. The tie and repeat-tie callbacks are responsible for setting the notehead display. I think that this is the right logic. Because the tie can be split, the tie (and repeat-tie) callbacks need to decide on the note head display. The slur and glissando callbacks don't need to set the display, they just respond to it if need be. +1 Marc Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Hello Carl, I didn't test the patch, but the general logic behind it seems to be the right way to do. Regards, Marc http://codereview.appspot.com/2723043/diff/57001/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/2723043/diff/57001/scm/define-grob-properties.scm#newcode1021 scm/define-grob-properties.scm:1021: (display-cautionary ,boolean? "Display in cautionary form.") Hey, good choice - I like the name of the new property, it's much better than tie-follow. http://codereview.appspot.com/2723043/diff/57001/scm/tablature.scm File scm/tablature.scm (right): http://codereview.appspot.com/2723043/diff/57001/scm/tablature.scm#newcode267 scm/tablature.scm:267: ;;(and tie-follow this should be 'cautionary', IIUC. http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
This patch has revised the callbacks in scm/tablature.scm. It appears to work properly. The tab-tie-follow-engraver is still using SCM calls to analyze the grobs. I will get that worked out soon, I hope (although I seem to have gone backward, not forward, today). In this patch, the glissando, slur, tie, and repeat-tie callbacks have been modified. The slur callback doesn't even check for the tie status, since there's no change. The glissando callback checks to see if the notehead has been forced to display with parentheses, but it does not set the notehead display format. The tie and repeat-tie callbacks are responsible for setting the notehead display. I think that this is the right logic. Because the tie can be split, the tie (and repeat-tie) callbacks need to decide on the note head display. The slur and glissando callbacks don't need to set the display, they just respond to it if need be. Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/11/04 08:31:30, marc wrote:
No, I don't think we should it do more complicated that necessary.
Perhaps the name 'tie-follow is misleading, but the engraver (before
you
left out the slur and glissando bits) did the right job - marking
exactly
the tab-note-heads that have to be treated specially.
If we mark *every* tied-to note, we have to mark *every* start of a
slur
and *every* start
of a glissando as well and check for the appearance of (and
('tie-follow
(or ( 'slur-start 'gliss-start,
which is overkill - just let the engraver take the decision, raise a
flag, and the callbacks do their job.
But right now, the callbacks are fighting over the notes -- and I don't
think that's right. In order to work correctly, we need to know the
order in which the callbacks are called.
I've got an algorithm that I think is clearer and simplifies the
callbacks, but I haven't been able to fully test it yet because I can't
get the C++ engraver to work right in terms of checking equality.
I'll post a patch for comments.
Thanks,
Carl
http://codereview.appspot.com/2723043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Am 03.11.2010 20:33, schrieb Carl Sorensen:
On 11/3/10 10:15 AM, "Marc Hohl" wrote:
Am 03.11.2010 15:10, schrieb Carl Sorensen:
On 11/3/10 6:50 AM, "Marc Hohl" wrote:
Am 02.11.2010 04:04, schrieb carl.d.soren...@gmail.com:
Updated to only acknowledge tab-note-head, not note-head.
Makes perfect sense to me.
Thanks for your work!
You're welcome.
Pushed to git.
Oops, I found an error!
While my engraver added 'tie-follow only to the notes followed by a slur
or a glissando,
your engraver adds this to *every* tied note, so the callbacks in
tablature.scm are
not working properly - *every* note that is 'tied to' gets parenthesized!
Moreover, Neil's argument that slurs and glissandos do not belong to the
engraver
seems not to be valid - every note that is 'tied to' now gets the
'tie-follow mark, so the
tie handler will make them invisible and the slur routine that may
follow changes the visibility
again, and that's exactly what Neil critiziced at my first approach (and
he was right about that).
So perhaps what we need are 'tie-follow, 'slur-start, and 'gliss-start
properties for a tab note head.
No, I don't think we should it do more complicated that necessary.
Perhaps the name 'tie-follow is misleading, but the engraver (before you
left out the slur and glissando bits) did the right job - marking exactly
the tab-note-heads that have to be treated specially.
If we mark *every* tied-to note, we have to mark *every* start of a slur
and *every* start
of a glissando as well and check for the appearance of (and ('tie-follow
(or ( 'slur-start 'gliss-start,
which is overkill - just let the engraver take the decision, raise a
flag, and the callbacks do their job.
Marc
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Am 03.11.2010 21:22, schrieb Neil Puttock: On 3 November 2010 19:33, Carl Sorensen wrote: But the tie callback *should* make the notehead transparent if there's no slur or gliss (or bend, in the future). In the absence of slur, gliss, or split tie the notehead is transparent. In the presence of one of these, it's visible and parenthesized. So how would one achieve this behavior without the tie callback making the notehead transparent? The slur and Glissando callbacks should make the notehead visible again (this isn't ideal, but I can't see any other way of doing it without making the code more complex). Why can't we use the mechanism we already had instead? The engraver sets tie-follow *only* for tab-note-heads which are right bounds of a tie *and* left bounds of either a slur or a glissando - then the callback code works properly, as far as I can see. Marc Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 11/3/10 5:52 PM, "n.putt...@gmail.com" wrote:
> On 2010/11/03 23:14:47, Carl wrote:
>
>> I think I'd probably go a bit farther.
>
>> What if we acknowledged ties, slurs, and glissandos (glissandi?) in
> the
>> Tab_note_head_engraver and set the desired properties there.
>
> Which properties?
Well, it would seem that in order to know how to display a tab note head
properly we need to know the following:
1. Is it on the right hand end of a tie ('tie-follow property)
2. Is it on the left hand end of a slur, glissando, or bend ('begin-spanner
property?)
>
> If it involves multiple nested loops (like the original patch), then I'd
> say no.
I think we could avoid multiple nested loops, but there would be loops:
First of all, instead of having slurs_, and glissandos_, we'd have
spanners_, and all of the spanner grobs (slur, glissando, bend) would be
pushed to the same vector
In pseudocode
For each tab-note-head
for each tie
if right-bound of tie is tab-note-head
set 'tie-follow
break
end if
end for
for each spanner
if left-bound of spanner is tab-note-head
set 'begin-spanner
break
end if
end for
end for
>
> How would you deal with TabNoteHeads after a line break? IIRC, there is
> some code in tablature.scm which treats them differently, but this is
> something you can't do in an engraver.
Right -- that would be part of the TabNoteHead callback -- it would look at
the current status of the notehead to see if it's after a break, and make
the appropriate decisions based on the 'tie-follow and 'begin-spanner
properties.
Thanks,
Carl
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/11/03 20:46:27, Neil Puttock wrote: Hi Carl, What do you think about folding this code into the Tab_note_heads_engraver? We could store the created TabNoteHead grobs and add an acknowledger for Tie, then set 'tie-follow in stop_translation_timestep (). I think I'd probably go a bit farther. What if we acknowledged ties, slurs, and glissandos (glissandi?) in the Tab_note_head_engraver and set the desired properties there. And then we eliminated all of the notehead handling from the slur callback, the tie callback, and the glissando callback. That way the notehead callback could take care of the notehead, and we wouldn't need to mix things at all. What do you think? Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/11/03 23:14:47, Carl wrote: I think I'd probably go a bit farther. What if we acknowledged ties, slurs, and glissandos (glissandi?) in the Tab_note_head_engraver and set the desired properties there. Which properties? If it involves multiple nested loops (like the original patch), then I'd say no. How would you deal with TabNoteHeads after a line break? IIRC, there is some code in tablature.scm which treats them differently, but this is something you can't do in an engraver. Cheers, Neil http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Hi Carl,
What do you think about folding this code into the
Tab_note_heads_engraver? We could store the created TabNoteHead grobs
and add an acknowledger for Tie, then set 'tie-follow in
stop_translation_timestep ().
Cheers,
Neil
http://codereview.appspot.com/2723043/diff/31001/lily/tab-tie-follow-engraver.cc
File lily/tab-tie-follow-engraver.cc (right):
http://codereview.appspot.com/2723043/diff/31001/lily/tab-tie-follow-engraver.cc#newcode72
lily/tab-tie-follow-engraver.cc:72: SCM proc = ly_lily_module_constant
("ly:spanner-bound");
remove
http://codereview.appspot.com/2723043/diff/31001/lily/tab-tie-follow-engraver.cc#newcode75
lily/tab-tie-follow-engraver.cc:75: SCM right_bound =
ties_[1]->get_bound (LEFT);
Item *right_bound = ties_[i]->get_bound (RIGHT);
(will require casting or declaring ties_ as vector)
http://codereview.appspot.com/2723043/diff/31001/lily/tab-tie-follow-engraver.cc#newcode78
lily/tab-tie-follow-engraver.cc:78: if (right_bound ==
note_heads_[k]->self_scm ())
if (right_bound = note_heads_[k])
(again, casting required, or declare note_heads_ as vector)
http://codereview.appspot.com/2723043/diff/31001/scm/tablature.scm
File scm/tablature.scm (right):
http://codereview.appspot.com/2723043/diff/31001/scm/tablature.scm#newcode187
scm/tablature.scm:187: (if tie-follow
the tie callback shouldn't care about 'tie-follow, so this extra code
should be removed
http://codereview.appspot.com/2723043/diff/31001/scm/tablature.scm#newcode223
scm/tablature.scm:223: (if tie-follow
see comment for tie::handle-tab-note-head
http://codereview.appspot.com/2723043/diff/31001/scm/tablature.scm#newcode266
scm/tablature.scm:266: (ly:grob-set-property! left-tab-note-head
'stencil
add
(set! (ly:grob-property tied-tab-note-head 'transparent) #t)
http://codereview.appspot.com/2723043/diff/31001/scm/tablature.scm#newcode279
scm/tablature.scm:279: (ly:grob-set-property! left-tab-note-head
'stencil
add
(set! (ly:grob-property tied-tab-note-head 'transparent) #t)
http://codereview.appspot.com/2723043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 3 November 2010 19:33, Carl Sorensen wrote: > But the tie callback *should* make the notehead transparent if there's no > slur or gliss (or bend, in the future). In the absence of slur, gliss, or > split tie the notehead is transparent. In the presence of one of these, > it's visible and parenthesized. > > So how would one achieve this behavior without the tie callback making the > notehead transparent? The slur and Glissando callbacks should make the notehead visible again (this isn't ideal, but I can't see any other way of doing it without making the code more complex). Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 11/3/10 10:15 AM, "Marc Hohl" wrote: > Am 03.11.2010 15:10, schrieb Carl Sorensen: >> >> >> On 11/3/10 6:50 AM, "Marc Hohl" wrote: >> >> >>> Am 02.11.2010 04:04, schrieb carl.d.soren...@gmail.com: >>> Updated to only acknowledge tab-note-head, not note-head. >>> Makes perfect sense to me. >>> Thanks for your work! >>> >> You're welcome. >> >> Pushed to git. >> > Oops, I found an error! > > While my engraver added 'tie-follow only to the notes followed by a slur > or a glissando, > your engraver adds this to *every* tied note, so the callbacks in > tablature.scm are > not working properly - *every* note that is 'tied to' gets parenthesized! > > Moreover, Neil's argument that slurs and glissandos do not belong to the > engraver > seems not to be valid - every note that is 'tied to' now gets the > 'tie-follow mark, so the > tie handler will make them invisible and the slur routine that may > follow changes the visibility > again, and that's exactly what Neil critiziced at my first approach (and > he was right about that). So perhaps what we need are 'tie-follow, 'slur-start, and 'gliss-start properties for a tab note head. Or perhaps we need to change the name of the property from tie-follow to show-hidden. And the show-hidden property gets set only if it's tie-follow and slur-start, or tie-follow and gliss-start, or tie-follow and bend-start (in the future). Under this proposal, we'd change the engraver name to something like tab_tied_head_engraver. It wouldn't take too much to do this, but is it the *right* approach to solve this problem? Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 11/3/10 12:43 PM, "Neil Puttock" wrote: > On 3 November 2010 16:15, Marc Hohl wrote: >> Am 03.11.2010 15:10, schrieb Carl Sorensen: >>> >>> >>> On 11/3/10 6:50 AM, "Marc Hohl" wrote: >>> >>> Am 02.11.2010 04:04, schrieb carl.d.soren...@gmail.com: > > Updated to only acknowledge tab-note-head, not note-head. > Makes perfect sense to me. Thanks for your work! >>> >>> You're welcome. >>> >>> Pushed to git. >>> >> >> Oops, I found an error! >> >> While my engraver added 'tie-follow only to the notes followed by a slur or >> a glissando, >> your engraver adds this to *every* tied note, so the callbacks in >> tablature.scm are >> not working properly - *every* note that is 'tied to' gets parenthesized! > > It's a shame Carl's already pushed the patch; I don't think it's ready yet. I'm sorry. I'll revert it. > > The code for checking bounds is wrong: as I noted for the original > patchset, get_bound () is the correct method, rather than jumping into > scheme (ly:spanner-bound). Fixed. > >> Moreover, Neil's argument that slurs and glissandos do not belong to the >> engraver >> seems not to be valid - every note that is 'tied to' now gets the >> 'tie-follow mark, so the >> tie handler will make them invisible and the slur routine that may follow >> changes the visibility >> again, and that's exactly what Neil critiziced at my first approach (and he >> was right about that). > > That's because the tie callback is interfering with the slur and > glissando callbacks: you need to stop it making the noteheads > transparent. > But the tie callback *should* make the notehead transparent if there's no slur or gliss (or bend, in the future). In the absence of slur, gliss, or split tie the notehead is transparent. In the presence of one of these, it's visible and parenthesized. So how would one achieve this behavior without the tie callback making the notehead transparent? Thanks, Carl > Cheers, > Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On Wed, Nov 3, 2010 at 7:43 PM, Neil Puttock wrote: > It's a shame Carl's already pushed the patch; I don't think it's ready yet. Nah, I think Carl pushed his patch too early just so I don't feel alone anymore :) Valentin. ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 3 November 2010 16:15, Marc Hohl wrote: > Am 03.11.2010 15:10, schrieb Carl Sorensen: >> >> >> On 11/3/10 6:50 AM, "Marc Hohl" wrote: >> >> >>> >>> Am 02.11.2010 04:04, schrieb carl.d.soren...@gmail.com: >>> Updated to only acknowledge tab-note-head, not note-head. >>> >>> Makes perfect sense to me. >>> Thanks for your work! >>> >> >> You're welcome. >> >> Pushed to git. >> > > Oops, I found an error! > > While my engraver added 'tie-follow only to the notes followed by a slur or > a glissando, > your engraver adds this to *every* tied note, so the callbacks in > tablature.scm are > not working properly - *every* note that is 'tied to' gets parenthesized! It's a shame Carl's already pushed the patch; I don't think it's ready yet. The code for checking bounds is wrong: as I noted for the original patchset, get_bound () is the correct method, rather than jumping into scheme (ly:spanner-bound). > Moreover, Neil's argument that slurs and glissandos do not belong to the > engraver > seems not to be valid - every note that is 'tied to' now gets the > 'tie-follow mark, so the > tie handler will make them invisible and the slur routine that may follow > changes the visibility > again, and that's exactly what Neil critiziced at my first approach (and he > was right about that). That's because the tie callback is interfering with the slur and glissando callbacks: you need to stop it making the noteheads transparent. Cheers, Neil ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Am 03.11.2010 15:10, schrieb Carl Sorensen: On 11/3/10 6:50 AM, "Marc Hohl" wrote: Am 02.11.2010 04:04, schrieb carl.d.soren...@gmail.com: Updated to only acknowledge tab-note-head, not note-head. Makes perfect sense to me. Thanks for your work! You're welcome. Pushed to git. Oops, I found an error! While my engraver added 'tie-follow only to the notes followed by a slur or a glissando, your engraver adds this to *every* tied note, so the callbacks in tablature.scm are not working properly - *every* note that is 'tied to' gets parenthesized! Moreover, Neil's argument that slurs and glissandos do not belong to the engraver seems not to be valid - every note that is 'tied to' now gets the 'tie-follow mark, so the tie handler will make them invisible and the slur routine that may follow changes the visibility again, and that's exactly what Neil critiziced at my first approach (and he was right about that). So, IIUC, you'll have to use the version of your engraver with the slur and glissando stuff included. Sorry for understanding this so late, Marc Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 11/3/10 6:50 AM, "Marc Hohl" wrote: > Am 02.11.2010 04:04, schrieb carl.d.soren...@gmail.com: >> Updated to only acknowledge tab-note-head, not note-head. > Makes perfect sense to me. > Thanks for your work! You're welcome. Pushed to git. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Am 02.11.2010 04:04, schrieb carl.d.soren...@gmail.com: Updated to only acknowledge tab-note-head, not note-head. Makes perfect sense to me. Thanks for your work! Marc Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Updated to only acknowledge tab-note-head, not note-head. Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 11/1/10 1:53 AM, "Marc Hohl" wrote: > > Sorry for causing so much trouble. > No trouble, Marc. You never need to apologize for working to improve LilyPond! Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Am 31.10.2010 23:24, schrieb n.putt...@gmail.com: Hi Carl, This is too complicated (though that's really a criticism of Marc's Scheme engraver). Thank you, Neil, for pointing this out! I was referring to your engraver example which checked for slurs, too, and I overlooked the fact that the slur callback just does the right thing when tie-follow is set. Sorry for causing so much trouble. Regards, Marc ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Or perhaps the engraver should only listen to tab-note-heads, instead of to note-heads, since the tie-follow property is part of the tab-note-head interface. Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Further thought about this causes me to think the engraver should just be Tie_follow_engraver It doesn't change a TabNoteHead property, it changes a NoteHead property. The only reason it applies to TabNoteHeads is because it is in a TabVoice context. Am I wrong in thinking this? Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
New patch set with simplified engraver -- it only acknowledges ties and note-heads, and it still works. Thanks, Neil! Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/10/31 22:40:57, Carl wrote: This is going away, so it won't apply to this patch (because we don't need to acknowledge glissandos). But if we did, and we added a glissando-interface, then instead of Tab_tie_follow_engraver::acknowledge_line_spanner wouldn't we just use Tab_tie_follow_engraver::acknowledge_glissando? Yes. But it would seem strange to me to add an interface with no properties that can be set, which is what I think I'd be doing if I added a glissando_interface. Any comment on this thought? If you look in define-grob-interfaces.scm, you'll see several interfaces with no properties: most of them exist just to allow engravers to distinguish between grobs which would normally be lumped together (e.g., acknowledging line-spanner-interface, when you really want something more specific). Cheers, Neil http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/10/31 22:24:17, Neil Puttock wrote:
http://codereview.appspot.com/2723043/diff/7001/lily/tab-tie-follow-engraver.cc#newcode69
lily/tab-tie-follow-engraver.cc:69: if (info.grob ()->name () ==
"Glissando")
If you needed to distinguish glissandos from other lines, it would be
more
idiomatic to add an interface (glissando-interface), then use
info.grob->internal_has_interface (ly_symbol2scm
("glissando-interface"))
This is going away, so it won't apply to this patch (because we don't
need to acknowledge glissandos). But if we did, and we added a
glissando-interface, then instead of
Tab_tie_follow_engraver::acknowledge_line_spanner wouldn't we just use
Tab_tie_follow_engraver::acknowledge_glissando?
But it would seem strange to me to add an interface with no properties
that can be set,
which is what I think I'd be doing if I added a glissando_interface.
Any comment on this thought?
Thanks,
Carl
http://codereview.appspot.com/2723043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/10/31 22:24:17, Neil Puttock wrote: Hi Carl, This is too complicated (though that's really a criticism of Marc's Scheme engraver). The point of using 'tie-follow is that it defers the decision to parenthesize TabNoteHead to the point where it matters: in the callbacks for Glissando and Slur. Thus there should be no need to acknowledge glissandos and slurs in the engraver: we only need to check whether the tie's right bound is one of the acknowledged noteheads. Ah, yes. I hadn't thought through the logic carefully; I was just implementing Marc's logic in C++. Glissandos and slurs are irrelevant for determining tied-to. A note is tied-to if it's at the right hand end of a tie. Whether it's parenthesized or not is determined by the presence of a split tie, a glissando, or a slur. So we don't need to acknowledge glissando or slur. Just tie and note_head. Got it! Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Hi Carl,
This is too complicated (though that's really a criticism of Marc's
Scheme engraver).
The point of using 'tie-follow is that it defers the decision to
parenthesize TabNoteHead to the point where it matters: in the callbacks
for Glissando and Slur. Thus there should be no need to acknowledge
glissandos and slurs in the engraver: we only need to check whether the
tie's right bound is one of the acknowledged noteheads.
Cheers,
Neil
http://codereview.appspot.com/2723043/diff/7001/lily/tab-tie-follow-engraver.cc
File lily/tab-tie-follow-engraver.cc (right):
http://codereview.appspot.com/2723043/diff/7001/lily/tab-tie-follow-engraver.cc#newcode69
lily/tab-tie-follow-engraver.cc:69: if (info.grob ()->name () ==
"Glissando")
If you needed to distinguish glissandos from other lines, it would be
more idiomatic to add an interface (glissando-interface), then use
info.grob->internal_has_interface (ly_symbol2scm
("glissando-interface"))
http://codereview.appspot.com/2723043/diff/7001/lily/tab-tie-follow-engraver.cc#newcode98
lily/tab-tie-follow-engraver.cc:98: scm_from_int (LEFT));
slurs_[j]->get_bound (LEFT)
http://codereview.appspot.com/2723043/diff/7001/lily/tab-tie-follow-engraver.cc#newcode99
lily/tab-tie-follow-engraver.cc:99: if (scm_call_1
(ly_lily_module_constant ("ly:grob?"), left_bound) == SCM_BOOL_T)
I'm not sure this serves any useful purpose; unless there's something
seriously wrong, all slur bounds are grobs (of class Item).
http://codereview.appspot.com/2723043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
I've updated the comments and the doc string, and added a check for Glissando. Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
On 2010/10/31 09:29:29, Trevor Daniels wrote: I may be missing something, but doesn't this assume all line spanners are glissandi? I thought the same thing. I haven't investigated it carefully; I was just translating Marc's Scheme engraver to C++. Any comments, Mark? Thanks, Carl http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
I may be missing something, but doesn't this assume all line spanners are glissandi? Trevor http://codereview.appspot.com/2723043/diff/1/lily/tab-tie-follow-engraver.cc File lily/tab-tie-follow-engraver.cc (right): http://codereview.appspot.com/2723043/diff/1/lily/tab-tie-follow-engraver.cc#newcode69 lily/tab-tie-follow-engraver.cc:69: glissandi_.push_back (info.grob ()); Does this not catch line spanners other than glissandi?? http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add tab-tie-follow-engraver (issue2723043)
Hello Carl, wow, you were fast ... LGTM ;-) As said before, I wouldn't be able to do this engraver myself, but I think I understand now a tiny bit better how c++ engraver work. Thank you, Marc http://codereview.appspot.com/2723043/diff/1/lily/tab-tie-follow-engraver.cc File lily/tab-tie-follow-engraver.cc (right): http://codereview.appspot.com/2723043/diff/1/lily/tab-tie-follow-engraver.cc#newcode34 lily/tab-tie-follow-engraver.cc:34: are present. IMHO, the description should be more detailed, e.g. Change the tab-note-head properties when a tie is followed by a slur or a glissando. http://codereview.appspot.com/2723043/diff/1/lily/tab-tie-follow-engraver.cc#newcode145 lily/tab-tie-follow-engraver.cc:145: "Adjust tabNoteHead properties when accuring with ties," uppercase: TabNoteHead http://codereview.appspot.com/2723043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
