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); } }
