S2: Regioncoverer slightly intersecting another face doesn't cover whole region

[zeevogels]

I created a regioncoverer from a rectangle as following:

var regionCoverer s2.RegionCoverer

regionCoverer.MaxCells = 10000
regionCoverer.MinLevel = 7
regionCoverer.MaxLevel = 7

topleft := s2.LatLngFromDegrees(72.0000064, -141.0000000)
topright := s2.LatLngFromDegrees(72.0000064, -55.6152420)
bottomleft := s2.LatLngFromDegrees(41.9017143, -141.0000000)
bottomright := s2.LatLngFromDegrees(41.9017143, -55.6152420)

rect = s2.Rect{}
rect = rect.AddPoint(bottomleft)
rect = rect.AddPoint(bottomright)
rect = rect.AddPoint(topright)
rect = rect.AddPoint(topleft)

covering := regionCoverer.Covering(s2.Region(rect))

The result is not 100% what I was expecting. A cellUnion of CellID's covering the whole rectangle should be the result but the small part of the rectangle that lies on a different face does not get any cellID's returned for it. I visualized all cell's in QGIS so that you can see what I mean:

The part where it looks like somebody took a bite out of it lies on a different face (face 4 vs. face 2).

Slightly lowering the bottomleft and bottomright latitude solves the problem.

[dsymonds]

Yes, looks like we had a couple of typos in the region coverer. I'll try to get a fix out today.

[dsymonds]

Actually the fix might not be so trivial to figure out, but e0bfefd fixes a few little things around it.

[azr]

Hello there , I also made this fix : f509ac2

Intersect returned false if the cell is completely 'inside' the loop.

This fixed this test: 46c3b8b

[azr]

To explain the fix:

Covering returns an empty CellUnion where it should not.

This happens because the generated cell by FastCovering is much bigger than the actual Region ( which is a Loop ) and Cells for which region.IntersectsCell(cell) == false are filtered out inside coverer.newCandidate.

Because the region is fully contained by the cell it does not intersects it.
I thought intersect should return true if the first point of the Loop is inside the cell.

[azr]

Please ignore the two last messages as they just fix a bug from a feature in a commit I tried to merge in from someone else.

Sorry !

[dsymonds]

It's possible that 0c31b84 fixes this bug (by fixing Rect.IntersectsCell). Can you give it a test?

[dsymonds]

I suspect the fix mentioned in my previous comment solves this, but feel free to reply if it doesn't.