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

Issue 4810072: Fixes bad slur heights by limiting fit_factor to the interior of slurs. (Closed)

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

Description

Fixes bad slur heights by limiting fit_factor to the interior of slurs.

Patch Set 1 #

Patch Set 2 : Incorporates Han Wen's suggestions and adds a regtest. #

Total comments: 5

Patch Set 3 : Changes regtest texidoc. #

Total comments: 6

Patch Set 4 : Incorporates Keith's comments. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+86 lines, -89 lines) Patch
M Documentation/contributor/programming-work.itexi View 1 2 3 1 chunk +0 lines, -10 lines 0 comments Download
M Documentation/notation/changing-defaults.itely View 1 2 3 2 chunks +33 lines, -36 lines 0 comments Download
M Documentation/notation/keyboards.itely View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M Documentation/notation/repeats.itely View 1 2 3 1 chunk +3 lines, -3 lines 0 comments Download
A input/regression/slur-height-capping.ly View 1 2 3 1 chunk +15 lines, -0 lines 2 comments Download
M lily/include/slur-score-parameters.hh View 1 1 chunk +1 line, -0 lines 0 comments Download
M lily/slur-configuration.cc View 1 2 3 3 chunks +13 lines, -2 lines 0 comments Download
M lily/slur-score-parameters.cc View 1 1 chunk +2 lines, -0 lines 0 comments Download
M mf/feta-noteheads.mf View 1 2 3 5 chunks +16 lines, -37 lines 0 comments Download
M scm/layout-slur.scm View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 20
MikeSol
This is much shorter than my previous work on this issue and is in the ...
7 years, 10 months ago (2011-08-05 00:20:18 UTC) #1
hanwenn
LGTM On Thu, Aug 4, 2011 at 9:20 PM, <mtsolo@gmail.com> wrote: > Instead of defining ...
7 years, 10 months ago (2011-08-05 01:50:15 UTC) #2
MikeSol
On 2011/08/05 01:50:15, hanwenn wrote: > LGTM > > On Thu, Aug 4, 2011 at ...
7 years, 10 months ago (2011-08-05 09:03:32 UTC) #3
hanwenn
LGTM http://codereview.appspot.com/4810072/diff/3002/input/regression/slur-height-capping.ly File input/regression/slur-height-capping.ly (right): http://codereview.appspot.com/4810072/diff/3002/input/regression/slur-height-capping.ly#newcode4 input/regression/slur-height-capping.ly:4: texidoc = "Lilypond ignores extra-collision-objects towards "slur shape ...
7 years, 10 months ago (2011-08-05 12:08:29 UTC) #4
MikeSol
http://codereview.appspot.com/4810072/diff/3002/input/regression/slur-height-capping.ly File input/regression/slur-height-capping.ly (right): http://codereview.appspot.com/4810072/diff/3002/input/regression/slur-height-capping.ly#newcode4 input/regression/slur-height-capping.ly:4: texidoc = "Lilypond ignores extra-collision-objects towards On 2011/08/05 12:08:29, ...
7 years, 10 months ago (2011-08-05 18:15:11 UTC) #5
Keith
I like this approach. http://codereview.appspot.com/4810072/diff/3002/lily/slur-configuration.cc File lily/slur-configuration.cc (right): http://codereview.appspot.com/4810072/diff/3002/lily/slur-configuration.cc#newcode102 lily/slur-configuration.cc:102: close_to_edge = close_to_edge || -d ...
7 years, 10 months ago (2011-08-06 08:04:18 UTC) #6
Janek Warchol
Hi Mike, i tried writing a review, but i don't understand what's going on here. ...
7 years, 10 months ago (2011-08-06 09:09:32 UTC) #7
mike_apollinemike.com
On Aug 6, 2011, at 11:09 AM, lemniskata.bernoullego@gmail.com wrote: > Hi Mike, > > i ...
7 years, 10 months ago (2011-08-06 15:37:12 UTC) #8
pkx166h
Passes make and one reg test difference http://code.google.com/p/lilypond/issues/detail?id=163#c23 see attached there.
7 years, 10 months ago (2011-08-06 21:04:51 UTC) #9
Janek Warchol
2011/8/6 mike@apollinemike.com <mike@apollinemike.com>: > On Aug 6, 2011, at 11:09 AM, lemniskata.bernoullego@gmail.com wrote: >> i ...
7 years, 10 months ago (2011-08-07 09:33:51 UTC) #10
mike_apollinemike.com
On Aug 7, 2011, at 11:33 AM, Janek Warchoł wrote: > 2011/8/6 mike@apollinemike.com <mike@apollinemike.com>: >> ...
7 years, 10 months ago (2011-08-07 09:46:59 UTC) #11
Janek Warchol
2011/8/7 mike@apollinemike.com <mike@apollinemike.com>: > On Aug 7, 2011, at 11:33 AM, Janek Warchoł wrote: > ...
7 years, 10 months ago (2011-08-07 13:00:24 UTC) #12
Graham Percival
On Sun, Aug 07, 2011 at 11:46:51AM +0200, mike@apollinemike.com wrote: > I'm certainly not against ...
7 years, 10 months ago (2011-08-07 19:02:09 UTC) #13
Keith
LGTM. I had time to try it on several scores; it often helped and never ...
7 years, 10 months ago (2011-08-08 03:52:52 UTC) #14
Graham Percival
On Mon, Aug 08, 2011 at 03:52:52AM +0000, k-ohara5a5a@oco.net wrote: > >If any of the ...
7 years, 10 months ago (2011-08-08 03:55:10 UTC) #15
MikeSol
http://codereview.appspot.com/4810072/diff/7001/input/regression/slur-height-capping.ly File input/regression/slur-height-capping.ly (right): http://codereview.appspot.com/4810072/diff/7001/input/regression/slur-height-capping.ly#newcode5 input/regression/slur-height-capping.ly:5: towards the edges of the slurs. said objects are ...
7 years, 10 months ago (2011-08-08 06:44:04 UTC) #16
Janek Warchol
2011/8/8 Graham Percival <graham@percival-music.ca>: > On Mon, Aug 08, 2011 at 03:52:52AM +0000, k-ohara5a5a@oco.net wrote: ...
7 years, 10 months ago (2011-08-08 08:07:51 UTC) #17
hanwenn
On Sun, Aug 7, 2011 at 4:02 PM, Graham Percival <graham@percival-music.ca> wrote: > On Sun, ...
7 years, 10 months ago (2011-08-08 13:30:42 UTC) #18
Neil Puttock
Needs a rebase. http://codereview.appspot.com/4810072/diff/1003/input/regression/slur-height-capping.ly File input/regression/slur-height-capping.ly (right): http://codereview.appspot.com/4810072/diff/1003/input/regression/slur-height-capping.ly#newcode4 input/regression/slur-height-capping.ly:4: texidoc = "Slur shaping is not ...
7 years, 10 months ago (2011-08-11 20:23:26 UTC) #19
MikeSol
7 years, 10 months ago (2011-08-11 23:04:09 UTC) #20
Pushed as 94ea10f3f341fff503599a9eb947c81f1803290f.

Cheers,
MS
Sign in to reply to this message.

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