Reviewers: thomasmorley651, Message: On 2020/03/01 14:37:19, thomasmorley651 wrote: > I've applied your patch and did some testings. > > It indeed solves the code given with boom.ly at > https://sourceforge.net/p/testlilyissues/issues/5801/ > > Though, I posted two minimals there. > While the first one now compiles fine, the second still crashes with: > > GNU LilyPond 2.21.0 > Processing `atest-99.ly' > Parsing... > Interpreting music...[8][16][24][24] > Preprocessing graphical objects... > Calculating page and line breaks (1 possible page breaks)...[1] > Drawing systems... > Layout output to `/tmp/lilypond-n0vRDz'... > Converting to `atest-99.pdf'... > Deleting `/tmp/lilypond-n0vRDz'... > Interpreting music... > Preprocessing graphical objects...lilypond: > /home/hermann/lilypond-git-guile-2.2/lily/constrained-breaking.cc:481: void > Constrained_breaking::initialize(Paper_score*, const std::vector<long unsigned > int>&): Assertion `pagebreak_col_indices[i] > pagebreak_col_indices[i - 1]' > failed. > Aborted (core dumped) > > Probably > https://sourceforge.net/p/testlilyissues/issues/5767/ > is related as well.
thanks, that is useful to know, but I don't think it should be a blocker for this patch to go in. Description: Do not allow page breaks on the first column of a score This duplicates a column index in the vector of allowed break points, causing a crash. Do some more sanity checking on the input of Constrained_breaking. Fixes #5801 Please review this at https://codereview.appspot.com/581720054/ Affected files (+56, -11 lines): A input/regression/page-turn-page-breaking-first-column.ly M lily/constrained-breaking.cc M lily/include/constrained-breaking.hh M lily/page-breaking.cc Index: input/regression/page-turn-page-breaking-first-column.ly diff --git a/input/regression/page-turn-page-breaking-first-column.ly b/input/regression/page-turn-page-breaking-first-column.ly new file mode 100644 index 0000000000000000000000000000000000000000..a46d06c0446792e407f90b1ab3d2e3bf5b65f780 --- /dev/null +++ b/input/regression/page-turn-page-breaking-first-column.ly @@ -0,0 +1,34 @@ +\version "2.21.0" +\header { + texidoc = "Allowing the first command column to be breakable caused a crash in +Page_turn_page_breaking." +} + +\book { + \score { + \new Staff + \new Voice << + { + \time 4/4 + \bar "||" % trick Page_turn_engraver into allowing page break + s1*5 | + } + { + R1 + d4 + } + >> + + \layout { + \set Score.skipBars = ##t + \context { + \Staff + \consists "Page_turn_engraver" + } + } + } + + \paper { + page-breaking = #ly:page-turn-breaking + } +} Index: lily/constrained-breaking.cc diff --git a/lily/constrained-breaking.cc b/lily/constrained-breaking.cc index eba3b76390d6a83ff1ab607c31234d1a1c28bf93..8bddbbb539283164b4c755c133da11eada24730f 100644 --- a/lily/constrained-breaking.cc +++ b/lily/constrained-breaking.cc @@ -352,14 +352,14 @@ Constrained_breaking::prepare_solution (vsize start, vsize end, vsize sys_count) Constrained_breaking::Constrained_breaking (Paper_score *ps) { - start_.push_back (0); - initialize (ps); + vector<vsize> start; + start.push_back (0); + initialize (ps, start); } Constrained_breaking::Constrained_breaking (Paper_score *ps, vector<vsize> const &start) - : start_ (start) { - initialize (ps); + initialize (ps, start); } static SCM @@ -373,9 +373,11 @@ min_permission (SCM perm1, SCM perm2) return SCM_EOL; } -/* find the forces for all possible lines and cache ragged_ and ragged_right_ */ +/* find the forces for all possible lines and cache ragged_ and ragged_right_ + */ void -Constrained_breaking::initialize (Paper_score *ps) +Constrained_breaking::initialize (Paper_score *ps, + vector<vsize> const &pagebreak_col_indices) { valid_systems_ = systems_ = 0; pscore_ = ps; @@ -471,13 +473,20 @@ Constrained_breaking::initialize (Paper_score *ps) } /* work out all the starting indices */ - for (vsize i = 0; i < start_.size (); i++) + start_.reserve (pagebreak_col_indices.size ()); + for (vsize i = 0; i < pagebreak_col_indices.size (); i++) { + if (i) + { + assert (pagebreak_col_indices[i] > pagebreak_col_indices[i - 1]); + } vsize j; - for (j = 0; j + 1 < breaks_.size () && breaks_[j] < start_[i]; j++) + for (j = 0; + j + 1 < breaks_.size () && breaks_[j] < pagebreak_col_indices[i]; + j++) ; starting_breakpoints_.push_back (j); - start_[i] = breaks_[j]; + start_.push_back (breaks_[j]); } state_.resize (start_.size ()); } Index: lily/include/constrained-breaking.hh diff --git a/lily/include/constrained-breaking.hh b/lily/include/constrained-breaking.hh index f4eccddfd498f18b606eedd41b8cea92dc590652..693c6880f103b7c04cf86a272fdeadb25af20652 100644 --- a/lily/include/constrained-breaking.hh +++ b/lily/include/constrained-breaking.hh @@ -196,7 +196,7 @@ private: std::vector<Paper_column *> all_; std::vector<vsize> breaks_; - void initialize (Paper_score *); + void initialize (Paper_score *, std::vector<vsize> const &break_col_indices); void resize (vsize systems); Column_x_positions space_line (vsize start_col, vsize end_col); Index: lily/page-breaking.cc diff --git a/lily/page-breaking.cc b/lily/page-breaking.cc index 7fda30b7dc09604550440102dd00dde584150d1b..5ba3c1021056a937fa94ccadd7357419a1972ae0 100644 --- a/lily/page-breaking.cc +++ b/lily/page-breaking.cc @@ -730,6 +730,8 @@ Page_breaking::create_system_list () with page-break-permission = 'force. By using a grob predicate, we can accommodate both of these uses. + + is_break indicates if the column is an allowed page turn. */ void Page_breaking::find_chunks_and_breaks (Break_predicate is_break, Prob_break_predicate prob_is_break) @@ -776,7 +778,7 @@ Page_breaking::find_chunks_and_breaks (Break_predicate is_break, Prob_break_pred } bool last = (j == cols.size () - 1); - bool break_point = is_break && is_break (cols[j]); + bool break_point = is_break && j > 0 && is_break (cols[j]); bool chunk_end = scm_is_eq (cols[j]->get_property ("page-break-permission"), force_sym); Break_position cur_pos = Break_position (i, line_breaker_columns.size (),
