Sets TabVoice Stem height to ##f

Sets TabVoice Stem height to ##f

[MikeSol, 2012-06-10 18:22:50]

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.

[dak, 2012-06-10 18:32:58]

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 resulting height explicitly.

[Keith, 2012-06-11 03:44:32]

http://codereview.appspot.com/6303065/diff/4001/lily/stem.cc
File lily/stem.cc (right):

http://codereview.appspot.com/6303065/diff/4001/lily/stem.cc#newcode703
lily/stem.cc:703: if (calc_beam && !unsmob_stencil (me->get_property
("stencil")))
beam-stem-stemlet.ly shows that LilyPond produces no stencil for the invisible
stems on beamed rests, yet she depends on stem extents for tuplet brackets. 
Maybe
  if (calc_beam && !beam && !stenxil) ?

[MikeSol, 2012-06-11 05:34:23]

On 2012/06/11 03:44:32, Keith wrote:
> http://codereview.appspot.com/6303065/diff/4001/lily/stem.cc
> File lily/stem.cc (right):
> 
> http://codereview.appspot.com/6303065/diff/4001/lily/stem.cc#newcode703
> lily/stem.cc:703: if (calc_beam && !unsmob_stencil (me->get_property
> ("stencil")))
> beam-stem-stemlet.ly shows that LilyPond produces no stencil for the invisible
> stems on beamed rests, yet she depends on stem extents for tuplet brackets. 
> Maybe
>   if (calc_beam && !beam && !stenxil) ?

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.  I think that, even though this requires redundancy
(specifying no stencil and an empty height), if LilyPond treats cases where
there is no stencil and a non-empty height (like beam-stem-stemlet.ly), then the
method in the first patch set is better.

[dak, 2012-06-11 07:10:48]

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.  I think that, even though this requires
redundancy
> (specifying no stencil and an empty height), if LilyPond treats cases where
> there is no stencil and a non-empty height (like beam-stem-stemlet.ly), then
the
> method in the first patch set is better.

What do you mean with "treats cases"?  That it implements some useful behavior
based on the exact given height?  Or merely that it catches the case?

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, then setting the stencil to ##f is the wrong measure: it should be a
point-stencil instead.

[MikeSol, 2012-06-11 07:24:33]

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 must be made
> > explicit through overrides.  I think that, even though this requires
> redundancy
> > (specifying no stencil and an empty height), if LilyPond treats cases where
> > there is no stencil and a non-empty height (like beam-stem-stemlet.ly), then
> the
> > method in the first patch set is better.
> 
> What do you mean with "treats cases"?  That it implements some useful behavior
> based on the exact given height?  Or merely that it catches the case?
> 
> 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, then setting the stencil to ##f is the wrong measure: it should be a
> point-stencil instead.

So when stencil is #f for stems, the height should always be empty?  In this
case, patch set 2 was the correct approach, and the fact that it failed regtests
reveals a hidden bug in the handling of beamed rests under tuplets.

If you think this is the best way to tackle it, then I'll do that.  It'll likely
mean a larger patch as it'll unearth some bugs that didn't surface with the stem
work in 2.15.

[dak, 2012-06-11 07:48:34]

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, then setting the stencil to ##f is the wrong measure: it should be a
> > point-stencil instead.
> 
> So when stencil is #f for stems, the height should always be empty?  In this
> case, patch set 2 was the correct approach, and the fact that it failed
regtests
> reveals a hidden bug in the handling of beamed rests under tuplets.

Not necessarily a hidden bug, but rather a hidden expectation.  If LilyPond
will, when setting the stencil to #f for some reason of its own, accompany this
with consistent height data, then the behavior might be consistent and not
exhibit problems.

Passing information with that sort of secret contract, however, is a bad idea
when users and/or other programmers are expected to be able to unstencil grobs. 
In that case, passing information in the height (or otherwise manipulating it)
without taking the nonexistence of the stencil into consideration is a rather
bad idea.

> If you think this is the best way to tackle it, then I'll do that.  It'll
likely
> mean a larger patch as it'll unearth some bugs that didn't surface with the
stem
> work in 2.15.

As I said: these may not necessarily bugs but rather larvae: things that are
rather certain to grow into bugs under reasonably natural conditions.

I think it would be worth the trouble to get rid of them.  In case we have code
that relies on passing information through the Y-height of a stencil even when
the stencil has been set to #f by the user, this will require finding a
different solution.

[MikeSol, 2012-06-11 11:46:31]

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 really required to have a non-zero
> > > height, then setting the stencil to ##f is the wrong measure: it should be
a
> > > point-stencil instead.
> > 
> > So when stencil is #f for stems, the height should always be empty?  In this
> > case, patch set 2 was the correct approach, and the fact that it failed
> regtests
> > reveals a hidden bug in the handling of beamed rests under tuplets.
> 
> Not necessarily a hidden bug, but rather a hidden expectation.  If LilyPond
> will, when setting the stencil to #f for some reason of its own, accompany
this
> with consistent height data, then the behavior might be consistent and not
> exhibit problems.
> 
> Passing information with that sort of secret contract, however, is a bad idea
> when users and/or other programmers are expected to be able to unstencil
grobs. 
> In that case, passing information in the height (or otherwise manipulating it)
> without taking the nonexistence of the stencil into consideration is a rather
> bad idea.
> 
> > If you think this is the best way to tackle it, then I'll do that.  It'll
> likely
> > mean a larger patch as it'll unearth some bugs that didn't surface with the
> stem
> > work in 2.15.
> 
> As I said: these may not necessarily bugs but rather larvae: things that are
> rather certain to grow into bugs under reasonably natural conditions.
> 
> I think it would be worth the trouble to get rid of them.  In case we have
code
> that relies on passing information through the Y-height of a stencil even when
> the stencil has been set to #f by the user, this will require finding a
> different solution.

My compromise solution in the most recent patch set is to use Keith's idea but
to leave a comment so that people know that beam, stencil-less stems are treated
as having a height of 0 at the point of the beam whereas unbeamed stems are
heightless.  The former is the case because otherwise the beam doesn't know
where to go - it seems like a reasonable exception.

[Keith, 2012-06-11 16:33:27]

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 exactly match the code (we the full stem interval for the
imaginary stems on beamed rests, not just a point) so take another read before
you push.  Maybe let the code show what you do, and just say why you do it.  "If
there is a beam but no stem, slope calculations depend on this routine to return
where the stem end /would/ be."

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")))
If you have time to test the output, consider removing the test for calc_beam,
and combining with similar line 692.

[dak, 2012-06-12 12:32:37]

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 adding another nan or an infinite offset with different sign.

What case is this supposed to catch?

[MikeSol, 2012-06-12 12:39:23]

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 from adding an offset to
> infinity is by adding another nan or an infinite offset with different sign.
> 
> What case is this supposed to catch?

Sometimes there is an infinite offset (either positive or negative) which, when
added to an empty interval of (+inf.0 . -inf.0), will create nan (or -nan) in
one of the two directions.

[dak, 2012-06-12 12:49:45]

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 adding an offset to
> infinity is by adding another nan or an infinite offset with different sign.
> 
> What case is this supposed to catch?

So what you actually meant to say was
  if (!real_ext.is_empty ())
    real_ext.translate (offset);

If that's what you mean, why don't you write it instead of some puzzle?

And frankly, shouldn't we rather put this into flower/include/interval.hh
instead of trying to catch this in arbitrary places in our code?

[MikeSol, 2012-06-12 12:54:40]

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 this.  The only way to get a nan from adding an offset to
> > infinity is by adding another nan or an infinite offset with different sign.
> > 
> > What case is this supposed to catch?
> 
> So what you actually meant to say was
>   if (!real_ext.is_empty ())
>     real_ext.translate (offset);
> 
> If that's what you mean, why don't you write it instead of some puzzle?

This is not what I mean.  An empty interval is, in LilyPond, an interval whose
left is greater than it's right.  So (3 . 2) is empty, but (3 . 2) can be
translated by an infinite value in either direction and not result in nan.  So
just checking for emptiness isn't enough.

An infinite value in an interval should not be translated by another infinite
value.  It either does nothing (if they both have the same sign) or results in a
nan.

> And frankly, shouldn't we rather put this into flower/include/interval.hh
> instead of trying to catch this in arbitrary places in our code?

I had toyed with this idea - this is a bit out of my league, as I don't know if
LilyPond will take a performance hit if we add extra operations to something as
elemental as translate or is_empty.  If this is appropriate, then the check can
go there.

[dak, 2012-06-12 13:22:10]

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 2012/06/12 12:32:37, dak wrote:
> > > I don't understand this.  The only way to get a nan from adding an offset
to
> > > infinity is by adding another nan or an infinite offset with different
sign.
> > > 
> > > What case is this supposed to catch?
> > 
> > So what you actually meant to say was
> >   if (!real_ext.is_empty ())
> >     real_ext.translate (offset);
> > 
> > If that's what you mean, why don't you write it instead of some puzzle?
> 
> This is not what I mean.  An empty interval is, in LilyPond, an interval whose
> left is greater than it's right.

I disagree.  An empty interval is an interval not containing any point.  The
details are not important.

> So (3 . 2) is empty, but (3 . 2) can be
> translated by an infinite value in either direction and not result in nan.

What meaning is there in "translating" an "empty" interval and afterwards
getting an interval no longer being empty since its lower bound no longer is
smaller than its upper bound?

Are you, by chance, confusing an _interval_ with a tuple of points?  You can
_implement_ a closed interval as a tuple of points, but that does not lend
meaning to "shifting an empty interval".

> So just checking for emptiness isn't enough.

For mimicking all side effects of the current implementation, it isn't.  But if
some code relies on those side effects, it is broken in my opinion.  If you want
to manipulate a two-dimensional vector without inherent relation between its two
elements, use a Drul, not an Interval.

> > And frankly, shouldn't we rather put this into flower/include/interval.hh
> > instead of trying to catch this in arbitrary places in our code?
> 
> I had toyed with this idea - this is a bit out of my league, as I don't know
if
> LilyPond will take a performance hit if we add extra operations to something
as
> elemental as translate or is_empty.

Every piece of inherent safety takes a performance hit.  Personally I think that
the only case where we have is_empty true is for (+inf, -inf).  And I don't mean
that we should change the test: the test is fine.  There just isn't a
combination of interval bounds that makes more sense.  We could equally well
define the empty interval as (0, -1) and get consistent behavior from that.  But
while the interval is empty, its bounds should not be interpreted as conveying
meaning.

[MikeSol, 2012-06-12 13:27:23]

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
> > > 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 adding an
offset
> to
> > > > infinity is by adding another nan or an infinite offset with different
> sign.
> > > > 
> > > > What case is this supposed to catch?
> > > 
> > > So what you actually meant to say was
> > >   if (!real_ext.is_empty ())
> > >     real_ext.translate (offset);
> > > 
> > > If that's what you mean, why don't you write it instead of some puzzle?
> > 
> > This is not what I mean.  An empty interval is, in LilyPond, an interval
whose
> > left is greater than it's right.
> 
> I disagree.  An empty interval is an interval not containing any point.  The
> details are not important.
> 
> > So (3 . 2) is empty, but (3 . 2) can be
> > translated by an infinite value in either direction and not result in nan.
> 
> What meaning is there in "translating" an "empty" interval and afterwards
> getting an interval no longer being empty since its lower bound no longer is
> smaller than its upper bound?
> 
> Are you, by chance, confusing an _interval_ with a tuple of points?  You can
> _implement_ a closed interval as a tuple of points, but that does not lend
> meaning to "shifting an empty interval".
> 
> > So just checking for emptiness isn't enough.
> 
> For mimicking all side effects of the current implementation, it isn't.  But
if
> some code relies on those side effects, it is broken in my opinion.  If you
want
> to manipulate a two-dimensional vector without inherent relation between its
two
> elements, use a Drul, not an Interval.
> 
> > > And frankly, shouldn't we rather put this into flower/include/interval.hh
> > > instead of trying to catch this in arbitrary places in our code?
> > 
> > I had toyed with this idea - this is a bit out of my league, as I don't know
> if
> > LilyPond will take a performance hit if we add extra operations to something
> as
> > elemental as translate or is_empty.
> 
> Every piece of inherent safety takes a performance hit.  Personally I think
that
> the only case where we have is_empty true is for (+inf, -inf).  And I don't
mean
> that we should change the test: the test is fine.  There just isn't a
> combination of interval bounds that makes more sense.  We could equally well
> define the empty interval as (0, -1) and get consistent behavior from that. 
But
> while the interval is empty, its bounds should not be interpreted as conveying
> meaning.

If all empty intervals are the same then, by design, we need to make sure that
all of them equal (+inf.0 . -inf.0).  This means that:

--) Interval should have a method set_to_empty () that sets it to (+inf.0 .
-inf.0) (maybe it already does).
--) Anytime an interval is set to its left being greater than its right such
that it is not infinitely empty, a programming error should be issued.
--) Perhaps all math done on empty intervals should raise programming errors.

I'm all for making the code more coherent.  It sounds like we're talking about a
much deeper problem than this patch, and perhaps it's wiser to come up w/ a
solution to that first before pushing this.

Cheers,
MS

[dak, 2012-06-12 13:43:09]

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 is the
point in type abstraction if one creates total conceptual chaos without inherent
relation to the invariants of the mathematical model underlying the type?

swap never should have been a public member function (it is used internally when
mirroring intervals).

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 than more,
since you always need to take all side-effects into consideration.

[MikeSol, 2012-06-12 13:47:34]

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 an interval does not even make _sense_.  Where is the
> point in type abstraction if one creates total conceptual chaos without
inherent
> relation to the invariants of the mathematical model underlying the type?
> 
> swap never should have been a public member function (it is used internally
when
> mirroring intervals).
> 
> 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 than more,
> since you always need to take all side-effects into consideration.

Dunno - it sounds like time for clean up.

Y-position of beams were stored as intervals for years (they may still be - I
forget).

This sounds like a pretty major task, so as always, I'd touch base w/ Han-Wen to
see what Intervals were supposed to be at their inception and then evaluate what
they've grown into.

We can then firm up an Interval API and write a strongly-worded comment in
interval.hh and interval.tcc NOT to touch either of these files.

[dak, 2012-06-12 14:09:35]

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 than more,
> > since you always need to take all side-effects into consideration.
> 
> Dunno - it sounds like time for clean up.

Maybe.  It does not make sense in my opinion to pepper the whole code with
fixups intended to cater for cases where intervals are supposed to be used as
non-intervals, or with fixups in order to cater for cases where intervals are
supposed to be used as intervals, and we can't put the respective fixes into the
interval implementation itself since it would violate the assumptions of those
uses where intervals are used as non-intervals.

> Y-position of beams were stored as intervals for years (they may still be - I
> forget).

Storing Y-positions of beams in intervals is fine when we are talking about a
possible set of definitions for Y-positions at one point.  When we are talking
about "lower interval bound is left Y-position, higher interval bound is right
Y-position, and left Y-position may be higher than right Y-position", we are
venturing into the domain of nonsense.

> This sounds like a pretty major task, so as always, I'd touch base w/ Han-Wen
to
> see what Intervals were supposed to be at their inception and then evaluate
what
> they've grown into.
> 
> We can then firm up an Interval API and write a strongly-worded comment in
> interval.hh and interval.tcc NOT to touch either of these files.

Sounds like it would make sense.  And _if_ we have use cases for ordered
point/value pairs (like using Linear_combination or whatever), then we should
make sure that those are available on _appropriate_ data types as well rather
than misusing Intervals for them because their internals happen to be convenient
for that.

[dak, 2012-06-15 07:33:31]

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
> > > 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 adding an
offset
> to
> > > > infinity is by adding another nan or an infinite offset with different
> sign.
> > > > 
> > > > What case is this supposed to catch?
> > > 
> > > So what you actually meant to say was
> > >   if (!real_ext.is_empty ())
> > >     real_ext.translate (offset);
> > > 
> > > If that's what you mean, why don't you write it instead of some puzzle?
> > 
> > This is not what I mean.  An empty interval is, in LilyPond, an interval
whose
> > left is greater than it's right.
> 
> I disagree.  An empty interval is an interval not containing any point.  The
> details are not important.

I see that you chose to push this unchanged instead of using my
suggestion.  The result is that both interval ends are translated
_independently_.

Treating them _independently_ in this obfuscate manner only makes
sense when you expect them to be set independently, namely one
interval end being infinite, the other finite.

For one thing, you have not explained how you expect such a
configuration to come about.  For another it means that
(inf,3)
which is an empty interval, gets transformed under an infinite shift
to
(inf,inf)
an interval that will likely blow the emptiness check.

Your example,
(3,2)
is transformed with an infinite shift into
(inf,inf)
again blowing the emptiness check since inf<inf can't be determined.

Could you please explain the cases where you expect to get sensible
results with that approach?

[mike_apollinemike.com, 2012-06-15 16:36:45]

On 15 juin 2012, at 09:33, dak@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://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 adding
> an offset
>> to
>> > > > infinity is by adding another nan or an infinite offset with
> different
>> sign.
>> > > >
>> > > > What case is this supposed to catch?
>> > >
>> > > So what you actually meant to say was
>> > >   if (!real_ext.is_empty ())
>> > >     real_ext.translate (offset);
>> > >
>> > > If that's what you mean, why don't you write it instead of some
> puzzle?
>> >
>> > This is not what I mean.  An empty interval is, in LilyPond, an
> interval whose
>> > left is greater than it's right.
> 
>> I disagree.  An empty interval is an interval not containing any
> point.  The
>> details are not important.
> 
> I see that you chose to push this unchanged instead of using my
> suggestion.  The result is that both interval ends are translated
> _independently_.
> 
> Treating them _independently_ in this obfuscate manner only makes
> sense when you expect them to be set independently, namely one
> interval end being infinite, the other finite.
> 
> For one thing, you have not explained how you expect such a
> configuration to come about.  For another it means that
> (inf,3)
> which is an empty interval, gets transformed under an infinite shift
> to
> (inf,inf)
> an interval that will likely blow the emptiness check.
> 
> Your example,
> (3,2)
> is transformed with an infinite shift into
> (inf,inf)
> again blowing the emptiness check since inf<inf can't be determined.
> 
> Could you please explain the cases where you expect to get sensible
> results with that approach?
> 

Hey,

Sorry - I've been swamped the past few days w/ concerts and forgot about your
suggestion.  I'll take some time this weekend or next week to think it through
and let you know.

Cheers,
MS

[Keith, 2012-07-14 07:52:16]

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 have time to test the output, consider removing the test for
calc_beam,

Bad suggestion, apparently, given that it disconnects the stems form merged
triangle note-heads.  I'll put back the test for calc_beam after I get through
regression testing.