Skip to content

Add incidentEdgeTracker and indexCellData types and tests #180

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.

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
181 changes: 181 additions & 0 deletions s2/incident_edge_tracker.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
// Copyright 2025 The S2 Geometry Project Authors. All rights reserved.
Copy link
Collaborator

@alan-strohm alan-strohm May 27, 2025

Choose a reason for hiding this comment

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

I assume we need to resolve this discussion about what to do with Copyrights before proceeding: https://github.com/golang/geo/pull/178/files#r2094005567

//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package s2

// incidentEdgeKey is a tuple of (shape id, vertex) that compares by shape id.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: id -> ID throughout.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

type incidentEdgeKey struct {
shapeID int32
vertex Point
}

// We need a strict ordering to be a valid key for an ordered container, but
// we don't actually care about the ordering of the vertices (as long as
// they're grouped by shape id). Vertices are 3D points so they don't have a
// natural ordering, so we'll just compare them lexicographically.
func (i incidentEdgeKey) Cmp(o incidentEdgeKey) int {
if i.shapeID < o.shapeID {
return -1
}
if i.shapeID > o.shapeID {
return 1
}

return i.vertex.Cmp(o.vertex.Vector)
}

// incidentVertexEdge is a tuple of vertex and edgeID for processing incident edges.
type incidentVertexEdge struct {
vertex Point
edgeID int32
}

// incidentEdgeTracker is a used for detecting and tracking shape edges that
// are incident on the same vertex. Edges of multiple shapes may be tracked,
// but lookup is by shape id and vertex: there is no facility to get all
// edges of all shapes at a vertex. Edge vertices must compare exactly equal
// to be considered the same vertex, no tolerance is applied as this isn't
// intended for actions like snapping shapes together, which Builder does better
// and more robustly.
//
// To use, instantiate and then add edges with one or more sequences of calls;
// each sequence begins with startShape(), followed by repeated addEdge() calls
// to add edges for that shape, and ends with finishShape(). Those sequences do
// not need to visit shapes or edges in order. Then, read the edgeMap to get
// the resulting map from incidentEdgeKeys (which are shapeID, vertex pairs) to
// a set of edgeIDs of the shape that are incident to that vertex.
//
// This works on a block of edges at a time, meaning that to detect incident
// edges on a particular vertex, we must have at least three edges incident
// at that vertex when finishShape() is called. We don't maintain partial
// information between calls. However, subject to this constraint, a single
// shape's edges may be defined with multiple sequences of startShape(),
// addEdge()... , finishShape() calls.
//
// The reason for this is simple: most vertices don't have more than two incident
// edges (the incoming and outgoing edge). If we had to maintain information
// between calls, we'd end up with a map that contains every vertex, to no
// benefit. Instead, when finishShape() is called, we discard vertices that
// contain two or fewer incident edges.
//
// In principle this isn't a real limitation because generally we process a
// ShapeIndex one cell at a time, and, if a vertex has multiple edges, we'll see
// all the edges in the same cell as the vertex, and, in general, it's possible
// to aggregate edges before calling.
//
// The tracker maintains incident edges until it's reset. If you call it with
// each cell from an ShapeIndex, then at the end you will have all the
// incident edge information for the whole index. If only a subset is needed,
// call reset() when you're done edding edges.
type incidentEdgeTracker struct {
currentShapeID int32

// nursery is used to store points being processed for the map.
nursery []incidentVertexEdge

// edgeMap tracks edgeIDs for each incidentEdgeKey since we can, and do,
// encounter the same edges multiple times. This map gives us easy
// deduplication of edges as they're inserted.
edgeMap map[incidentEdgeKey]map[int32]bool
}

// newIncidentEdgeTracker returns a new incidentEdgeTracker.
func newIncidentEdgeTracker() *incidentEdgeTracker {
return &incidentEdgeTracker{
currentShapeID: -1,
nursery: []incidentVertexEdge{},
edgeMap: make(map[incidentEdgeKey]map[int32]bool),
}
}

// startShape is used to start a new shape.
func (t *incidentEdgeTracker) startShape(id int32) {
t.currentShapeID = id
t.nursery = t.nursery[:0]
}

// addEdge adds the given edge for the current shape to the nursery. If the
// edge is not degenerate, add its second endpoint as well.
func (t *incidentEdgeTracker) addEdge(edgeID int32, e Edge) {
if t.currentShapeID < 0 {
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

The error handling across these functions is pretty bad. Here, you silently drop edges. There's no checking to ensure that calls are made in the right sequence. Should we re-examine the API to make erroneous usage more difficult? All the internal calls to the C++ version look like:

      incident_edge_tracker_.StartShape(shape_id);
      for (const auto& edge : CurrentCell().shape_edges(shape_id)) {
        incident_edge_tracker_.AddEdge(edge.id, edge);
      }
      incident_edge_tracker_.FinishShape();

So we could have an interface that looks like:

// Add all edges to the tracker.   After calling, any vertices with multiple (> 2) incident
// edges will appear in the incident edge map.  Returns an error if any edges have a different
// shape ID than shapeID.
func (t *incidentEdgeTracker) addShapeEdges(shapeID int32, edges []ShapeEdge) error

This would also remove the need for a "nursery" and the associated type, resolving one of my other comments.

}

// Add non-degenerate edges to the nursery.
t.nursery = append(t.nursery, incidentVertexEdge{vertex: e.V0, edgeID: edgeID})
if !e.IsDegenerate() {
t.nursery = append(t.nursery, incidentVertexEdge{vertex: e.V1, edgeID: edgeID})
}
}

// finishShape is called once finished adding edges so they can be processed.
//
// After calling, only vertices with 2 or more incident edges will appear in
// the edge map.
func (t *incidentEdgeTracker) finishShape() {
// We want to keep any vertices with more than two incident edges. We could
// sort the array by vertex and remove any with fewer, but that would require
// shifting the array and could turn quadratic quickly.
//
// Instead we'll scan forward from each vertex, swapping entries with the same
// vertex into a contiguous range. Once we've done all the swapping we can
// just make sure that we have at least three edges in the range.
nurserySize := len(t.nursery)
for start := 0; start < nurserySize; {
end := start + 1

// Scan to the end of the array, swap entries so that entries with
// the same vertex as the start are adjacent.
next := start
currVertex := t.nursery[start].vertex
for next+1 < nurserySize {
next++
if t.nursery[next].vertex == currVertex {
t.nursery[next], t.nursery[end] = t.nursery[end], t.nursery[next]
end++
}
}

// Most vertices will have two incident edges (the incoming edge and the
// outgoing edge), which aren't interesting, skip them.
numEdges := end - start
if numEdges <= 2 {
start = end
continue
}

key := incidentEdgeKey{
shapeID: t.currentShapeID,
vertex: t.nursery[start].vertex,
}

// If we don't have this key yet, create it manually.
if _, ok := t.edgeMap[key]; !ok {
t.edgeMap[key] = map[int32]bool{}
}

for ; start != end; start++ {
t.edgeMap[key][t.nursery[start].edgeID] = true
}
}

// We are finished with this shape, clear the nursery.
t.nursery = t.nursery[:0]
}

// reset removes all incident edges from the tracker.
func (t *incidentEdgeTracker) reset() {
t.nursery = t.nursery[:0]
t.edgeMap = make(map[incidentEdgeKey]map[int32]bool)
}
70 changes: 70 additions & 0 deletions s2/incident_edge_tracker_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright 2025 The S2 Geometry Project Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package s2

import (
"testing"
)

func TestIncidentEdgeTrackerBasic(t *testing.T) {
tests := []struct {
index string
want int
}{
// These shapeindex strings came from validation query's test
Copy link
Collaborator

Choose a reason for hiding this comment

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

// corpus to determine which ones ended up actually getting
// tracked edges.
{
// Has 0 tracked edges
index: "## 0:0, 1:1",
want: 0,
},
{
// Has 1 tracked edges
index: "## 2:0, 0:-2, -2:0, 0:2; 2:0, 0:-1, -1:0, 0:1",
want: 1,
},
{
// Has 2 tracked edges
index: "## 2:0, 0:-2, -2:0, 0:2; 2:0, 0:-1, -2:0, 0:1",
want: 2,
},
}

for _, test := range tests {
index := makeShapeIndex(test.index)
index.Build()

iter := index.Iterator()
celldata := newIndexCellData()
celldata.loadCell(index, iter.CellID(), iter.IndexCell())

tracker := newIncidentEdgeTracker()

for _, clipped := range celldata.indexCell.shapes {
shapeID := clipped.shapeID
tracker.startShape(shapeID)
for _, e := range celldata.shapeEdges(shapeID) {
tracker.addEdge(e.ID, e.Edge)
}
tracker.finishShape()
}

if got := len(tracker.edgeMap); got != test.want {
t.Errorf("incidentEdgeTracker should have %d edges, got %d",
test.want, got)
}
}
}
Loading