Re: Corrects beamed cross-staff stems' pure heights (issue 6543046)

[mtsolo]

Reviewers: Keith,

Message:
I'm gonna hold off on posting to this one as it evolves constantly with
my work on issue 2801, which is revealing several issues in cross-staff
stem calculations.  Irrespective of how issue 2801 pans out, I'll take
all the relevant code from stem.cc and either push it as a separate
commit or include it in this Rietveld if my patch for 2801 isn't LGTM'd.

Description:
Corrects beamed cross-staff stems' pure heights

An incorrect subtraction made it such that the pure heighs were
too long for stems on extremal staves and too short on interior
staves.

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

Affected files:
  M lily/stem.cc


Index: lily/stem.cc
diff --git a/lily/stem.cc b/lily/stem.cc
index  
8069a456c814e48cf24bddec96bf9d8fae2279ac..43894123d7fc329bde7f9694edba29f042a8d298  
100644

--- a/lily/stem.cc
+++ b/lily/stem.cc
@@ -323,6 +323,7 @@ Stem::internal_pure_height (Grob *me, bool calc_beam)
   vector heights;
   vector my_stems;
   extract_grob_set (beam, "normal-stems", normal_stems);
+  // find the internal pure heights of all stems pointing in this  
direction

   for (vsize i = 0; i < normal_stems.size (); i++)
 if (get_grob_direction (normal_stems[i]) == dir)
   {
@@ -332,23 +333,27 @@ Stem::internal_pure_height (Grob *me, bool calc_beam)
   heights.push_back (iv);
 my_stems.push_back (normal_stems[i]);
   }
-  //iv.unite (heights.back ());
-  // look for cross staff effects
+
   vector coords;
   Grob *common = common_refpoint_of_array (my_stems, me, Y_AXIS);
   Real min_pos = infinity_f;
   Real max_pos = -infinity_f;
+
+  // find the offset of various staves as well as the minimum and  
maximum offset

   for (vsize i = 0; i < my_stems.size (); i++)
 {
   coords.push_back (my_stems[i]->pure_relative_y_coordinate  
(common, 0, INT_MAX));

   min_pos = min (min_pos, coords[i]);
   max_pos = max (max_pos, coords[i]);
 }
+  // Tack on the difference of the current stem's minimum-translation  
and the
+  // lowest or highest minimum translation to get the stem up to the  
axis

+  // group where the beam lies
   for (vsize i = 0; i < heights.size (); i++)
 {
   heights[i][dir] += dir == DOWN
- ? coords[i] - max_pos
- : coords[i] - min_pos;
+ ? min_pos - coords[i]
+ : max_pos - coords[i];
 }

   for (vsize i = 0; i < heights.size (); i++) iv.unite (heights[i]);



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Corrects beamed cross-staff stems' pure heights (issue 6543046)

[k-ohara5a5a]

LGTM of course


http://codereview.appspot.com/6543046/diff/1/lily/stem.cc
File lily/stem.cc (right):

http://codereview.appspot.com/6543046/diff/1/lily/stem.cc#newcode342
lily/stem.cc:342: // find the offset of various staves as well as the
minimum and maximum offset
How does this "find the offset of various staves"?
Maybe the common_refpoint is the System, in the cases where the current
Stem is attached to a cross-staff Beam,
but then the code is still finding offsets of Stems, note Staves.

http://codereview.appspot.com/6543046/diff/1/lily/stem.cc#newcode351
lily/stem.cc:351: // group where the beam lies
Now you're writing too much comment.  Most people think it is a bad idea
to write what the code does, only the reason why.
// Estimate the Stem to extend to the level of the furthest
// note-head in within its beamed group.  If that head is in
// a different VerticalAxisGroup, the height assumes the
// staff-spacing tentatively assumed before line-breaking.

http://codereview.appspot.com/6543046/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel