[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Remove if/else for shift/raise (issue 573770043 by address@hidden)
From: |
hanwenn |
Subject: |
Re: Remove if/else for shift/raise (issue 573770043 by address@hidden) |
Date: |
Fri, 01 May 2020 15:20:34 -0700 |
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);
}
}