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

Issue 4961041: Sketch for broken beams with consistent slopes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 7 months ago by MikeSol
Modified:
12 years, 5 months ago
Reviewers:
Keith, Neil Puttock, mikesol, joeneeman, pkx166h, hanwenn
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Sketch for broken beams with consistent slopes

Patch Set 1 #

Patch Set 2 : Fixes one name with two underscores. #

Total comments: 1

Patch Set 3 : Sketch for broken beams with consistent slopes. #

Patch Set 4 : Regtest added. #

Patch Set 5 : Fixes kneed beams. #

Patch Set 6 : Fixes chord tremolos. #

Patch Set 7 : Better handling of normal stems. #

Patch Set 8 : Rebase against current master. #

Patch Set 9 : Most recent stem slope patch against current master. #

Patch Set 10 : Beams with consistent slopes over line breaks. #

Total comments: 1

Patch Set 11 : Merge after cosmetic variable name push. #

Patch Set 12 : Rebase against cosmetic patch (issue 1952). #

Patch Set 13 : Rebase against current master. #

Patch Set 14 : Dampens slope for beams with stemlets. #

Patch Set 15 : Adds changes to changes.tely #

Patch Set 16 : Gets x_span_ right. #

Total comments: 15
Unified diffs Side-by-side diffs Delta from patch set Stats (+380 lines, -444 lines) Patch
M Documentation/changes.tely View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +19 lines, -0 lines 4 comments Download
M input/regression/beam-concave.ly View 1 2 3 1 chunk +1 line, -5 lines 0 comments Download
M input/regression/beam-concave-chord.ly View 1 2 3 1 chunk +3 lines, -13 lines 0 comments Download
A input/regression/beam-consistent-broken-slope.ly View 1 2 3 4 5 6 1 chunk +31 lines, -0 lines 3 comments Download
M input/regression/beam-default-lengths.ly View 1 2 3 1 chunk +1 line, -10 lines 0 comments Download
M input/regression/beam-shortened-lengths.ly View 1 2 3 1 chunk +1 line, -10 lines 0 comments Download
M lily/beam.cc View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +4 lines, -3 lines 0 comments Download
M lily/beam-quanting.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 chunks +288 lines, -358 lines 4 comments Download
M lily/include/beam.hh View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +1 line, -5 lines 0 comments Download
M lily/include/beam-scoring-problem.hh View 1 2 3 4 5 6 7 8 9 10 4 chunks +17 lines, -7 lines 0 comments Download
M ly/music-functions-init.ly View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -12 lines 0 comments Download
M scm/define-grob-properties.scm View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +3 lines, -0 lines 1 comment Download
M scm/define-grobs.scm View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -9 lines 1 comment Download
M scm/layout-beam.scm View 1 2 3 1 chunk +8 lines, -12 lines 2 comments Download

Messages

Total messages: 29
MikeSol
Hey all, This patch changes the name of several variables that I'll need to work ...
12 years, 7 months ago (2011-08-25 07:29:38 UTC) #1
pkx166h
passes make and reg tests
12 years, 7 months ago (2011-08-27 08:49:50 UTC) #2
hanwenn
LGTM On Thu, Aug 25, 2011 at 4:29 AM, <mtsolo@gmail.com> wrote: > Reviewers: , > ...
12 years, 7 months ago (2011-08-27 14:11:29 UTC) #3
joeneeman
http://codereview.appspot.com/4961041/diff/3001/lily/include/beam-scoring-problem.hh File lily/include/beam-scoring-problem.hh (right): http://codereview.appspot.com/4961041/diff/3001/lily/include/beam-scoring-problem.hh#newcode114 lily/include/beam-scoring-problem.hh:114: TODO - use trailing _ on data members. I ...
12 years, 7 months ago (2011-09-01 15:20:17 UTC) #4
MikeSol
Hey all, Sorry for hijacking this issue to turn it into another one. I'll take ...
12 years, 7 months ago (2011-09-01 16:13:00 UTC) #5
mikesol_ufl.edu
On Sep 1, 2011, at 6:13 PM, mtsolo@gmail.com wrote: > Other than that, I believe ...
12 years, 7 months ago (2011-09-01 16:22:12 UTC) #6
MikeSol
Update: looks like cross-staff beams are broken because I removed all of the commony code ...
12 years, 7 months ago (2011-09-01 19:00:12 UTC) #7
MikeSol
Hey all, This is now in a state where people can run regtests on it ...
12 years, 6 months ago (2011-09-18 18:19:28 UTC) #8
MikeSol
All of the previously-reported bugs are now squashed with this newest patchset: the regtests only ...
12 years, 6 months ago (2011-09-19 09:14:59 UTC) #9
pkx166h
Passes make but there are a few reg tests that someone should look at See ...
12 years, 6 months ago (2011-09-24 19:57:04 UTC) #10
mikesol_ufl.edu
On Sep 24, 2011, at 9:57 PM, pkx166h@gmail.com wrote: > Passes make but there are ...
12 years, 6 months ago (2011-09-25 03:44:36 UTC) #11
MikeSol
Hey all, I've already heard back from Neil about this patch - he's gonna have ...
12 years, 6 months ago (2011-10-03 04:05:09 UTC) #12
hanwenn
i know it's annoying, but could you separate out the cosmetics (adding _ ) to ...
12 years, 6 months ago (2011-10-03 04:53:48 UTC) #13
pkx166h
Passes make. I get a few reg tests pop up; see attached at http://code.google.com/p/lilypond/issues/detail?id=1952#c1 but ...
12 years, 6 months ago (2011-10-03 07:20:33 UTC) #14
mikesol_ufl.edu
On Oct 3, 2011, at 6:53 AM, hanwenn@gmail.com wrote: > i know it's annoying, but ...
12 years, 5 months ago (2011-10-03 18:50:27 UTC) #15
mikesol_ufl.edu
On Oct 3, 2011, at 6:53 AM, hanwenn@gmail.com wrote: > i know it's annoying, but ...
12 years, 5 months ago (2011-10-03 18:51:01 UTC) #16
mikesol_ufl.edu
On Oct 3, 2011, at 8:50 PM, Mike Solomon wrote: > On Oct 3, 2011, ...
12 years, 5 months ago (2011-10-04 09:57:55 UTC) #17
hanwenn
You skipped the cosmetic patch that folds together the (SCM, SCM) callbacks into one big ...
12 years, 5 months ago (2011-10-04 13:26:46 UTC) #18
mikesol_ufl.edu
On Oct 4, 2011, at 3:26 PM, Han-Wen Nienhuys wrote: > You skipped the cosmetic ...
12 years, 5 months ago (2011-10-04 13:31:01 UTC) #19
mikesol_ufl.edu
On Oct 4, 2011, at 3:26 PM, Han-Wen Nienhuys wrote: > You skipped the cosmetic ...
12 years, 5 months ago (2011-10-04 14:48:41 UTC) #20
hanwenn
On Tue, Oct 4, 2011 at 10:30 AM, Mike Solomon <mikesol@ufl.edu> wrote: > On Oct ...
12 years, 5 months ago (2011-10-04 17:47:12 UTC) #21
pkx166h
passes make but I get some reg tests show up. See http://code.google.com/p/lilypond/issues/detail?id=1844#c11
12 years, 5 months ago (2011-10-05 07:21:58 UTC) #22
mikesol_ufl.edu
On Oct 4, 2011, at 7:47 PM, Han-Wen Nienhuys wrote: > On Tue, Oct 4, ...
12 years, 5 months ago (2011-10-05 08:20:17 UTC) #23
pkx166h
Passes make, make check gives me the usual suspects on the reg test http://code.google.com/p/lilypond/issues/detail?id=1952#c3 but ...
12 years, 5 months ago (2011-10-06 19:16:44 UTC) #24
Neil Puttock
http://codereview.appspot.com/4961041/diff/60001/Documentation/changes.tely File Documentation/changes.tely (right): http://codereview.appspot.com/4961041/diff/60001/Documentation/changes.tely#newcode66 Documentation/changes.tely:66: \override Beam #'breakable = ##t no indent http://codereview.appspot.com/4961041/diff/60001/Documentation/changes.tely#newcode68 Documentation/changes.tely:68: ...
12 years, 5 months ago (2011-10-20 19:24:44 UTC) #25
mikesol_ufl.edu
Thanks Neil! On Oct 20, 2011, at 9:24 PM, n.puttock@gmail.com wrote: > http://codereview.appspot.com/4961041/diff/60001/scm/define-grob-properties.scm > File ...
12 years, 5 months ago (2011-10-21 06:50:18 UTC) #26
Neil Puttock
On 21 October 2011 07:50, Mike Solomon <mikesol@ufl.edu> wrote: > Of this I am not ...
12 years, 5 months ago (2011-10-21 11:20:00 UTC) #27
Keith
Rather indecipherable. The 350 lines of code re-ordering make it difficult to find the change ...
12 years, 5 months ago (2011-10-26 18:57:38 UTC) #28
MikeSol
12 years, 5 months ago (2011-10-26 19:18:07 UTC) #29
Thanks for the comments!
Just a technical note - this issue was closed after the patch was pushed to
current master, but I don't mind opening it again to continue a discussion about
what this patch (if Colin has no objections).

The code I'm dealing with in beam-quanting and beam is difficult to decipher to
begin with: it took me at least a year to get my head fully around what it's
doing and to figure out where the gaps were.  I know there is a lot of
reorganization, but I wouldn't do it unless I felt that it was necessary to make
the code perform more consistently (i.e. to standardize the measuring of X
extent).

Cheers,
MS

http://codereview.appspot.com/4961041/diff/60001/lily/beam-quanting.cc
File lily/beam-quanting.cc (right):

http://codereview.appspot.com/4961041/diff/60001/lily/beam-quanting.cc#newcod...
lily/beam-quanting.cc:202: for (vsize i = 0; i < beams.size (); i++)
On 2011/10/26 18:57:38, Keith wrote:
> When there is more than one beam in 'beams' what do you intend the members
> Beam_scoring_problem to represent ?  The beam as if it were on one line, but
> allowing the extra horizontal span due to the line-break ?  Should the members
> like segments_ and is_knee_ include both halves of the beam ?

All of Beam_scoring_problem happens in an artificial space as if there was no
line break (which is why this function is so long - you'll see x_span_ and
x_left being used quite a bit to bring elements to their positions on this
imaginary line).  So, Beam_scoring_problem does not know what line breaking is,
and it is only at the very end that the normalized endpoints are used to take
care of the breaks.

One of the reasons for my most recent patch is that the normalized endpoints of
the beam don't sync up with its actual extent, which is only reliably derived
from the segments.

http://codereview.appspot.com/4961041/diff/60001/lily/beam-quanting.cc#newcod...
lily/beam-quanting.cc:947: final_positions[RIGHT] -= (1 -
normalized_endpoints[RIGHT]) * y_length;
On 2011/10/26 18:57:38, Keith wrote:
> At what horizontal locations are these final_positions? First and last stems
or
> the end of the beam including overhang or x_span_ ?

Including overhang.  This is improved in the most recent patch set.
Sign in to reply to this message.

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