Re: Issue 5217: Fix sorting order without outside-staff-priority (issue 554960043 by address@hidden)

[jonas . hahnfeld]

Reviewers: Dan Eble, carl.d.sorensen_gmail.com, lilypond-pkx,

Message:
On 2019/11/06 09:33:45, lilypond-pkx wrote:
> Interestingly (or not) I did 3 patch runs on this patch and 2 out of 3
times I
> still got the reg test showing.

> I have some time today so I'll see if I can spot anything and  update
> accordingly.

> Jams

Compared to which baseline, run with master or with this patch already
applied? If with master, you'll have the random positions in the
baseline...
If with this patch applied, I'll try later if I can reproduce on my
system.

Description:
Issue 5217: Fix sorting order without outside-staff-priority

If the two Grobs have no outside-staff-priority, the compare function
staff_priority_less() would relate the two pointers. This may lead to
changing sorting orders in subsequent runs, apparently resulting in
"random" positions in the regression tests rest-dot-position.ly and
sometimes merge-rests-engraver.ly.
Solve this by keeping the original order in the vector:
 * Mark two Grobs without outside-staff-priority as being equal by
   always returning false (none is less than the other), and
 * use vector_stable_sort() to keep equal items in their relation.

Please review this at https://codereview.appspot.com/554960043/

Affected files (+5, -5 lines):
  M lily/axis-group-interface.cc


Index: lily/axis-group-interface.cc
diff --git a/lily/axis-group-interface.cc b/lily/axis-group-interface.cc
index  
248e6de1b4687ec4143d66b70c96803f76b9c977..1370c6e8cea168c9d9fb303b3e22380a240bbab7  
100644
--- a/lily/axis-group-interface.cc
+++ b/lily/axis-group-interface.cc
@@ -622,10 +622,10 @@ staff_priority_less (Grob *const &g1, Grob *const &g2)
     return false;

   /* if neither grob has an outside-staff priority, the ordering will have  
no
-     effect -- we just need to choose a consistent ordering. We do this to
-     avoid the side-effect of calculating extents. */
-  if (isinf (priority_1))
-    return g1 < g2;
+     effect and we assume the two grobs to be equal (none of the two is  
less).
+     We do this to avoid the side-effect of calculating extents. */
+  if (isinf (priority_1) && isinf (priority_2))
+    return false;

   /* if there is no preference in staff priority, choose the left-most one  
*/
   Grob *common = g1->common_refpoint (g2, X_AXIS);
@@ -892,7 +892,7 @@ Axis_group_interface::skyline_spacing (Grob *me)
     if (scm_is_number (elements[i]->get_property  
("outside-staff-priority")))
       elements[i]->extent (elements[i], X_AXIS);

-  vector_sort (elements, staff_priority_less);
+  vector_stable_sort (elements, staff_priority_less);
   Grob *x_common = common_refpoint_of_array (elements, me, X_AXIS);
   Grob *y_common = common_refpoint_of_array (elements, me, Y_AXIS);