[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Move get_normal to Offset::normal (issue 576000043 by address@hidden
From: |
hanwenn |
Subject: |
Re: Move get_normal to Offset::normal (issue 576000043 by address@hidden) |
Date: |
Mon, 13 Apr 2020 12:08:30 -0700 |
Reviewers: dak,
https://codereview.appspot.com/576000043/diff/577750046/flower/include/offset.hh
File flower/include/offset.hh (right):
https://codereview.appspot.com/576000043/diff/577750046/flower/include/offset.hh#newcode123
flower/include/offset.hh:123: Offset normal() const {
On 2020/04/13 18:24:47, dak wrote:
> It's kind of unusual to make a constructing function a (non-static)
member
> function unless this is necessitated for operator semantics, because
different
> conversion rules apply and there is some expectation that member
functions
> delivering the class type may change stuff in-place (the const is not
seen at
> call sites).
>
> Any reason to depart from conventions here?
It's consistent with offset::direction, offset::swapped, but I followed
your suggestion.
Description:
Move get_normal to Offset::normal
Please review this at https://codereview.appspot.com/576000043/
Affected files (+19, -24 lines):
M flower/include/offset.hh
M lily/stencil-integral.cc
Index: flower/include/offset.hh
diff --git a/flower/include/offset.hh b/flower/include/offset.hh
index
955b4a226bff213178365381cc3ad16b43858f41..a2d62cf01d6c69126cb82fa9f8dae8925de50f3f
100644
--- a/flower/include/offset.hh
+++ b/flower/include/offset.hh
@@ -115,6 +115,14 @@ public:
Real length () const;
bool is_sane () const;
Offset operator *= (Offset z2);
+
+ /*
+ Gets an orthogonal vector with same size to orig, pointing left
+ (in the complex domain, a multiplication by i)
+ */
+ Offset normal() const {
+ return Offset(-coordinate_a_[Y_AXIS], coordinate_a_[X_AXIS]);
+ }
};
#include "arithmetic-operator.hh"
@@ -175,4 +183,3 @@ cross_product (Offset o1, Offset o2)
}
#endif /* OFFSET_HH */
-
Index: lily/stencil-integral.cc
diff --git a/lily/stencil-integral.cc b/lily/stencil-integral.cc
index
6f4dfbae3fee4e764a9a308f290d009e2bb5fae9..6e667992a1af109a9e531c29e91754fd834f5e4a
100644
--- a/lily/stencil-integral.cc
+++ b/lily/stencil-integral.cc
@@ -128,15 +128,6 @@ get_path_list (SCM l)
return SCM_BOOL_F;
}
-// Gets an orthogonal vector with same size to orig, pointing left
-// (in the complex domain, a multiplication by i)
-
-Offset
-get_normal (Offset orig)
-{
- return Offset (-orig[Y_AXIS], orig[X_AXIS]);
-}
-
//// END UTILITY FUNCTIONS
/*
@@ -172,7 +163,7 @@ make_draw_line_boxes (vector<Box> &boxes,
vector<Drul_array<Offset> > &buildings
Offset dir = (right - left).direction ();
for (DOWN_and_UP (d))
{
- Offset outward = d * get_normal ((thick / 2) * dir);
+ Offset outward = (d * thick / 2) * dir.normal ();
Offset inter_l = scm_transform (trans, left + outward);
Offset inter_r = scm_transform (trans, right + outward);
if ((inter_l[X_AXIS] == inter_r[X_AXIS]) || (inter_l[Y_AXIS] ==
inter_r[Y_AXIS]))
@@ -268,7 +259,7 @@ make_partial_ellipse_boxes (vector<Box> &boxes,
Real ang
= linear_map (start, end, 0, quantization, static_cast<Real> (i));
Offset pt (offset_directed (ang).scale (rad));
- Offset inter = t (pt + d * get_normal ((th / 2) * pt.direction ()));
+ Offset inter = t (pt + (d * th / 2) * pt.direction ().normal ());
points[d].push_back (inter);
}
}
@@ -299,21 +290,11 @@ make_partial_ellipse_boxes (vector<Box> &boxes,
{
// beg line cap
Offset pt (offset_directed (start).scale (rad));
- create_path_cap (boxes,
- buildings,
- trans,
- pt,
- th / 2,
- -get_normal (pt));
+ create_path_cap (boxes, buildings, trans, pt, th / 2, -pt.normal ());
// end line cap
pt = offset_directed (end).scale (rad);
- create_path_cap (boxes,
- buildings,
- trans,
- pt,
- th / 2,
- get_normal (pt));
+ create_path_cap (boxes, buildings, trans, pt, th / 2, pt.normal ());
}
}
@@ -440,6 +421,13 @@ create_path_cap (vector<Box> &boxes,
SCM_UNDEFINED));
}
+// TODO - remove once https://codereview.appspot.com/583750044/ is in.
+Offset
+get_normal (Offset o)
+{
+ return o.normal ();
+}
+
void
make_draw_bezier_boxes (vector<Box> &boxes,
vector<Drul_array<Offset> > &buildings,