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

Issue 4442083: rest-collision.cc: adjust all rests in column. Issues 1618 and 1547 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 4 months ago by Keith
Modified:
8 years, 4 months ago
Reviewers:
Graham Percival (old account), carl.d.sorensen
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

rest-collision.cc: adjust all rests in column Process all rests in a noteColumn, not just the first; fix issue 1618. Print warning only for rests that are moved but don't fit; fix issue 1547.

Patch Set 1 #

Patch Set 2 : fake whitespace for diff for review, add regtest #

Total comments: 3

Patch Set 3 : Carl suggestions #

Unified diffs Side-by-side diffs Delta from patch set Stats (+85 lines, -63 lines) Patch
A input/regression/rest-polyphonic-2.ly View 1 2 1 chunk +19 lines, -0 lines 0 comments Download
M lily/rest-collision.cc View 1 2 1 chunk +66 lines, -63 lines 0 comments Download

Messages

Total messages: 3
Keith
It was tempting to resolve all collisions, as is already done when there are only ...
8 years, 4 months ago (2011-04-26 04:41:50 UTC) #1
Graham Percival (old account)
LTGM, and regtests are fine.
8 years, 4 months ago (2011-04-28 12:20:49 UTC) #2
Carl
8 years, 4 months ago (2011-04-30 14:46:51 UTC) #3
LGTM.  A couple of non-essential comments.

http://codereview.appspot.com/4442083/diff/12001/input/regression/rest-polyph...
File input/regression/rest-polyphonic-2.ly (right):

http://codereview.appspot.com/4442083/diff/12001/input/regression/rest-polyph...
input/regression/rest-polyphonic-2.ly:5: result in collision, but is supressed
if the rest has a pitch."
The texidoc should have a statement that describes what the output should look
like.

http://codereview.appspot.com/4442083/diff/12001/lily/rest-collision.cc
File lily/rest-collision.cc (right):

http://codereview.appspot.com/4442083/diff/12001/lily/rest-collision.cc#newco...
lily/rest-collision.cc:281: "Move around ordinary rests (not
multi-measure-rests) to avoid"
Perhaps include pitched rests along with mult-measure-rests in the description
of rests not affected by this interface.
Sign in to reply to this message.

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