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

Issue 28134: Fix spacing for accidentals in tied chords (Closed)

Can't Edit
Can't Publish+Mail
Start Review
10 years, 3 months ago by csnyder
9 years, 7 months ago


Fix spacing for accidentals in tied chords Previously, accidentals that weren't printed because they were tied still took up space in the placement of accidentals in chords. This is noted as issue # 612 in the bug tracker. This patch changes the accidental spacing code to calculate spacing for two sets of accidentals: those that will be printed mid-line, and those that will be printed after a line break, each with the correct spacing for the particular situation.

Patch Set 1 #

Total comments: 11
Unified diffs Side-by-side diffs Delta from patch set Stats (+211 lines, -88 lines) Patch
A input/regression/accidental-tied-chords.ly View 1 chunk +22 lines, -0 lines 0 comments Download
M lily/accidental.cc View 5 chunks +21 lines, -3 lines 1 comment Download
M lily/accidental-engraver.cc View 9 chunks +50 lines, -25 lines 1 comment Download
M lily/accidental-placement.cc View 7 chunks +78 lines, -46 lines 6 comments Download
M lily/include/accidental-placement.hh View 1 chunk +5 lines, -4 lines 0 comments Download
M lily/include/note-head.hh View 1 chunk +1 line, -0 lines 0 comments Download
M lily/ledger-line-spanner.cc View 2 chunks +2 lines, -2 lines 1 comment Download
M lily/new-fingering-engraver.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M lily/note-column.cc View 1 chunk +1 line, -1 line 0 comments Download
M lily/note-head.cc View 3 chunks +18 lines, -1 line 0 comments Download
M lily/tie-formatting-problem.cc View 1 chunk +1 line, -1 line 0 comments Download
M scm/define-grob-properties.scm View 2 chunks +9 lines, -3 lines 2 comments Download
M scm/define-grobs.scm View 1 chunk +1 line, -0 lines 0 comments Download


Total messages: 2
This patch implements Joe's suggested approach for correcting the spacing of tied chords with accidentals. ...
10 years, 3 months ago (2009-03-28 16:43:33 UTC) #1
10 years, 3 months ago (2009-03-29 03:42:01 UTC) #2
File lily/accidental-engraver.cc (right):

Line 431: Grob *newa = 0;
Please use a more descriptive name. Also, add a comment regarding why you need
two copies of the accidental.

File lily/accidental-placement.cc (right):

Line 30: Accidental_placement::add_accidental (Grob *me, Grob *a, Grob *newa)
Please use more descriptive parameter names.

Line 71: if (break_reminder)
slightly cleaner, perhaps, to do
SCM accidental_sym = ly_symbol2scm(break_reminder ?
"accidental-break-reminder-grobs" : "accidental_grobs");

and then
accs = me->get_object(accidental_sym);
me->set_object(accidental_sym, accs);

Line 97: ret.push_back(a);
space before (

Line 433: return;
Don't need the return here

Line 436: void Accidental_placement::filter_tied_accidentals (Grob * me)
Rather than coping every accidental and then filtering out some, have you
considered delaying the copying until calc_positioning_done and only copying the
accidentals you care about? This also has the advantage of hiding more stuff in
accidental-placement instead of modifying accidental-engraver.

Line 460: "accidental-break-reminder-grobs "
Only properties go here, not objects.

File lily/accidental.cc (right):

Line 181: }
I'm a little confused about this. Depending on whether a group of accidentals is
at the beginning of a line or not, you want either all of the break-reminder
accidentals or all of the non-break-reminder accidentals to suicide. It's not
clear to me that this is achieved by the above code.

File lily/ledger-line-spanner.cc (right):

Line 237: if (Grob *g = unsmob_grob (me->get_property ("accidental-grob")))
get_object (and subsequently, too)

File scm/define-grob-properties.scm (right):

Line 835: @var{groblist})} entries. Includes accidentals that will be printed if
at the beginning of the line.")
"Consists of" is more accurate than "Includes", I think.

Line 841: @var{groblist})} entries. Does not include tied accidentals that won't
be printed mid-line.")
Can you rephrase to avoid the double-negative?
Sign in to reply to this message.

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