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

Issue 5293060: Fixes slope errors from incorrect X extents in Beam::print. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 1 month ago by MikeSol
Modified:
8 years ago
Reviewers:
Keith, mike, pkx166h, carl.d.sorensen
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Fixes slope errors from incorrect X extents in Beam::print.

Patch Set 1 #

Patch Set 2 : Makes calc_x_span more coherent. #

Total comments: 1

Patch Set 3 : Modifications to bring patch closer to Scriabin Op. 11 No. 1 ed. Peters. #

Patch Set 4 : Consistent X measurements across the board. #

Patch Set 5 : Gets rid of uninitialized variables, fixes some borderline quants. #

Total comments: 13

Patch Set 6 : Incorporates changes from Keith and Carl. #

Patch Set 7 : Moves all quanting manipulation to Scheme. #

Patch Set 8 : Responses to Keith's ideas. #

Total comments: 22

Patch Set 9 : Adds suggestions from Keith. #

Patch Set 10 : Gets concaveness right. #

Patch Set 11 : Uses naming conventions proposed by Keith. #

Patch Set 12 : Removes condition that limits increase in region size. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+495 lines, -334 lines) Patch
M Documentation/changes.tely View 1 2 3 4 5 6 7 8 9 10 2 chunks +7 lines, -4 lines 0 comments Download
A input/regression/beam-broken-classic.ly View 1 2 3 4 5 6 7 8 9 10 1 chunk +43 lines, -0 lines 0 comments Download
A input/regression/beam-broken-difficult.ly View 1 2 3 4 5 6 7 8 9 10 1 chunk +27 lines, -0 lines 0 comments Download
D input/regression/beam-consistent-broken-slope.ly View 1 2 3 1 chunk +0 lines, -31 lines 0 comments Download
A input/regression/beam-quanting-overhang.ly View 1 2 3 4 5 6 7 8 1 chunk +13 lines, -0 lines 0 comments Download
M lily/beam.cc View 1 2 3 4 5 6 7 8 9 10 12 chunks +98 lines, -39 lines 0 comments Download
D lily/beam-concave.cc View 1 2 3 4 5 6 7 8 9 1 chunk +0 lines, -170 lines 0 comments Download
M lily/beam-quanting.cc View 1 2 3 4 5 6 7 8 9 10 11 14 chunks +219 lines, -68 lines 0 comments Download
M lily/include/beam.hh View 1 2 3 4 5 6 7 8 9 2 chunks +4 lines, -3 lines 0 comments Download
M lily/include/beam-scoring-problem.hh View 1 2 3 4 5 6 7 8 9 10 2 chunks +5 lines, -4 lines 0 comments Download
M lily/spanner.cc View 1 2 3 4 5 6 7 8 1 chunk +24 lines, -7 lines 0 comments Download
M python/convertrules.py View 1 2 3 4 5 6 7 8 9 2 chunks +7 lines, -1 line 0 comments Download
M scm/define-grob-properties.scm View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +1 line, -3 lines 0 comments Download
scm/define-grobs.scm View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +3 lines, -2 lines 0 comments Download
M scm/layout-beam.scm View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -2 lines 0 comments Download
M scm/output-lib.scm View 1 2 3 4 5 6 7 8 9 10 1 chunk +42 lines, -0 lines 0 comments Download

Messages

Total messages: 19
Keith
http://codereview.appspot.com/5293060/diff/2001/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/5293060/diff/2001/lily/beam.cc#newcode987 lily/beam.cc:987: Beam::calc_x_span (Grob *me_non_spanner, Grob *commonx) Why should the x-span ...
8 years ago (2011-10-25 03:52:31 UTC) #1
mike_apollinemike.com
On Oct 25, 2011, at 5:52 AM, k-ohara5a5a@oco.net wrote: > > http://codereview.appspot.com/5293060/diff/2001/lily/beam.cc > File lily/beam.cc ...
8 years ago (2011-10-25 05:19:47 UTC) #2
MikeSol
All problems fixed and ready for a review. A few things: 1) please ignore the ...
8 years ago (2011-10-26 15:41:12 UTC) #3
Keith
On 2011/10/26 15:41:12, MikeSol wrote: > All problems fixed and ready for a review. Thoroughly ...
8 years ago (2011-10-26 18:22:41 UTC) #4
mike_apollinemike.com
On Oct 26, 2011, at 8:22 PM, k-ohara5a5a@oco.net wrote: > On 2011/10/26 15:41:12, MikeSol wrote: ...
8 years ago (2011-10-26 18:40:33 UTC) #5
Keith
On Wed, 26 Oct 2011 11:40:27 -0700, mike@apollinemike.com <mike@apollinemike.com> wrote: > What needs comments? Description ...
8 years ago (2011-10-26 19:41:33 UTC) #6
Carl
I don't understand beam-quanting well enough to evaluate most of the code, but I've seen ...
8 years ago (2011-10-26 20:13:49 UTC) #7
mike_apollinemike.com
On Oct 26, 2011, at 10:13 PM, Carl.D.Sorensen@gmail.com wrote: > I don't understand beam-quanting well ...
8 years ago (2011-10-27 08:34:18 UTC) #8
mike_apollinemike.com
On Oct 26, 2011, at 9:41 PM, k-ohara5a5a@oco.net wrote: > On Wed, 26 Oct 2011 ...
8 years ago (2011-10-27 08:34:21 UTC) #9
mike_apollinemike.com
On Oct 28, 2011, at 7:30 AM, Keith OHara wrote: > On Thu, 27 Oct ...
8 years ago (2011-10-28 09:09:56 UTC) #10
mike_apollinemike.com
On Oct 28, 2011, at 11:09 AM, mike@apollinemike.com wrote: > On Oct 28, 2011, at ...
8 years ago (2011-10-28 12:24:39 UTC) #11
Keith
This looks more reasonable. I read through a few times until I had it figured ...
8 years ago (2011-11-01 04:19:29 UTC) #12
mike_apollinemike.com
Thanks Keith! I've incorporated most of your comments into the code and otherwise have a ...
8 years ago (2011-11-01 11:41:30 UTC) #13
mike_apollinemike.com
Hey all, I've decided to pull my patch for issue 1986 off the current countdown. ...
8 years ago (2011-11-02 09:15:09 UTC) #14
mike_apollinemike.com
On Nov 1, 2011, at 7:59 PM, Keith OHara wrote: > On Tue, 01 Nov ...
8 years ago (2011-11-02 10:19:10 UTC) #15
pkx166h
Passes Make but a lot of reg test diffs show up Load index.html from the ...
8 years ago (2011-11-03 23:17:35 UTC) #16
Keith
On 2011/11/02 10:19:10, mike_apollinemike.com wrote: > http://codereview.appspot.com/5293060/diff/19014/lily/beam-quanting.cc#newcode743 > >>> lily/beam-quanting.cc:743: if (do_initial_slope_calculations_) > >>> Even ...
8 years ago (2011-11-04 07:43:07 UTC) #17
MikeSol
On 2011/11/04 07:43:07, Keith wrote: > On 2011/11/02 10:19:10, http://mike_apollinemike.com wrote: > > > http://codereview.appspot.com/5293060/diff/19014/lily/beam-quanting.cc#newcode743 ...
8 years ago (2011-11-04 22:46:44 UTC) #18
pkx166h
8 years ago (2011-11-04 23:47:13 UTC) #19
Sign in to reply to this message.

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