Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(305)

Issue 2723043: Add tab-tie-follow-engraver (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 7 months ago by Carl
Modified:
8 years, 6 months ago
Reviewers:
marc, Neil Puttock, carl.d.sorensen, Trevor Daniels, c_sorensen
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Add tab-tie-follow-engraver Based on the Scheme engraver in Marc Hohl's pack

Patch Set 1 #

Total comments: 3

Patch Set 2 : Act only on Glissando line spanners #

Total comments: 3

Patch Set 3 : Simplify engraver (no slur or glissando) #

Patch Set 4 : Change to only listen to tab-note-head events #

Patch Set 5 : Get tie bounds directly in C++ #

Total comments: 7

Patch Set 6 : Remove SCM from all but slurs #

Total comments: 2

Patch Set 7 : Removed all possible scheme calls -- can't figure out one C++ call #

Patch Set 8 : Switch from one property to two; decouple loops #

Total comments: 26

Patch Set 9 : Respond to comments, new print function #

Total comments: 2

Patch Set 10 : Redefine tab-note-head::print-custom-fret-label #

Total comments: 11

Patch Set 11 : Add offset to tab-note-head, delete unnecessary whiteout #

Patch Set 12 : Remove tab-harmonic-engraver #

Patch Set 13 : Adjust display-cautionary docstring. #

Patch Set 14 : integrate tab harmonics #

Patch Set 15 : Remove HarmonicParenthesesItem #

Patch Set 16 : Adjust offset #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -4 lines) Patch
M scm/tablature.scm View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +4 lines, -4 lines 0 comments Download

Messages

Total messages: 61
Carl
Here's a patch to Marc's tab stuff that has the engraver moved to a C++ ...
8 years, 7 months ago (2010-10-31 03:35:10 UTC) #1
marc
Hello Carl, wow, you were fast ... LGTM ;-) As said before, I wouldn't be ...
8 years, 7 months ago (2010-10-31 08:23:59 UTC) #2
Trevor Daniels
I may be missing something, but doesn't this assume all line spanners are glissandi? Trevor ...
8 years, 7 months ago (2010-10-31 09:29:29 UTC) #3
Carl
On 2010/10/31 09:29:29, Trevor Daniels wrote: > I may be missing something, but doesn't this ...
8 years, 7 months ago (2010-10-31 09:42:24 UTC) #4
Carl
I've updated the comments and the doc string, and added a check for Glissando. Thanks, ...
8 years, 7 months ago (2010-10-31 10:30:17 UTC) #5
Neil Puttock
Hi Carl, This is too complicated (though that's really a criticism of Marc's Scheme engraver). ...
8 years, 7 months ago (2010-10-31 22:24:17 UTC) #6
Carl
On 2010/10/31 22:24:17, Neil Puttock wrote: > Hi Carl, > > This is too complicated ...
8 years, 7 months ago (2010-10-31 22:36:34 UTC) #7
Carl
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 () == ...
8 years, 7 months ago (2010-10-31 22:40:57 UTC) #8
Neil Puttock
On 2010/10/31 22:40:57, Carl wrote: > This is going away, so it won't apply to ...
8 years, 7 months ago (2010-10-31 22:51:38 UTC) #9
Carl
New patch set with simplified engraver -- it only acknowledges ties and note-heads, and it ...
8 years, 7 months ago (2010-10-31 22:54:36 UTC) #10
Carl
Further thought about this causes me to think the engraver should just be Tie_follow_engraver It ...
8 years, 7 months ago (2010-11-01 01:21:38 UTC) #11
Carl
Or perhaps the engraver should only listen to tab-note-heads, instead of to note-heads, since the ...
8 years, 7 months ago (2010-11-01 01:25:43 UTC) #12
marc
Am 31.10.2010 23:24, schrieb n.puttock@gmail.com: > Hi Carl, > > This is too complicated (though ...
8 years, 7 months ago (2010-11-01 07:53:27 UTC) #13
c_sorensen
On 11/1/10 1:53 AM, "Marc Hohl" <marc@hohlart.de> wrote: > > Sorry for causing so much ...
8 years, 7 months ago (2010-11-01 12:31:16 UTC) #14
Carl
Updated to only acknowledge tab-note-head, not note-head. Thanks, Carl
8 years, 7 months ago (2010-11-02 03:04:32 UTC) #15
marc
Am 02.11.2010 04:04, schrieb Carl.D.Sorensen@gmail.com: > Updated to only acknowledge tab-note-head, not note-head. Makes perfect ...
8 years, 7 months ago (2010-11-03 12:50:29 UTC) #16
c_sorensen
On 11/3/10 6:50 AM, "Marc Hohl" <marc@hohlart.de> wrote: > Am 02.11.2010 04:04, schrieb Carl.D.Sorensen@gmail.com: >> ...
8 years, 7 months ago (2010-11-03 14:10:54 UTC) #17
marc
Am 03.11.2010 15:10, schrieb Carl Sorensen: > > > On 11/3/10 6:50 AM, "Marc Hohl"<marc@hohlart.de> ...
8 years, 7 months ago (2010-11-03 16:16:00 UTC) #18
Neil Puttock
On 3 November 2010 16:15, Marc Hohl <marc@hohlart.de> wrote: > Am 03.11.2010 15:10, schrieb Carl ...
8 years, 7 months ago (2010-11-03 18:43:51 UTC) #19
c_sorensen
On 11/3/10 12:43 PM, "Neil Puttock" <n.puttock@gmail.com> wrote: > On 3 November 2010 16:15, Marc ...
8 years, 7 months ago (2010-11-03 19:33:29 UTC) #20
c_sorensen
On 11/3/10 10:15 AM, "Marc Hohl" <marc@hohlart.de> wrote: > Am 03.11.2010 15:10, schrieb Carl Sorensen: ...
8 years, 7 months ago (2010-11-03 19:34:45 UTC) #21
Neil Puttock
On 3 November 2010 19:33, Carl Sorensen <c_sorensen@byu.edu> wrote: > But the tie callback *should* ...
8 years, 7 months ago (2010-11-03 20:22:59 UTC) #22
Neil Puttock
Hi Carl, What do you think about folding this code into the Tab_note_heads_engraver? We could ...
8 years, 7 months ago (2010-11-03 20:46:27 UTC) #23
Carl
On 2010/11/03 20:46:27, Neil Puttock wrote: > Hi Carl, > > What do you think ...
8 years, 7 months ago (2010-11-03 23:14:47 UTC) #24
Neil Puttock
On 2010/11/03 23:14:47, Carl wrote: > I think I'd probably go a bit farther. > ...
8 years, 7 months ago (2010-11-03 23:52:22 UTC) #25
c_sorensen
On 11/3/10 5:52 PM, "n.puttock@gmail.com" <n.puttock@gmail.com> wrote: > On 2010/11/03 23:14:47, Carl wrote: > >> ...
8 years, 7 months ago (2010-11-04 03:02:15 UTC) #26
marc
Am 03.11.2010 21:22, schrieb Neil Puttock: > On 3 November 2010 19:33, Carl Sorensen<c_sorensen@byu.edu> wrote: ...
8 years, 7 months ago (2010-11-04 08:26:52 UTC) #27
marc
Am 03.11.2010 20:33, schrieb Carl Sorensen: > > > On 11/3/10 10:15 AM, "Marc Hohl"<marc@hohlart.de> ...
8 years, 7 months ago (2010-11-04 08:31:30 UTC) #28
Carl
On 2010/11/04 08:31:30, marc wrote: > No, I don't think we should it do more ...
8 years, 7 months ago (2010-11-07 03:52:29 UTC) #29
Carl
This patch has revised the callbacks in scm/tablature.scm. It appears to work properly. The tab-tie-follow-engraver ...
8 years, 7 months ago (2010-11-07 07:47:01 UTC) #30
marc
Hello Carl, I didn't test the patch, but the general logic behind it seems to ...
8 years, 7 months ago (2010-11-07 09:21:45 UTC) #31
marc
Am 07.11.2010 08:47, schrieb Carl.D.Sorensen@gmail.com: > This patch has revised the callbacks in scm/tablature.scm. It ...
8 years, 7 months ago (2010-11-07 09:24:31 UTC) #32
Carl
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 ...
8 years, 7 months ago (2010-11-12 04:07:02 UTC) #33
Carl
On 2010/11/07 09:24:31, marc wrote: > Am 07.11.2010 08:47, schrieb Carl.D.Sorensen@gmail.com: > > In this ...
8 years, 7 months ago (2010-11-12 04:08:37 UTC) #34
Carl
Here's a new version of the patch that tries to eliminate any scheme calls, and ...
8 years, 7 months ago (2010-11-12 04:12:48 UTC) #35
marc
Am 12.11.2010 05:07, schrieb Carl.D.Sorensen@gmail.com: > On 2010/11/07 09:21:45, marc wrote: > > http://codereview.appspot.com/2723043/diff/57001/scm/define-grob-properties.scm#newcode1021 > ...
8 years, 7 months ago (2010-11-12 06:55:08 UTC) #36
Neil Puttock
On 2010/11/12 04:12:48, Carl wrote: > Here's a new version of the patch that tries ...
8 years, 7 months ago (2010-11-12 17:40:42 UTC) #37
Carl
Thanks for the help on the null pointer. I was thinking that it was some ...
8 years, 7 months ago (2010-11-13 05:21:46 UTC) #38
marc
Am 13.11.2010 06:21, schrieb Carl.D.Sorensen@gmail.com: > Thanks for the help on the null pointer. I ...
8 years, 7 months ago (2010-11-13 10:18:45 UTC) #39
c_sorensen
On 11/13/10 3:18 AM, "Marc Hohl" <marc@hohlart.de> wrote: > Am 13.11.2010 06:21, schrieb Carl.D.Sorensen@gmail.com: >> ...
8 years, 7 months ago (2010-11-13 14:29:52 UTC) #40
marc
Am 13.11.2010 15:29, schrieb Carl Sorensen: > On 11/13/10 3:18 AM, "Marc Hohl"<marc@hohlart.de> wrote: > ...
8 years, 7 months ago (2010-11-13 19:00:38 UTC) #41
Neil Puttock
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 <Spanner ...
8 years, 7 months ago (2010-11-16 23:30:41 UTC) #42
Carl
Thanks for the review, Neil. I've responded to all your comments. I've also defined a ...
8 years, 7 months ago (2010-11-27 03:34:06 UTC) #43
marc
Hello Carl, your code looks great (at least at a quick glance), but it looks ...
8 years, 7 months ago (2010-11-27 09:05:58 UTC) #44
Carl
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 ...
8 years, 7 months ago (2010-11-27 14:40:16 UTC) #45
marc
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: ...
8 years, 7 months ago (2010-11-28 15:42:47 UTC) #46
Neil Puttock
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. ...
8 years, 7 months ago (2010-11-28 22:24:05 UTC) #47
Carl
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")); ...
8 years, 7 months ago (2010-11-29 01:11:37 UTC) #48
Carl
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 ...
8 years, 6 months ago (2010-11-30 03:25:43 UTC) #49
Carl
On 2010/11/28 15:42:47, marc wrote: > Just some remarks about the harmonic detection. Thanks, Marc. ...
8 years, 6 months ago (2010-12-03 16:23:32 UTC) #50
Carl
Thanks for the feedback. I've responded to the comments and posted a new patch set. ...
8 years, 6 months ago (2010-12-03 16:32:07 UTC) #51
Neil Puttock
On 2010/12/03 16:32:07, Carl wrote: > Added an offset to get the column centered one ...
8 years, 6 months ago (2010-12-04 23:50:29 UTC) #52
c_sorensen
On 12/4/10 4:50 PM, "n.puttock@gmail.com" <n.puttock@gmail.com> wrote: > On 2010/12/03 16:32:07, Carl wrote: > >> ...
8 years, 6 months ago (2010-12-05 00:35:32 UTC) #53
Neil Puttock
On 5 December 2010 00:35, Carl Sorensen <c_sorensen@byu.edu> wrote: > How's this? To my eye ...
8 years, 6 months ago (2010-12-05 00:58:13 UTC) #54
Carl
On 2010/12/05 00:58:13, Neil Puttock wrote: > I don't know, but they're consistently shifted to ...
8 years, 6 months ago (2010-12-05 01:19:10 UTC) #55
Carl
On 2010/12/05 00:58:13, Neil Puttock wrote: > I don't know, but they're consistently shifted to ...
8 years, 6 months ago (2010-12-05 01:19:12 UTC) #56
marc
Am 05.12.2010 02:19, schrieb Carl.D.Sorensen@gmail.com: > On 2010/12/05 00:58:13, Neil Puttock wrote: >> I don't ...
8 years, 6 months ago (2010-12-05 19:22:57 UTC) #57
c_sorensen
On 12/5/10 12:22 PM, "Marc Hohl" <marc@hohlart.de> wrote: > Am 05.12.2010 02:19, schrieb Carl.D.Sorensen@gmail.com: >> ...
8 years, 6 months ago (2010-12-05 22:12:39 UTC) #58
marc
Am 05.12.2010 23:12, schrieb Carl Sorensen: > > > [...] >> Perhaps the left edge ...
8 years, 6 months ago (2010-12-06 10:37:04 UTC) #59
c_sorensen
On 12/6/10 3:37 AM, "Marc Hohl" <marc@hohlart.de> wrote: > Am 05.12.2010 23:12, schrieb Carl Sorensen: ...
8 years, 6 months ago (2010-12-06 23:56:52 UTC) #60
marc
8 years, 6 months ago (2010-12-08 07:36:00 UTC) #61
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

Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b