Re: Fixes issue 39 by raising stems (issue3934041)

[mtsolo]

Reviewers: ,

Message:
New patch uploaded to the issue tracker to address Keith's comments.


http://codereview.appspot.com/3934041/diff/2001/lily/note-collision.cc
File lily/note-collision.cc (right):

http://codereview.appspot.com/3934041/diff/2001/lily/note-collision.cc#newcode177
lily/note-collision.cc:177: if (touch)
On 2011/01/09 09:52:42, Keith wrote:
> why not apply the stem lengthening here?  This boolean says we can try
to line
> the stems up, and that is the case when big flags can interfere.
> (I do not think you want to address the case of stem_to_stem, below.
It includes
> back-to-back chords that that are either acceptable without
lengthening, or
> would require a different approach altogether : << g'64 \\ <g' a'>2 >>
)

Done.

http://codereview.appspot.com/3934041/diff/2001/lily/note-collision.cc#newcode269
lily/note-collision.cc:269: else if (distant_half_collide ||
close_half_collide || full_collide) {
On 2011/01/09 09:52:42, Keith wrote:
> placing your new code here, you affect situations with shift>0, that
is up-stem
> to the right. << g'64 \\ <g' a'>2 >>
> Why?

Moved above.

http://codereview.appspot.com/3934041/diff/2001/lily/note-collision.cc#newcode276
lily/note-collision.cc:276: // magic numbers...
On 2011/01/09 07:03:45, Keith wrote:
> already been said, referring to this block.  I wanted assurance from
the comment
> that these numbers are intentionally non-monotonic, and reminder that
they are
> in half-spaces (right?)

Done.

http://codereview.appspot.com/3934041/diff/2001/lily/note-collision.cc#newcode278
lily/note-collision.cc:278: Real rv = raise_vals[durlog - 3];
On 2011/01/09 07:03:45, Keith wrote:
> Do we know 2 < durlog < 7 here? Forever?

Done.

http://codereview.appspot.com/3934041/diff/2001/lily/note-collision.cc#newcode280
lily/note-collision.cc:280: Real xtra = (distant_half_collide ||
close_half_collide) ? 0.0 : 1.3;
On 2011/01/09 07:03:45, Keith wrote:
> I take you to mean,  if (full_collide) rv += 1.3;

Done.

http://codereview.appspot.com/3934041/diff/2001/lily/stem-engraver.cc
File lily/stem-engraver.cc (right):

http://codereview.appspot.com/3934041/diff/2001/lily/stem-engraver.cc#newcode68
lily/stem-engraver.cc:68: stem_->set_property ("note-collision-bump",
scm_from_double (0.0));
On 2011/01/09 07:03:45, Keith wrote:
> Tell us why you use a new property, instead of adjusting 'length.   I
would need
> a better name to understand this after I forget about this bug; for
the way it
> acts now, something like extra-raise-tip would help the future me.

Done.

Description:
Fixes issue 39 by raising stems

Facultative

Potential fix for issue 39

Please review this at http://codereview.appspot.com/3934041/

Affected files:
  A input/regression/flag-collision.ly
  M lily/note-collision.cc
  M lily/stem-engraver.cc
  M lily/stem.cc
  scm/define-grob-properties.scm


Index: input/regression/flag-collision.ly
diff --git a/input/regression/flag-collision.ly  
b/input/regression/flag-collision.ly
new file mode 100644
index  
0000000000000000000000000000000000000000..6aa3c29a9c8e4d3acf7ec2e0ca78375314f4a0a3
--- /dev/null
+++ b/input/regression/flag-collision.ly
@@ -0,0 +1,10 @@
+\version "2.13.46"
+\header{
+  texidoc = "Stems shift up when flags would otherwise bump into  
noteheads."
+}
+
+\relative c'' {
+\time 2/4
+<< { \autoBeamOff c8 s4. c16 s4.. c32 s4... c64 s4.... c128 s4..... } \\ {  
b2 b2 b2 b2 b2 } >>
+<< { \autoBeamOff c8 s4. c16 s4.. c32 s4... c64 s4.... c128 s4..... } \\ {  
c2 c2 c2 c2 c2 } >>
+}
\ No newline at end of file
Index: lily/note-collision.cc
diff --git a/lily/note-collision.cc b/lily/note-collision.cc
index  
cc2ddb71885968c6656f8b95b2189617cf943364..3b7f5bb204d5bae4d31e1c5851ec56c5a77510db  
100644
--- a/lily/note-collision.cc
+++ b/lily/note-collision.cc
@@ -175,7 +175,31 @@ check_meshing_chords (Grob *me,
     touch = false;

   if (touch)
-    shift_amount *= -1;
+    {
+      if (!unsmob_grob (Note_column::get_stem (clash_up)->get_object  
("beam")))
+      {
+         Grob *s = Note_column::get_stem (clash_up);
+         if (s)
+           {
+             int durlog = scm_to_int (s->get_property ("duration-log"));
+             if (durlog < 3)
+               durlog = 3;
+             if (durlog > 7)
+               durlog = 7;
+             /* magic numbers are in half spaces and are non-monotonic to
+                achieve consistency in input/regression/flag-collision.ly  
*/
+             Real raise_vals [] = {0.0, 0.8, 0.4, 1.3, 1.3};
+             Real raise_tip = raise_vals[durlog - 3];
+             // magic number
+             if (full_collide)
+               raise_tip += 1.3;
+             Real current_raise_tip = scm_to_double (s->get_property  
("extra-raise-tip"));
+             s->set_property ("extra-raise-tip",
+                                 scm_from_double (current_raise_tip +  
raise_tip));
+           }
+      }
+      shift_amount *= -1;
+    }

   /* For full collisions, the right hand head may obscure dots, so
      make sure the dotted heads go to the right. */
@@ -266,8 +290,10 @@ check_meshing_chords (Grob *me,
     shift_amount *= 0.52;
   else if (distant_half_collide && !touch)
     shift_amount *= 0.4;
-  else if (distant_half_collide || close_half_collide || full_collide)
+  else if (distant_half_collide || close_half_collide || full_collide) {
+
     shift_amount *= 0.5;
+  }

   /* we're meshing. */
   else if (Rhythmic_head::dot_count (head_up) || Rhythmic_head::dot_count  
(head_down))
Index: lily/stem-engraver.cc
diff --git a/lily/stem-engraver.cc b/lily/stem-engraver.cc
index  
d09e3b95b8bb637de93018f12ab9f5f51eade291..8454fd9610d6e2df6342c062ec691ad247e9dccb  
100644
--- a/lily/stem-engraver.cc
+++ b/lily/stem-engraver.cc
@@ -65,6 +65,9 @@ Stem_engraver::make_stem (Grob_info gi)
   /* Announce the cause of the head as cause of the stem.  The
      stem needs a rhythmic structure to fit it into a beam.  */
   stem_ = make_item ("Stem", gi.grob ()->self_scm ());
+  /* This property should be used in cases where the stem
+     needs to be raised but its length has not been calculated yet. */
+  stem_->set_property ("extra-raise-tip", scm_from_double (0.0));

   if (tremolo_ev_)
     {
Index: lily/stem.cc
diff --git a/lily/stem.cc b/lily/stem.cc
index  
6bbeb162889776eebb2078056613432fe90f6eab..fbdd52e7d9e7885ea3b78a651248a7a4492f59a1  
100644
--- a/lily/stem.cc
+++ b/lily/stem.cc
@@ -317,7 +317,10 @@ Stem::calc_stem_end_position (SCM smob)
   if (!no_extend && dir * stem_end < 0)
     stem_end = 0.0;

-  return scm_from_double (stem_end);
+  /* This property is set so as to only effect upstems.  It should
+     return 0 for all stems pointing downwards. */
+  Real extra_raise_tip = scm_to_double (me->get_property  
("extra-raise-tip"));
+  return scm_from_double (stem_end + extra_raise_tip);
 }

 /* Length is in half-spaces (or: positions) here. */
@@ -1093,6 +1096,7 @@ ADD_INTERFACE (Stem,
               "neutral-direction "
               "no-stem-extend "
               "note-heads "
+              "extra-raise-tip "
               "positioning-done "
               "rests "
               "stem-end-position "
Index: scm/define-grob-properties.scm
diff --git a/scm/define-grob-properties.scm b/scm/define-grob-properties.scm
index  
36c8954123672804858bb2a4166c2a43ab035d01..b83c0158fb99012fba4e00f37d2ae386c6bf6046  
100644
--- a/scm/define-grob-properties.scm
+++ b/scm/define-grob-properties.scm
@@ -980,6 +980,8 @@ set, which grob to get the direction from.")
 the grob where this is set in.")
      (encompass-objects ,ly:grob-array? "Objects that a slur should avoid
 in addition to notes and stems.")
+     (extra-raise-tip ,number? "How much to bump up flags when notes
+collide.")

      (figures ,ly:grob-array? "Figured bass objects for continuation  
line.")