Re: Colored box behind a single note

[Werner LEMBERG]

>> diff --git a/lily/horizontal-bracket-engraver.cc 
>> b/lily/horizontal-bracket-engraver.cc
>> index 608af35965..a42268a357 100644
>> --- a/lily/horizontal-bracket-engraver.cc
>> +++ b/lily/horizontal-bracket-engraver.cc
>> @@ -56,10 +56,23 @@ Horizontal_bracket_engraver::listen_note_grouping 
>> (Stream_event *ev)
>>  {
>>    Direction d = to_dir (ev->get_property ("span-direction"));
>>  
>> +  // Allow one single-moment bracket.  Abbreviating a horizontal bracket's
>> +  // `START' span-direction with `L' and `STOP' with `R', this means that we
>> +  // can have
>> +  //
>> +  //   LLL...LLR
>> +  //
>> +  // or
>> +  //
>> +  //   LRRR...RR
>> +  //
>> +  // events attached to a single moment (we don't take care of the order of
>> +  // `L' and `R' events).
>> +
>>    if (d == STOP)
>>      {
>>        pop_count_++;
>> -      if (pop_count_ > bracket_stack_.size ())
>> +      if (pop_count_ > bracket_stack_.size () + 1)
>>          ev->origin ()->warning (_ ("do not have that many brackets"));
>>      }
>>    else
>> @@ -68,22 +81,33 @@ Horizontal_bracket_engraver::listen_note_grouping 
>> (Stream_event *ev)
>>        events_.push_back (ev);
>>      }
>>  
>> -  if (pop_count_ && push_count_)
>> +  if (pop_count_ >= 2 && push_count_ >= 2)
>>      ev->origin ()->warning (_ ("conflicting note group events"));
>>  }
> 
> Those number changes do not appear like reflecting some design but
> rather like poking the code until the wanted construct happens not
> to cause warnings or errors.  Why increase push/pop counts by 2 when
> admitting one more level?  Why allow popping exactly 1 more than
> pushing?

OK, more comments added to the source code.

I wish that other parts of lilypond are having similarly detailed
comments...


    Werner

diff --git a/lily/horizontal-bracket-engraver.cc 
b/lily/horizontal-bracket-engraver.cc
index 608af35965..0889ae10af 100644
--- a/lily/horizontal-bracket-engraver.cc
+++ b/lily/horizontal-bracket-engraver.cc
@@ -28,6 +28,35 @@
 
 #include "translator.icc"
 
+// In the following (simplified) description of the algorithm, we abbreviate
+// a horizontal bracket's `START' and `STOP' span-direction with `L' and
+// `R', respectively.
+//
+// For a given moment,
+//
+// (1) count the number of `L' and `R' events; they are also pushed onto a
+//     stack so that they can be referenced later on (method
+//     `listen_note_grouping'),
+//
+// (2) create a HorizontalBracket spanner for every `L' event, link it as
+//     necessary, and push it onto `bracket_stack' (method `process_music'),
+//
+// (3) set one boundary for every element of `bracket_stack' (method
+//     `acknowledge_note_column'),
+//
+// (4) check whether there is a single `R' event within a bunch of `L' events
+//     (or a single `L' event within a bunch of `R' events) and set the
+//     other boundary with the same values to create a single-moment
+//     horizontal bracket for this case (method `acknowledge_note_column'),
+//
+// (5) pop an element off `bracket_stack' for every `R' event; ignore
+//     excessive `L' and `R' events (method `stop_translation_timestep').
+//
+// The above algorithm allows for nested horizontal brackets.
+//
+// TODO: Allow overlapping horizontal brackets (issue #5240).  This would
+//       need a complete rewrite of the algorithm, however.
+
 class Horizontal_bracket_engraver : public Engraver
 {
 public:
@@ -56,10 +85,27 @@ Horizontal_bracket_engraver::listen_note_grouping 
(Stream_event *ev)
 {
   Direction d = to_dir (ev->get_property ("span-direction"));
 
+  // Allow one single-moment bracket.  This means that we can have
+  //
+  //   LLL...LLR
+  //
+  // or
+  //
+  //   LRRR...RR   .
+  //
+  // Note that the order of events doesn't matter; in other words,
+  // `LLLRLLL...LLL' is valid, too.
+
   if (d == STOP)
     {
       pop_count_++;
-      if (pop_count_ > bracket_stack_.size ())
+
+      // Since N `L' events create N HorizontalBracket grobs we need (at
+      // most) N `R' events to finish them.  However, at this particular
+      // moment, a single-moment horizontal bracket might be created also;
+      // this takes another `L' event (which might not have caused a new
+      // element on `bracket_stack' yet) and its corresponding `R' event.
+      if (pop_count_ > bracket_stack_.size () + 1)
         ev->origin ()->warning (_ ("do not have that many brackets"));
     }
   else
@@ -68,22 +114,41 @@ Horizontal_bracket_engraver::listen_note_grouping 
(Stream_event *ev)
       events_.push_back (ev);
     }
 
-  if (pop_count_ && push_count_)
+  // The handled number of events are
+  //
+  //   LLL...LLR   =>   push_count_ >= 1 && pop_count_ == 1
+  //
+  // or
+  //
+  //   RRR...RRL   =>   pop_count_ >= 1 && push_count_ == 1   .
+  //
+  if (pop_count_ >= 2 && push_count_ >= 2)
     ev->origin ()->warning (_ ("conflicting note group events"));
 }
 
 void
 Horizontal_bracket_engraver::acknowledge_note_column (Grob_info gi)
 {
+  bool process_single_moment_bracket = pop_count_ && push_count_;
+
   for (vsize i = 0; i < bracket_stack_.size (); i++)
     {
-      Side_position_interface::add_support (bracket_stack_[i], gi.grob ());
-      Pointer_group_interface::add_grob (bracket_stack_[i],
-                                         ly_symbol2scm ("columns"), gi.grob 
());
-      add_bound_item (bracket_stack_[i],
-                      gi.grob ());
-      add_bound_item (text_stack_[i],
-                      gi.grob ());
+      // For a single-moment horizontal bracket (which is the most recently
+      // pushed element of `bracket_stack', use the current note column for
+      // both the left and right bound of the bracket.
+      int count = (process_single_moment_bracket
+                   && i + 1 == bracket_stack_.size ()) ? 2 : 1;
+
+      for (int j = 0; j < count; j++)
+        {
+          Side_position_interface::add_support (bracket_stack_[i],
+                                                gi.grob ());
+          Pointer_group_interface::add_grob (bracket_stack_[i],
+                                             ly_symbol2scm ("columns"),
+                                             gi.grob ());
+          add_bound_item (bracket_stack_[i], gi.grob ());
+          add_bound_item (text_stack_[i], gi.grob ());
+        }
     }
 }