golang / geo

S2 geometry library in Go
Apache License 2.0
1.69k stars 182 forks source link

RectBound() fails for cell "c004" (level 5) #60

Closed topac closed 4 years ago

topac commented 4 years ago
cellID := s2.CellIDFromToken("c004")
cell := s2.CellFromCellID(cellID)
cell.RectBound() # panic: runtime error: index out of range [6] with length 6
rsned commented 4 years ago

https://play.golang.org/p/uuVRTFrp35m

package main

import (
    "fmt"

    "github.com/golang/geo/s2"
)

func main() {
    cellID := s2.CellIDFromToken("c004")
    fmt.Printf("'%v'.isValid() = %v", cellID, cellID.IsValid())
}

'Invalid: -3ffc000000000000'.isValid() = false

The cell token there is not a valid cell id, so it won't be able to generate proper results.

Was that CellID generated by an s2 library call?

topac commented 4 years ago

That CellID comes from this code:

package main

import (
  "fmt"
  "github.com/golang/geo/s2"
)

func main() {
  for i := 0; i < 6; i++ {
    c := s2.CellIDFromFace(i)

    level := 6
    c4start := c.ChildBeginAtLevel(level)
    c4stop := c.ChildEndAtLevel(level)

    fmt.Printf("face no.%d: %s(%v) -> %s(%v)\n", i, 
      c4start.ToToken(), 
      c4start.IsValid(), 
      c4stop.ToToken(),
      c4stop.IsValid())
  }
}

The output is:

face no.0: 0001(true) -> 2001(true)
face no.1: 2001(true) -> 4001(true)
face no.2: 4001(true) -> 6001(true)
face no.3: 6001(true) -> 8001(true)
face no.4: 8001(true) -> a001(true)
face no.5: a001(true) -> c001(false)

For faces from 0 to 4 both the first and last cell are valid, while for face 5 the last cell is not valid. If you think, for e.g., looping from 0001 to 2001 using cell.Next() the condition would be <= 2001, while looping from a001 to c001 force me to change the condition to < c001.

Maybe there is more clean way to loop over these cells, at the moment I'm going to validate each cell with IsValid() (which I wasn't aware of, thank for the hint)