lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Avoid failed assertion in stem tremolo code (issue 572550043 by address@


From: k-ohara5a5a
Subject: Avoid failed assertion in stem tremolo code (issue 572550043 by address@hidden)
Date: Wed, 03 Apr 2019 01:16:22 -0700

If I understand, the example
  \relative { a32 8..:32 }
fails the assertion in the call to center() on line 182, because the
interval 'ph' is empty.

There are several problems in the existing code, that would seem to
prevent it from working.  The proposed change deviates further from what
seems to be the intent of the code, so I suggest trapping for an empty
interval explicitly.


https://codereview.appspot.com/572550043/diff/560590043/lily/stem-tremolo.cc
File lily/stem-tremolo.cc (left):

https://codereview.appspot.com/572550043/diff/560590043/lily/stem-tremolo.cc#oldcode156
lily/stem-tremolo.cc:156: Stem_tremolo::pure_height (SCM smob, SCM, SCM)
Stem_tremolo::pure_height (SCM smob, SCM /*start*/, SCM /*end*/)
The duty of this function is to return the vertical extent of the
tremolo, for purposes of figuring the length required to set a line of
music that begins at column 'start' and ends at column 'end'.

https://codereview.appspot.com/572550043/diff/560590043/lily/stem-tremolo.cc#oldcode175
lily/stem-tremolo.cc:175: Interval ph = stem->pure_y_extent (stem, 0,
INT_MAX);
The code clearly assumed that 'ph' would never be empty.
In the given example, it certainly should not be empty, so there is a
problem in stem.cc.

If in future stem->pure_y_extent() is repaired, and validly returns an
empty interval, this function should behave as it does with no stem at
all:
  if (is_empty(ph)) return s1.extent(Y_AXIS);

https://codereview.appspot.com/572550043/diff/560590043/lily/stem-tremolo.cc#oldcode177
lily/stem-tremolo.cc:177: ph[-dir] = si.shortest_y_;
shortest_y is the endpoint of the stem (including space for tremolo)
when it is compressed as short as it can be.  So the operation above
would normally be a shortening of the interval.
Doing instead add_point() would defeat its purpose.

https://codereview.appspot.com/572550043/diff/560590043/lily/stem-tremolo.cc#oldcode182
lily/stem-tremolo.cc:182: ph = ph - ph.center ();
There is the obvious problem that line 182 destroys the effort of line
181.   It reports every stem-tremolo to be centered on the central
staff-line, which makes line-spacing less accurate but could easily go
un-noticed.

https://codereview.appspot.com/572550043/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]