Decrease space between vertical beams by length-fraction.

Decrease space between vertical beams by length-fraction.

This causes 16th and higher grace beams to be closer together.

Thanks to Daniel Spreadbury for noting this in blog,
http://blog.steinberg.net/2015/03/development-diary-part-10/

[hanwenn, 2015-03-14 14:51:55]

comment

[hanwenn, 2015-03-14 14:52:23]

another try

[hanwenn, 2015-03-14 14:53:37]

I don't think we have explicit test coverage for this, but not sure what it
would look like.

[hanwenn, 2015-03-24 11:37:26]

On 2015/03/14 14:53:37, hanwenn wrote:
> I don't think we have explicit test coverage for this, but not sure what it
> would look like.

Ping?

[dak, 2015-03-24 12:01:30]

https://codereview.appspot.com/214250043/diff/40001/lily/beam.cc
File lily/beam.cc (right):

https://codereview.appspot.com/214250043/diff/40001/lily/beam.cc#newcode127
lily/beam.cc:127: ? (2 * staff_space + line - beam_thickness/fract) / 2.0
Is there some point to writing the stuff in a manner where the result comes
about by a lot of cancellation?  Here we have
(2 * staff_space + line - beamthickness/fract) / 2.0 * fract
instead of either
(staff_space + line/2.0)*fract - beamthickness/2.0
or
staff_space*fract + (line*fract - beamthickness)/2.0
and I don't see that we get better results by all that cancellation.  Also it
becomes fragile against things like fract being zero.

[dak, 2015-03-24 12:02:06]

hanwenn@gmail.com writes:

> On 2015/03/14 14:53:37, hanwenn wrote:
>> I don't think we have explicit test coverage for this, but not sure
> what it
>> would look like.
>
> Ping?
>
> https://codereview.appspot.com/214250043/

Is there a corresponding Google code issue?

-- 
David Kastrup

[pkx166h, 2015-03-24 19:36:40]

On 2015/03/24 12:02:06, dak wrote:
> mailto:hanwenn@gmail.com writes:
> 
> > On 2015/03/14 14:53:37, hanwenn wrote:
> >> I don't think we have explicit test coverage for this, but not sure
> > what it
> >> would look like.
> >
> > Ping?
> >
> > https://codereview.appspot.com/214250043/
> 
> Is there a corresponding Google code issue?
> 
> -- 
> David Kastrup

There is now

https://code.google.com/p/lilypond/issues/detail?id=4329

James

[pkx166h, 2015-03-24 23:03:32]

Passes make, make check and a full make doc.

Reg test diffs here:

https://www.hightail.com/download/UlRRWGJIcVh5UkhIRHRVag

[lemzwerg, 2015-03-25 06:14:00]

Thanks for the test diffs.  Looking at the regression tests, I see some
degradations.

. The second beam in `grace-start' is too far offset from the staff.  Ditto for
grace-`stem-length' and others.  Note that I got similar glitches earlier too in
my real-life scores but was never be able to reduce them to a minimum example,
since it seems to be highly dependent on the surrounding grobs.  So maybe this
is another bug that just has become more visible here.

. Regarding `magnifyMusic-dots-beamlets' I wonder whether a minimum distance
between beam lines for a given beam thickness should be retained to avoid the
visual clash shown in the first beam.

. `quote-cue-event-types' now shows too short stems for the grace notes IMHO.

[hanwenn, 2015-04-06 22:20:24]

On 2015/03/25 06:14:00, lemzwerg wrote:
> Thanks for the test diffs.  Looking at the regression tests, I see some
> degradations.

the test diffs expired. Are they still available?

> . The second beam in `grace-start' is too far offset from the staff.  Ditto
for
> grace-`stem-length' and others.  Note that I got similar glitches earlier too
in
> my real-life scores but was never be able to reduce them to a minimum example,
> since it seems to be highly dependent on the surrounding grobs.  So maybe this
> is another bug that just has become more visible here.

I have been looking at this for a bit. The problem is that the flat symbol is
thought to have [-.38, 1.20] as extents, which brings it at distance of 0.2 to
the beam.  From visual inspection, it seems that the bounding box is a little
too large, which is confirmed by looking at feta20.dvi.

We could make the flat bbox tight around the glyph, but it wouldn't solve the
problem. We should also scale down the padding (which is set to 0.5 in the
details property).  The beam length-fraction is decreased by a factor 0.8, but
the font size for accidentals is decreased more strongly, by a factor 0.63
(magstep -4).

> . Regarding `magnifyMusic-dots-beamlets' I wonder whether a minimum distance
> between beam lines for a given beam thickness should be retained to avoid the
> visual clash shown in the first beam.
> 
> . `quote-cue-event-types' now shows too short stems for the grace notes IMHO.

[hanwenn, 2015-04-06 22:29:46]

fix regression in grace-start.ly

[hanwenn, 2015-04-06 22:30:03]

fix regression in grace-start.ly

[hanwenn, 2015-04-06 22:30:56]

fix regression in grace-start.ly

[hanwenn, 2015-04-06 22:33:18]

fix regression

[hanwenn, 2015-04-06 22:34:17]

On 2015/04/06 22:33:18, hanwenn wrote:
> fix regression

PTAL. I fixed the grace-start issue. Are the other ones serious enough that I
should look into them too?

[pkx166h, 2015-04-07 00:47:12]

On 2015/04/06 22:20:24, hanwenn wrote:
> On 2015/03/25 06:14:00, lemzwerg wrote:
> > Thanks for the test diffs.  Looking at the regression tests, I see some
> > degradations.
> 
> the test diffs expired. Are they still available?

No. I'll have to re-run the make check and update the issue tracker.

[pkx166h, 2015-04-07 01:36:56]

Hanwen,


Passes make, make check and a full make doc

Reg test diffs here

https://www.hightail.com/download/UlRRZUNxa0RCMTZybHNUQw

James

[lemzwerg, 2015-04-07 05:20:55]

Thanks again for the regtests.  All my previous comments still hold,
unfortunately.

[pkx166h, 2015-04-09 12:12:10]

This patch has been put back to 'needs work' based on Werner's comments above.

[hanwenn, 2015-05-10 22:49:27]

fix stem length regressions

[hanwenn, 2015-05-10 22:58:54]

also handle length-fraction > 1

[hanwenn, 2015-05-10 23:16:14]

On 2015/05/10 22:58:54, hanwenn wrote:
> also handle length-fraction > 1

PTAL

* I had a further look at the short stems, and should have fixed more cases by
disabling forbidden beam quanting in case of grace notes.

* magnifyMusic: can you show examples of what music at 50% scale should look
like in print? I agree that the blotting may not be very desirable, but then
again magnifying music at 50% is fairly uncommon. It seems that you are asking
for a separate beam-gap-fraction variable? 

* quote-cue-event is actually fixed by this change: CueVoice specifies magstep
-4 for stemlength, ie. 63%. That puts the ideal stem length for a beam at 2.205.
The stems shown in the new image are very close to that.  If you think CueVoice
should use larger stems, that is a separate feature request IMO.

[lemzwerg, 2015-05-12 05:10:51]

> * I had a further look at the short stems, and should have fixed
>   more cases by disabling forbidden beam quanting in case of grace
>   notes.

Yes, everything looks OK now, thanks.

> * magnifyMusic: can you show examples of what music at 50% scale
>   should look like in print?

No, I can't.  My observation was not targeted at anything particular.

> It seems that you are asking for a separate beam-gap-fraction
> variable?

No, I don't :-) To be serious: I suggest working on such a feature only in case
someone *really* needs it.

[pkx166h, 2015-05-12 10:51:30]

Patch on countdown for May 16th

[pkx166h, 2015-05-16 06:41:45]

Patch counted down - please push to Staging branch

[hanwenn, 2015-05-17 09:09:59]

should I do this myself?

On Sat, May 16, 2015 at 8:41 AM,  <pkx166h@gmail.com> wrote:
> Patch counted down - please push to Staging branch
>
> https://codereview.appspot.com/214250043/



-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

[pkx166h, 2015-05-17 09:32:06]

On 2015/05/17 09:09:59, hanwenn wrote:
> should I do this myself?
> 
> On Sat, May 16, 2015 at 8:41 AM,  <mailto:pkx166h@gmail.com> wrote:
> > Patch counted down - please push to Staging branch
> >
> > https://codereview.appspot.com/214250043/
> 
> 
> 
> -- 
> Han-Wen Nienhuys - mailto:hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

Sure. I assume you have push permissions ;)

James

[pkx166h, 2015-05-22 06:57:33]

Please push to staging (you'll need to rebase by now though)

[pkx166h, 2015-05-25 11:48:58]

Patch counted down - please push

[hanwenn, 2015-05-26 07:24:14]

pushed to staging.

On Mon, May 25, 2015 at 1:48 PM,  <pkx166h@gmail.com> wrote:
> Patch counted down - please push
>
> https://codereview.appspot.com/214250043/



-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen