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

Issue 4860043: Better pure height approximations for beamed rests. (Closed)

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

Description

Better pure height approximations for beamed rests.

Patch Set 1 #

Total comments: 7

Patch Set 2 : Various fixes from Reinhold and Neil. #

Total comments: 1

Patch Set 3 : Changes the function to a pure Y-offset function. #

Patch Set 4 : Housekeeping. #

Total comments: 2

Patch Set 5 : Fixes snag with potential lack of staff. #

Total comments: 3

Patch Set 6 : Incorporates Neil's comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+90 lines, -1 line) Patch
A input/regression/beam-rest-extreme.ly View 1 chunk +14 lines, -0 lines 0 comments Download
M lily/beam.cc View 1 2 3 4 5 2 chunks +72 lines, -0 lines 0 comments Download
M lily/include/beam.hh View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M lily/rest.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M scm/define-grobs.scm View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 10
MikeSol
12 years, 7 months ago (2011-08-10 15:53:21 UTC) #1
Reinhold
I haven't actually tested it, but from reading the code it LGTM in general. Please ...
12 years, 7 months ago (2011-08-10 16:31:42 UTC) #2
Neil Puttock
http://codereview.appspot.com/4860043/diff/1/lily/rest.cc File lily/rest.cc (right): http://codereview.appspot.com/4860043/diff/1/lily/rest.cc#newcode238 lily/rest.cc:238: // Wont play nicely with weird staves where individual ...
12 years, 7 months ago (2011-08-10 16:37:37 UTC) #3
MikeSol
All comments have been addressed and a new patchset's uploaded. Cheers, MS
12 years, 7 months ago (2011-08-10 19:07:33 UTC) #4
Neil Puttock
http://codereview.appspot.com/4860043/diff/4002/lily/rest.cc File lily/rest.cc (right): http://codereview.appspot.com/4860043/diff/4002/lily/rest.cc#newcode251 lily/rest.cc:251: Interval rest_max_pos = Staff_symbol::line_span (Staff_symbol_referencer::get_staff_symbol (me)); unsafe with \stopStaff ...
12 years, 7 months ago (2011-08-10 20:22:58 UTC) #5
pkx166h
Passes make and reg tests
12 years, 7 months ago (2011-08-10 21:13:25 UTC) #6
Neil Puttock
LGTM. http://codereview.appspot.com/4860043/diff/11001/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/4860043/diff/11001/lily/beam.cc#newcode1736 lily/beam.cc:1736: Below, the prev_offset parameter is intentionally unused. This ...
12 years, 7 months ago (2011-08-10 22:22:02 UTC) #7
MikeSol
Pushed as 9c319db89ecc46cbf6c9c35e759eba973e412bb0. Cheers, MS
12 years, 7 months ago (2011-08-21 15:19:39 UTC) #8
Keith
http://codereview.appspot.com/4860043/diff/4/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/4860043/diff/4/lily/beam.cc#newcode1743 lily/beam.cc:1743: Beam::pure_rest_collision_callback (SCM smob, SCM prev_offset, Mike, Based on their ...
11 years, 10 months ago (2012-04-27 06:06:21 UTC) #9
mike7
11 years, 10 months ago (2012-04-27 06:44:24 UTC) #10
http://codereview.appspot.com/4860043/diff/4/lily/beam.cc
File lily/beam.cc (right):

http://codereview.appspot.com/4860043/diff/4/lily/beam.cc#newcode1743
lily/beam.cc:1743: Beam::pure_rest_collision_callback (SCM smob, SCM
prev_offset,
On 2012/04/27 06:06:21, Keith wrote:
> Mike, Based on their contents, the arguments appear to be in a different
order: 
>    grob, start, end, prev_offset
> which is different than what you wrote in CG 10.13.2 

You're right - this is an error.  Do you want me to fix it or are you working on
a patch into which the fix could be incorporated?
Sign in to reply to this message.

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