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

Issue 4536088: Spacing staves with dynamics between; issue 1668 (Closed)

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

Description

Spacing staves with loose lines between. Distinguish the minimum distances between 'spaceable' staves from those between lines such as Lyrics.

Patch Set 1 : set one #

Patch Set 2 : add regtest, tweak regtest #

Patch Set 3 : rebased #

Total comments: 1

Patch Set 4 : restore comments, restore minimum-distance #

Patch Set 5 : remove tweak in engraver-init.ly #

Unified diffs Side-by-side diffs Delta from patch set Stats (+58 lines, -36 lines) Patch
M input/regression/lyrics-bar.ly View 1 1 chunk +1 line, -1 line 0 comments Download
A input/regression/page-spacing-nonstaff-lines-independent.ly View 1 1 chunk +25 lines, -0 lines 0 comments Download
M lily/align-interface.cc View 1 2 3 5 chunks +28 lines, -28 lines 0 comments Download
M lily/page-layout-problem.cc View 1 2 3 3 chunks +4 lines, -7 lines 0 comments Download

Messages

Total messages: 6
Keith
Consider each minimum-distance and padding constraint, and choose spacing that meets the most demanding constraint. ...
7 years, 11 months ago (2011-05-27 17:37:06 UTC) #1
Keith
Joe just pushed half of this, fixing 1442, so never mind for now. I'll merge ...
7 years, 11 months ago (2011-05-27 18:02:23 UTC) #2
joeneeman
On 2011/05/27 18:02:23, Keith wrote: > Joe just pushed half of this, fixing 1442, so ...
7 years, 11 months ago (2011-05-27 19:42:47 UTC) #3
Keith
On 2011/05/27 19:42:47, joeneeman wrote: > Sorry, I didn't realize we were competing for this ...
7 years, 11 months ago (2011-05-27 19:47:15 UTC) #4
joeneeman
lgtm http://codereview.appspot.com/4536088/diff/15003/lily/page-layout-problem.cc File lily/page-layout-problem.cc (left): http://codereview.appspot.com/4536088/diff/15003/lily/page-layout-problem.cc#oldcode543 lily/page-layout-problem.cc:543: // using a scheme similar to the one ...
7 years, 11 months ago (2011-05-28 07:00:55 UTC) #5
Keith
7 years, 11 months ago (2011-05-28 18:42:02 UTC) #6
On 2011/05/28 07:00:55, joeneeman wrote:
> I'd prefer to leave this comment in
Oops; comments from your last patch are back in.

Re-reading our comments, it is clear that we always want to include
minimum-distances between adjacent lines when computing
get_minimum_translations.  Adjusted patch is up.

The inaccuracy described in your comment happens with lines M v and U where the
v can rattle around between M and U.  We fail to distinguish the distance
between rattling v and U from the actual minimum v-U distance.  Omitting M-v
minimum-distance just gives more room to rattle.

To fix 1442, we needed to ignore only the minimum-distance between
*non*-adjacent already-placed Staffs M and U while distributing loose-line v.
Sign in to reply to this message.

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