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

Issue 4290069: Adds beam collision avoidance to auto beaming (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years ago by MikeSol
Modified:
12 years, 12 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Adds beam collision avoidance to auto beaming

Patch Set 1 : Adds beam collision avoidance to auto beaming #

Patch Set 2 : Fixes sorting algorithm #

Total comments: 5

Patch Set 3 : Makes logic of loops in finalize more transparent #

Total comments: 10

Patch Set 4 : Incorporate's Neil's suggestions. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -120 lines) Patch
M lily/beam-collision-engraver.cc View 1 2 3 4 chunks +50 lines, -120 lines 2 comments Download

Messages

Total messages: 13
MikeSol
Actualizes Han Wen's idea for putting everything in finalize. Seems to work... Cheers, Mike
13 years ago (2011-03-27 00:39:11 UTC) #1
Graham Percival (old account)
http://codereview.appspot.com/4290069/diff/5001/lily/beam-collision-engraver.cc File lily/beam-collision-engraver.cc (right): http://codereview.appspot.com/4290069/diff/5001/lily/beam-collision-engraver.cc#newcode55 lily/beam-collision-engraver.cc:55: if (covered_grobs_.size ()) I'm going to start testing this ...
13 years ago (2011-03-27 19:29:47 UTC) #2
mike_apollinemike.com
On Mar 27, 2011, at 3:29 PM, percival.music.ca@gmail.com wrote: > > http://codereview.appspot.com/4290069/diff/5001/lily/beam-collision-engraver.cc > File lily/beam-collision-engraver.cc ...
13 years ago (2011-03-27 20:23:59 UTC) #3
hanwenn
http://codereview.appspot.com/4290069/diff/5001/lily/beam-collision-engraver.cc File lily/beam-collision-engraver.cc (right): http://codereview.appspot.com/4290069/diff/5001/lily/beam-collision-engraver.cc#newcode55 lily/beam-collision-engraver.cc:55: if (covered_grobs_.size ()) to drop 1 indent level, do ...
13 years ago (2011-03-28 11:13:35 UTC) #4
hanwenn
On Mon, Mar 28, 2011 at 8:13 AM, <hanwenn@gmail.com> wrote: > > http://codereview.appspot.com/4290069/diff/5001/lily/beam-collision-engraver.cc > File ...
13 years ago (2011-03-28 11:19:28 UTC) #5
MikeSol
Fixed w/ Han-Wen's suggestions. http://codereview.appspot.com/4290069/diff/5001/lily/beam-collision-engraver.cc File lily/beam-collision-engraver.cc (right): http://codereview.appspot.com/4290069/diff/5001/lily/beam-collision-engraver.cc#newcode55 lily/beam-collision-engraver.cc:55: if (covered_grobs_.size ()) On 2011/03/28 ...
13 years ago (2011-03-28 12:20:03 UTC) #6
Neil Puttock
http://codereview.appspot.com/4290069/diff/8001/lily/beam-collision-engraver.cc File lily/beam-collision-engraver.cc (right): http://codereview.appspot.com/4290069/diff/8001/lily/beam-collision-engraver.cc#newcode50 lily/beam-collision-engraver.cc:50: sort (covered_grobs_.begin (), covered_grobs_.end (), Grob::less); vector_sort (covered_grobs_, Grob::less); ...
13 years ago (2011-03-28 20:50:09 UTC) #7
MikeSol
New patch set uploaded. http://codereview.appspot.com/4290069/diff/8001/lily/beam-collision-engraver.cc File lily/beam-collision-engraver.cc (right): http://codereview.appspot.com/4290069/diff/8001/lily/beam-collision-engraver.cc#newcode50 lily/beam-collision-engraver.cc:50: sort (covered_grobs_.begin (), covered_grobs_.end (), ...
13 years ago (2011-03-28 22:12:25 UTC) #8
Graham Percival (old account)
Looks much more readable, thanks! http://codereview.appspot.com/4290069/diff/11001/lily/beam-collision-engraver.cc File lily/beam-collision-engraver.cc (right): http://codereview.appspot.com/4290069/diff/11001/lily/beam-collision-engraver.cc#newcode59 lily/beam-collision-engraver.cc:59: // Start conisdering grobs ...
13 years ago (2011-03-28 22:39:13 UTC) #9
mike_apollinemike.com
On Mar 28, 2011, at 6:39 PM, percival.music.ca@gmail.com wrote: > Looks much more readable, thanks! ...
13 years ago (2011-03-28 23:28:03 UTC) #10
mike_apollinemike.com
On Mar 28, 2011, at 6:39 PM, percival.music.ca@gmail.com wrote: > Looks much more readable, thanks! ...
13 years ago (2011-03-29 22:54:56 UTC) #11
hanwenn
lgtm http://codereview.appspot.com/4290069/diff/11001/lily/beam-collision-engraver.cc File lily/beam-collision-engraver.cc (right): http://codereview.appspot.com/4290069/diff/11001/lily/beam-collision-engraver.cc#newcode64 lily/beam-collision-engraver.cc:64: for (vsize j = start; covered_grobs_[j]->spanned_rank_interval ()[LEFT] <= ...
13 years ago (2011-03-30 04:30:22 UTC) #12
mike_apollinemike.com
13 years ago (2011-03-30 16:12:00 UTC) #13
On Mar 30, 2011, at 12:30 AM, hanwenn@gmail.com wrote:

> lgtm
> 
> 
>
http://codereview.appspot.com/4290069/diff/11001/lily/beam-collision-engraver.cc
> File lily/beam-collision-engraver.cc (right):
> 
>
http://codereview.appspot.com/4290069/diff/11001/lily/beam-collision-engraver...
> lily/beam-collision-engraver.cc:64: for (vsize j = start;
> covered_grobs_[j]->spanned_rank_interval ()[LEFT] <=
> beams_[i]->spanned_rank_interval ()[RIGHT]; j++)
> you migth want to store beam[i]->spanned_rank_interval() in a local
> variable to improve readability.

Done and pushed.
e4be20302c832985b7faac6fc0daf1f45f382391

Cheers,
MS
Sign in to reply to this message.

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