Re: Remove if/else for shift/raise (issue 573770043 by address@hidden)

[hanwenn]

Reviewers: hahnjo,


https://codereview.appspot.com/573770043/diff/553990043/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):

https://codereview.appspot.com/573770043/diff/553990043/lily/stencil-integral.cc#newcode966
lily/stencil-integral.cc:966: copy.raise (y_pos[i] - my_y);
On 2020/05/01 20:59:35, hahnjo wrote:
> hm, the old code passed different arguments in the two branches:
> copy.shift (x_pos[i] - my_x);
> vs.
> copy.shift (y_pos[i] - my_y);
> 
> and
> copy.raise (y_pos[i] - my_y);
> vs.
> copy.raise (x_pos[i] - my_x);
> 
> AFAICS this has nothing to do with commutativity of shift and raise.

Ah, you are right. Silly me.

Description:
Remove if/else for shift/raise

Shift (horizontal) and raise (vertical) movements are commutative

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

Affected files (+3, -17 lines):
  M lily/stencil-integral.cc


Index: lily/stencil-integral.cc
diff --git a/lily/stencil-integral.cc b/lily/stencil-integral.cc
index 
82c6ada525412ed3e4c43626c13bc047f6a28830..b78e04b1de65b63382bfbd8096c2a82ba6737ed1
 100644
--- a/lily/stencil-integral.cc
+++ b/lily/stencil-integral.cc
@@ -937,7 +937,6 @@ Grob::horizontal_skylines_from_stencil (SCM smob)
 SCM
 Grob::internal_skylines_from_element_stencils (Grob *me, Axis a, bool pure, 
int beg, int end)
 {
-
   extract_grob_set (me, "elements", elts);
   vector<Real> x_pos;
   vector<Real> y_pos;
@@ -961,23 +960,10 @@ Grob::internal_skylines_from_element_stencils (Grob *me, 
Axis a, bool pure, int
             Here, copying is essential.  Otherwise, the skyline pair will
             get doubly shifted!
           */
-          /*
-            It took Mike about 6 months of his life to add the `else' clause
-            below.  For horizontal skylines, the raise and shift calls need
-            to be reversed.  This is what was causing the problems in the
-            shifting with all of the tests. RIP 6 months!
-          */
           Skyline_pair copy = Skyline_pair (*skyp);
-          if (a == X_AXIS)
-            {
-              copy.shift (x_pos[i] - my_x);
-              copy.raise (y_pos[i] - my_y);
-            }
-          else
-            {
-              copy.raise (x_pos[i] - my_x);
-              copy.shift (y_pos[i] - my_y);
-            }
+
+          copy.shift (x_pos[i] - my_x);
+          copy.raise (y_pos[i] - my_y);
           res.merge (copy);
         }
     }