Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-09-16 Thread Keith OHara
mike apollinemike.com apollinemike.com> writes: > >> > On 2012/06/12 12:49:45, dak wrote: > >> > > http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc > >> > > File lily/grob.cc (right): > >> > > > >> > > > > http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc#newcode472 > >>

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-07-14 Thread k-ohara5a5a
http://codereview.appspot.com/6303065/diff/9001/lily/stem.cc File lily/stem.cc (right): http://codereview.appspot.com/6303065/diff/9001/lily/stem.cc#newcode710 lily/stem.cc:710: if (calc_beam && !beam && !unsmob_stencil (me->get_property ("stencil"))) On 2012/06/11 16:33:27, Keith wrote: If you

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-15 Thread m...@apollinemike.com
On 15 juin 2012, at 09:33, d...@gnu.org wrote: > On 2012/06/12 13:22:10, dak wrote: >> On 2012/06/12 12:54:40, MikeSol wrote: >> > On 2012/06/12 12:49:45, dak wrote: >> > > http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc >> > > File lily/grob.cc (right): >> > > >> > > > http://coder

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-15 Thread dak
On 2012/06/12 13:22:10, dak wrote: On 2012/06/12 12:54:40, MikeSol wrote: > On 2012/06/12 12:49:45, dak wrote: > > http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc > > File lily/grob.cc (right): > > > > http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc#newcode472 > > li

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-12 Thread dak
On 2012/06/12 13:47:34, MikeSol wrote: On 2012/06/12 13:43:09, dak wrote: > If a type is named "Interval", it needs to be employed as an Interval, not as > something totally different that relies on implementation details. > > Otherwise type abstraction makes code _less_ maintainable rather

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-12 Thread mtsolo
On 2012/06/12 13:43:09, dak wrote: Here is a code example from beam.cc: Interval weights (1 - multiplier, multiplier); if (feather_dir != LEFT) weights.swap (); This is _hogwash_. weights is not an _interval_ here, but a pair of numbers. Swapping the bounds of a

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-12 Thread dak
Here is a code example from beam.cc: Interval weights (1 - multiplier, multiplier); if (feather_dir != LEFT) weights.swap (); This is _hogwash_. weights is not an _interval_ here, but a pair of numbers. Swapping the bounds of an interval does not even make _sense_. Where

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-12 Thread mtsolo
On 2012/06/12 13:22:10, dak wrote: On 2012/06/12 12:54:40, MikeSol wrote: > On 2012/06/12 12:49:45, dak wrote: > > http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc > > File lily/grob.cc (right): > > > > http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc#newcode472 > > li

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-12 Thread dak
On 2012/06/12 12:54:40, MikeSol wrote: On 2012/06/12 12:49:45, dak wrote: > http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc > File lily/grob.cc (right): > > http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc#newcode472 > lily/grob.cc:472: real_ext[d] += offset; > On 201

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-12 Thread mtsolo
On 2012/06/12 12:49:45, dak wrote: http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc#newcode472 lily/grob.cc:472: real_ext[d] += offset; On 2012/06/12 12:32:37, dak wrote: > I don't understand

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-12 Thread dak
http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc#newcode472 lily/grob.cc:472: real_ext[d] += offset; On 2012/06/12 12:32:37, dak wrote: I don't understand this. The only way to get a nan from

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-12 Thread mtsolo
On 2012/06/12 12:32:37, dak wrote: http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc#newcode472 lily/grob.cc:472: real_ext[d] += offset; I don't understand this. The only way to get a nan fro

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-12 Thread dak
http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc#newcode472 lily/grob.cc:472: real_ext[d] += offset; I don't understand this. The only way to get a nan from adding an offset to infinity is by a

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-11 Thread k-ohara5a5a
LGTM if you re-read the comment before you push. http://codereview.appspot.com/6303065/diff/9001/lily/stem.cc File lily/stem.cc (right): http://codereview.appspot.com/6303065/diff/9001/lily/stem.cc#newcode707 lily/stem.cc:707: return an empty interval when there is no beam. The comment doesn't

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-11 Thread mtsolo
On 2012/06/11 07:48:34, dak wrote: On 2012/06/11 07:24:33, MikeSol wrote: > On 2012/06/11 07:10:48, dak wrote: > > > Personally, I consider it an accident waiting to happen if downing the stencil > > is not enough to suppress its consideration. > > > > _Iff_ there is a situation where it is rea

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-11 Thread dak
On 2012/06/11 07:24:33, MikeSol wrote: On 2012/06/11 07:10:48, dak wrote: > Personally, I consider it an accident waiting to happen if downing the stencil > is not enough to suppress its consideration. > > _Iff_ there is a situation where it is really required to have a non-zero > height,

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-11 Thread mtsolo
On 2012/06/11 07:10:48, dak wrote: On 2012/06/11 05:34:23, MikeSol wrote: > This shows a case where stem height cannot be determined by stem stencil > presence. The first version of the patch works under the assumption that > information about height cannot be gleaned from the stencil and

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-11 Thread dak
On 2012/06/11 05:34:23, MikeSol wrote: This shows a case where stem height cannot be determined by stem stencil presence. The first version of the patch works under the assumption that information about height cannot be gleaned from the stencil and must be made explicit through overrides.

Re: Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-10 Thread dak
On 2012/06/10 18:22:50, MikeSol wrote: Another way of going about this would be changing the Stem::height function so that it returned an empty interval for stencil-less stems. I'd consider that eminently reasonable. It makes much more sense to me than having to wipe out some non-sensical re

Sets TabVoice Stem height to ##f (issue 6303065)

2012-06-10 Thread mtsolo
Reviewers: , Message: Another way of going about this would be changing the Stem::height function so that it returned an empty interval for stencil-less stems. Then the overrides wouldn't be necessary. It's a design question more than anything else. Description: Sets TabVoice Stem height to ##f