Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

golint fixes #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

golint fixes #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 31 additions & 31 deletions clipper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,17 @@ func _DBG(f func()) {}
type polygonType int

const (
_SUBJECT polygonType = iota
_CLIPPING
polygonSubject polygonType = iota
polygonClipping
)

type edgeType int

const (
_EDGE_NORMAL edgeType = iota
_EDGE_NON_CONTRIBUTING
_EDGE_SAME_TRANSITION
_EDGE_DIFFERENT_TRANSITION
edgeNormal edgeType = iota
edgeNonContributing
edgeSameTransition
edgeDifferentTransition
)

// This class contains methods for computing clipping operations on polygons.
Expand Down Expand Up @@ -91,12 +91,12 @@ func (c *clipper) compute(operation Op) Polygon {
// Add each segment to the eventQueue, sorted from left to right.
for _, cont := range c.subject {
for i := range cont {
addProcessedSegment(&c.eventQueue, cont.segment(i), _SUBJECT)
addProcessedSegment(&c.eventQueue, cont.segment(i), polygonSubject)
}
}
for _, cont := range c.clipping {
for i := range cont {
addProcessedSegment(&c.eventQueue, cont.segment(i), _CLIPPING)
addProcessedSegment(&c.eventQueue, cont.segment(i), polygonClipping)
}
}

Expand All @@ -106,7 +106,7 @@ func (c *clipper) compute(operation Op) Polygon {
// by sweeping from left to right.
S := sweepline{}

MINMAX_X := math.Min(subjectbb.Max.X, clippingbb.Max.X)
XMINMAX := math.Min(subjectbb.Max.X, clippingbb.Max.X)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe xMinMax would be better?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, please make it xMinMax

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you confirm if that is the only change? I prefer to do them all at once. What about the other capitals such as UNION, DIFFERENCE, ...?


_DBG(func() {
e := c.eventQueue.dequeue()
Expand All @@ -124,11 +124,11 @@ func (c *clipper) compute(operation Op) Polygon {

// optimization 1
switch {
case operation == INTERSECTION && e.p.X > MINMAX_X:
case operation == INTERSECTION && e.p.X > XMINMAX:
fallthrough
case operation == DIFFERENCE && e.p.X > subjectbb.Max.X:
return connector.toPolygon()
//case operation == UNION && e.p.X > MINMAX_X:
//case operation == UNION && e.p.X > XMINMAX:
// _DBG(func() { fmt.Print("\nUNION optimization, fast quit\n") })
// // add all the non-processed line segments to the result
// if !e.left {
Expand Down Expand Up @@ -161,7 +161,7 @@ func (c *clipper) compute(operation Op) Polygon {
switch {
case prev == nil: // there is not a previous line segment in S?
e.inside, e.inout = false, false
case prev.edgeType != _EDGE_NORMAL:
case prev.edgeType != edgeNormal:
if pos-2 < 0 { // e overlaps with prev
// Not sure how to handle the case when pos - 2 < 0, but judging
// from the C++ implementation this looks like how it should be handled.
Expand Down Expand Up @@ -229,7 +229,7 @@ func (c *clipper) compute(operation Op) Polygon {

// Check if the line segment belongs to the Boolean operation
switch e.edgeType {
case _EDGE_NORMAL:
case edgeNormal:
switch operation {
case INTERSECTION:
if e.other.inside {
Expand All @@ -240,18 +240,18 @@ func (c *clipper) compute(operation Op) Polygon {
connector.add(e.segment())
}
case DIFFERENCE:
if (e.polygonType == _SUBJECT && !e.other.inside) ||
(e.polygonType == _CLIPPING && e.other.inside) {
if (e.polygonType == polygonSubject && !e.other.inside) ||
(e.polygonType == polygonClipping && e.other.inside) {
connector.add(e.segment())
}
case XOR:
connector.add(e.segment())
}
case _EDGE_SAME_TRANSITION:
case edgeSameTransition:
if operation == INTERSECTION || operation == UNION {
connector.add(e.segment())
}
case _EDGE_DIFFERENT_TRANSITION:
case edgeDifferentTransition:
if operation == DIFFERENCE {
connector.add(e.segment())
}
Expand Down Expand Up @@ -326,7 +326,7 @@ func findIntersection(seg0, seg1 segment) (int, Point, Point) {
s1 := s0 + (d0.X*d1.X+d0.Y*d1.Y)/sqrLen0
smin := math.Min(s0, s1)
smax := math.Max(s0, s1)
w := make([]float64, 0)
var w []float64
imax := findIntersection2(0.0, 1.0, smin, smax, &w)

if imax > 0 {
Expand Down Expand Up @@ -406,7 +406,7 @@ func (c *clipper) possibleIntersection(e1, e2 *endpoint) {
}

// The line segments overlap
sortedEvents := make([]*endpoint, 0)
var sortedEvents []*endpoint
switch {
case e1.p.Equals(e2.p):
sortedEvents = append(sortedEvents, nil) // WTF [MC: WTF]
Expand All @@ -426,17 +426,17 @@ func (c *clipper) possibleIntersection(e1, e2 *endpoint) {
}

if len(sortedEvents) == 2 { // are both line segments equal?
e1.edgeType, e1.other.edgeType = _EDGE_NON_CONTRIBUTING, _EDGE_NON_CONTRIBUTING
e1.edgeType, e1.other.edgeType = edgeNonContributing, edgeNonContributing
if e1.inout == e2.inout {
e2.edgeType, e2.other.edgeType = _EDGE_SAME_TRANSITION, _EDGE_SAME_TRANSITION
e2.edgeType, e2.other.edgeType = edgeSameTransition, edgeSameTransition
} else {
e2.edgeType, e2.other.edgeType = _EDGE_DIFFERENT_TRANSITION, _EDGE_DIFFERENT_TRANSITION
e2.edgeType, e2.other.edgeType = edgeDifferentTransition, edgeDifferentTransition
}
return
}

if len(sortedEvents) == 3 { // the line segments share an endpoint
sortedEvents[1].edgeType, sortedEvents[1].other.edgeType = _EDGE_NON_CONTRIBUTING, _EDGE_NON_CONTRIBUTING
sortedEvents[1].edgeType, sortedEvents[1].other.edgeType = edgeNonContributing, edgeNonContributing
var idx int
// is the right endpoint the shared point?
if sortedEvents[0] != nil {
Expand All @@ -445,9 +445,9 @@ func (c *clipper) possibleIntersection(e1, e2 *endpoint) {
idx = 2
}
if e1.inout == e2.inout {
sortedEvents[idx].other.edgeType = _EDGE_SAME_TRANSITION
sortedEvents[idx].other.edgeType = edgeSameTransition
} else {
sortedEvents[idx].other.edgeType = _EDGE_DIFFERENT_TRANSITION
sortedEvents[idx].other.edgeType = edgeDifferentTransition
}
if sortedEvents[0] != nil {
c.divideSegment(sortedEvents[0], sortedEvents[1].p)
Expand All @@ -459,24 +459,24 @@ func (c *clipper) possibleIntersection(e1, e2 *endpoint) {

if sortedEvents[0] != sortedEvents[3].other {
// no line segment includes totally the OtherEnd one
sortedEvents[1].edgeType = _EDGE_NON_CONTRIBUTING
sortedEvents[1].edgeType = edgeNonContributing
if e1.inout == e2.inout {
sortedEvents[2].edgeType = _EDGE_SAME_TRANSITION
sortedEvents[2].edgeType = edgeSameTransition
} else {
sortedEvents[2].edgeType = _EDGE_DIFFERENT_TRANSITION
sortedEvents[2].edgeType = edgeDifferentTransition
}
c.divideSegment(sortedEvents[0], sortedEvents[1].p)
c.divideSegment(sortedEvents[1], sortedEvents[2].p)
return
}

// one line segment includes the other one
sortedEvents[1].edgeType, sortedEvents[1].other.edgeType = _EDGE_NON_CONTRIBUTING, _EDGE_NON_CONTRIBUTING
sortedEvents[1].edgeType, sortedEvents[1].other.edgeType = edgeNonContributing, edgeNonContributing
c.divideSegment(sortedEvents[0], sortedEvents[1].p)
if e1.inout == e2.inout {
sortedEvents[3].other.edgeType = _EDGE_SAME_TRANSITION
sortedEvents[3].other.edgeType = edgeSameTransition
} else {
sortedEvents[3].other.edgeType = _EDGE_DIFFERENT_TRANSITION
sortedEvents[3].other.edgeType = edgeDifferentTransition
}
c.divideSegment(sortedEvents[3].other, sortedEvents[2].p)
}
Expand Down
32 changes: 16 additions & 16 deletions endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,18 @@ func (e endpoint) String() string {
" other:", e.other.p, " inout:", e.inout, " inside:", e.inside, " edgeType:", e.edgeType, "}")
}

func (e1 *endpoint) equals(e2 *endpoint) bool {
return e1.p.Equals(e2.p) &&
e1.left == e2.left &&
e1.polygonType == e2.polygonType &&
e1.other == e2.other &&
e1.inout == e2.inout &&
e1.edgeType == e2.edgeType &&
e1.inside == e2.inside
func (e *endpoint) equals(ep *endpoint) bool {
return e.p.Equals(ep.p) &&
e.left == ep.left &&
e.polygonType == ep.polygonType &&
e.other == ep.other &&
e.inout == ep.inout &&
e.edgeType == ep.edgeType &&
e.inside == ep.inside
}

func (se *endpoint) segment() segment {
return segment{se.p, se.other.p}
func (e *endpoint) segment() segment {
return segment{e.p, e.other.p}
}

func signedArea(p0, p1, p2 Point) float64 {
Expand All @@ -68,13 +68,13 @@ func signedArea(p0, p1, p2 Point) float64 {
}

// Checks if this sweep event is below point p.
func (se *endpoint) below(x Point) bool {
if se.left {
return signedArea(se.p, se.other.p, x) > 0
func (e *endpoint) below(x Point) bool {
if e.left {
return signedArea(e.p, e.other.p, x) > 0
}
return signedArea(se.other.p, se.p, x) > 0
return signedArea(e.other.p, e.p, x) > 0
}

func (se *endpoint) above(x Point) bool {
return !se.below(x)
func (e *endpoint) above(x Point) bool {
return !e.below(x)
}
12 changes: 9 additions & 3 deletions geom.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,22 @@ import (
"math"
)

// Point is an X, Y coordinate pair.
type Point struct {
X, Y float64
}

// Equals returns true if both p1 and p2 describe exactly the same point.
func (p1 Point) Equals(p2 Point) bool {
return p1.X == p2.X && p1.Y == p2.Y
func (p Point) Equals(q Point) bool {
return p.X == q.X && p.Y == q.Y
}

// Length returns distance from p to point (0, 0).
func (p Point) Length() float64 {
return math.Sqrt(p.X*p.X + p.Y*p.Y)
}

// A Rectangle contains the points with Min.X <= X <= Max.X, Min.Y <= Y <= Max.Y.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure if it's <= Max.X and <= Max.Y. Please comment here with how you verified that. Sorry I can't do this myself now, will try when I find time if you don't.

type Rectangle struct {
Min, Max Point
}
Expand Down Expand Up @@ -111,7 +113,7 @@ func (c Contour) segment(index int) segment {
// if out-of-bounds, we expect panic detected by runtime
}

// Checks if a point is inside a contour using the "point in polygon" raycast method.
// Contains checks if a point is inside a contour using the "point in polygon" raycast method.
// This works for all polygons, whether they are clockwise or counter clockwise,
// convex or concave.
// See: http://en.wikipedia.org/wiki/Point_in_polygon#Ray_casting_algorithm
Expand Down Expand Up @@ -203,9 +205,13 @@ func (p Polygon) Clone() Polygon {
type Op int

const (
// UNION operation
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how these comments bring any value here. I can't believe golint would require them this way. I don't think the standard library has them this way. Some random example I seem to have found in the stdlib: http://golang.org/pkg/unicode/#pkg-constants - the block with UpperCase etc. doesn't seem to have any comments. I prefer to have no useless comments here and have golint complain than add them to satisfy golint. On the other hand, a comment above const ( line would be totally OK. If this would satisfy golint, please do it this way. Thanks.

UNION Op = iota
// INTERSECTION operation
INTERSECTION
// DIFFERENCE operation
DIFFERENCE
// XOR operation
XOR
)

Expand Down
4 changes: 2 additions & 2 deletions sweepline_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import (

func TestVerticalZigzag(t *T) {
mkseg := func(xl, yl, xr, yr float64) (left *endpoint) {
left = &endpoint{p: Point{xl, yl}, left: true, polygonType: _SUBJECT}
right := &endpoint{p: Point{xr, yr}, polygonType: _SUBJECT, other: left}
left = &endpoint{p: Point{xl, yl}, left: true, polygonType: polygonSubject}
right := &endpoint{p: Point{xr, yr}, polygonType: polygonSubject, other: left}
left.other = right
return left
}
Expand Down