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

[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 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.

http://codereview.appspot.com/6303065/