LGTM
http://codereview.appspot.com/6503057/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
On Sat, 08 Jan 2011 06:19:16 -0800, Carl Sorensen wrote:
On Fri, 07 Jan 2011 15:50:40 -0800, Carl Sorensen wrote:
A default value of skyline-horizontal-padding of 1.2 gets
skyline-horizontal-padding.ly to work well.
An override to System 'skyline-horizontal-padding = #0 in
stem-length-estima
On 1/7/11 9:16 PM, "Keith OHara" wrote:
> On Fri, 07 Jan 2011 15:50:40 -0800, Carl Sorensen wrote:
>>
>> A default value of skyline-horizontal-padding of 1.2 gets
>> skyline-horizontal-padding.ly to work well.
>>
>> An override to System 'skyline-horizontal-padding = #0 in
>> stem-length-estim
"Keith OHara" writes:
> Original Message
>> From: "Carl Sorensen"
>> Sent: Friday, January 07, 2011 3:28 PM
>
>>
>> I was hoping that by setting the default, we'd get good spacing and we
>> wouldn't need the override.
>>
>
> I hadn't seen this exchange when I commented on the
On Fri, 07 Jan 2011 15:50:40 -0800, Carl Sorensen wrote:
A default value of skyline-horizontal-padding of 1.2 gets
skyline-horizontal-padding.ly to work well.
An override to System 'skyline-horizontal-padding = #0 in
stem-length-estimation.ly gets it working well. Since we already have
overri
On 1/7/11 4:28 PM, "Carl Sorensen" wrote:
> On 1/7/11 2:41 PM, "n.putt...@gmail.com" wrote:
>
>> Hi Carl,
>>
>> Do we have to set a default for skyline-horizontal-padding? It has a
>> detrimental effect on some of the regtests (particularly
>> stem-length-estimation.ly).
>
> I was hoping tha
Original Message
> From: "Carl Sorensen"
> Sent: Friday, January 07, 2011 3:28 PM
>
> I was hoping that by setting the default, we'd get good spacing and we
> wouldn't need the override.
>
I hadn't seen this exchange when I commented on the define-grobs.scm;
sorry.
The larg
On 2011/01/07 21:41:51, Neil Puttock wrote:
Hi Carl,
Do we have to set a default for skyline-horizontal-padding? It has a
detrimental effect on some of the regtests (particularly
stem-length-estimation.ly).
I've set the default to 0.5, in accordance with Keith's suggestions. It
leaves stem
Thanks for the feedback. I've responded to each of the suggestions
you've given me.
Carl
http://codereview.appspot.com/3832046/diff/27001/input/regression/skyline-horizontal-padding.ly
File input/regression/skyline-horizontal-padding.ly (right):
http://codereview.appspot.com/3832046/diff/270
http://codereview.appspot.com/3832046/diff/27001/scm/define-grobs.scm
File scm/define-grobs.scm (right):
http://codereview.appspot.com/3832046/diff/27001/scm/define-grobs.scm#newcode1940
scm/define-grobs.scm:1940: (skyline-horizontal-padding . 2)
too big for a default. I suggest 0.5.
A horizon
On 1/7/11 2:41 PM, "n.putt...@gmail.com" wrote:
> Hi Carl,
>
> Do we have to set a default for skyline-horizontal-padding? It has a
> detrimental effect on some of the regtests (particularly
> stem-length-estimation.ly).
Well, no, we don't. However, the original bug says that we're crowding
s
http://codereview.appspot.com/3832046/diff/27001/input/regression/skyline-horizontal-padding.ly
File input/regression/skyline-horizontal-padding.ly (right):
http://codereview.appspot.com/3832046/diff/27001/input/regression/skyline-horizontal-padding.ly#newcode12
input/regression/skyline-horizont
Hi Carl,
Do we have to set a default for skyline-horizontal-padding? It has a
detrimental effect on some of the regtests (particularly
stem-length-estimation.ly).
Cheers,
Neil
http://codereview.appspot.com/3832046/
___
lilypond-devel mailing list
li
On 1/7/11 9:47 AM, "joenee...@gmail.com" wrote:
>
>
> http://codereview.appspot.com/3832046/diff/2001/lily/skyline.cc
> File lily/skyline.cc (right):
>
> http://codereview.appspot.com/3832046/diff/2001/lily/skyline.cc#newcode396
> lily/skyline.cc:396: if ((i->slope_ == 0) && !isinf (i->y_in
http://codereview.appspot.com/3832046/diff/2001/lily/skyline.cc
File lily/skyline.cc (right):
http://codereview.appspot.com/3832046/diff/2001/lily/skyline.cc#newcode396
lily/skyline.cc:396: if ((i->slope_ == 0) && !isinf (i->y_intercept_) &&
i->y_intercept_ >= 0)
On 2011/01/03 03:48:09, Carl wro
I've added comments about the need for the horizontal-padding in the
distance call to be used with System grobs, and I've moved the
skyline-horizontal-padding from an override to a default value for the
System grob.
I think this should make everything work right out of the box now,
without
requir
LGTM, and it builds everything fine from scratch.
http://codereview.appspot.com/3832046/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
On 1/4/11 6:45 PM, "k-ohara5...@oco.net" wrote:
> I tried to break it; it did not break. I say push it.
Thanks for trying! And thanks for your help in identifying the right place
to put in the call to the new function.
>
> On 2011/01/02 05:19:57, joeneeman wrote:
>> I don't know if it's impor
I tried to break it; it did not break. I say push it.
On 2011/01/02 05:19:57, joeneeman wrote:
I don't know if it's important, but it may be worth mentioning
somewhere that
skyline-horizontal-padding works differently for VerticalAxisGroup and
System.
The existing docs are good enough, in th
Thanks, Keith for identifying the problem with bar numbers at the
beginning of the line.
I've fixed that problem, and I'm more confident in the logic of the box
extraction and padded skyline creation.
I've modified the regression test so it has 3 systems, and thus would
catch the problem that Ke
On 2011/01/04 01:46:45, Keith wrote:
For a 2-staff system
(with non-protruding bass clefs to make the math easier) the patch
computes
minimum_distance
3.05, 7.33, 29.41, 29.38
as it adds four systems to a page.
Can you send me a test file so I can check it out?
Thanks,
Carl
http://codere
The new patch fixes the issue, but we can't yet use it in larger scores,
if we want to do that.
Whenever there is more than one staff in a system, and any non-zero
skyline-horizontal-padding, minimum_distance is computed reasonably for
the first and second systems, but not the third and later. F
I've made the changes, and now the patch actually works.
Thanks all for your comments!
Carl
http://codereview.appspot.com/3832046/diff/2001/lily/page-layout-problem.cc
File lily/page-layout-problem.cc (right):
http://codereview.appspot.com/3832046/diff/2001/lily/page-layout-problem.cc#newcod
Thanks for all the comments.
Keith has identified the correct place to put include the system
horizontal padding, and it now works properly.
New patch set coming shortly.
Carl
http://codereview.appspot.com/3832046/diff/2001/input/regression/skyline-horizontal-padding.ly
File input/regression
It is interesting to see what gets horizontally padded and what does not
-- accidentals do not, for example.
Whether by accident or design, the choices on what to pad seem
reasonable.
It seems that the same padded skyline is used both for page layout, and
for drawing the outer skylines with debug
Yippee for the patch.
Doesn't quite work yet, probably because the function that uses your new
code, append_prob, is not used in the situation where we need the extra
padding. append_system is called. (Confirmed by printf-debugging using
your and my .ly test files)
http://codereview.appspot.com
http://codereview.appspot.com/3832046/diff/2001/input/regression/skyline-horizontal-padding.ly
File input/regression/skyline-horizontal-padding.ly (right):
http://codereview.appspot.com/3832046/diff/2001/input/regression/skyline-horizontal-padding.ly#newcode13
input/regression/skyline-horizontal
I don't know if it's important, but it may be worth mentioning somewhere
that skyline-horizontal-padding works differently for VerticalAxisGroup
and System. (Because in VerticalAxisGroup, skyline-horizontal-padding
takes effect while the outside-staff-grobs are being placed).
http://codereview.a
Reviewers: ,
Message:
Here is a patch to fix issue 1290.
It works, but it may need to be cleaned up. I'm not sure the code is as
elegant as it could be. I'm not really comfortable with all of the C++
syntax used in lilypond.
Please review it carefully, and let me know how it can b
I have a patch for Issue 1290 that fixes the improper spacing.
I am currently doing a regression test.
Thanks,
Carl
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
30 matches
Mail list logo
