Skip to content

RegionIntersection implementation with tests - #275

Open
fmeurisse wants to merge 1 commit into
golang:masterfrom
fmeurisse:region_intersection
Open

fmeurisse wants to merge 1 commit into
golang:masterfrom
fmeurisse:region_intersection

Conversation

@fmeurisse

Copy link
Copy Markdown

This PR ports S2RegionIntersection from the C++ S2 library to Go

s2/region.go — also adds _ Region = RegionUnion{}, which was previously missing from the compile-time interface assertions.

- Added `RegionIntersection` struct for computing the intersection of regions.
- Provided comprehensive unit tests to verify functionality.
- Updated `README.md` and `region.go` to reflect the addition of `RegionIntersection`.

@jmr jmr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just a few minor changes.

Comment thread s2/regionintersection.go
@@ -0,0 +1,74 @@
// Copyright 2006 Google Inc. All rights reserved.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copyright 2006-2026 Google LLC. All rights reserved.

Comment thread s2/region.go
_ Region = Cap{}
_ Region = Cell{}
_ Region = (*CellUnion)(nil)
_ Region = RegionIntersection{}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Move this to the end before RegionUntion.

Comment thread s2/regionintersection.go

// RectBound returns a bounding latitude-longitude rectangle for this RegionIntersection.
func (ri RegionIntersection) RectBound() Rect {
ret := FullRect()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

rect is more idiomatic. I updated RegionUnion to use rect.

@panmari

panmari commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

@fmeurisse do you have some cycles to address the changes? Would love to merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants