search

golang / geo

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

I have a question about code #33

Closed halfrost closed 7 years ago

halfrost commented 7 years ago

In cellid.go ,faceIJOrientation() , https://github.com/golang/geo/blob/e2863f625f4601d8db214d8ecf4db9b701ebfbf8/s2/cellid.go#L504 , I do not understand why we should add this judgment -- ci.lsb()&0x1111111111111110 != 0 ？ I think faceIJOrientation and cellIDFromFaceIJ reverse each other, but faceIJOrientation have one more calculation of orientation than cellIDFromFaceIJ . Can you help me answer my doubts , why add 504 line's judgment ? (I think for a long time, really did not figure out here)

dsymonds commented 7 years ago

ci.lsb() returns the least significant bit that is set. The condition on cellid.go:504 is checking whether that bit is checking whether that is the lowest of any of the lowest 16 nybbles (except the lowest) before flipping the lowest bit of orientation. Any non-leaf cell should only have zeros in that position. Compare this to the docs sitting on the CellID type, and run a few examples yourself.

halfrost commented 7 years ago

@dsymonds There is Level-29 cell，non-leaf cell(Level-30)，cellID is 11011011101111110011111100000111011000100011011100010000101100，ci.lsb()&0x1111111111111110 == 0 . Except Level-29，Level-30 cell，the condition on cellid.go:504 will return ture. I do not understand why except Level-29，Level-30 ? Level-29 is non-leaf cell.

dsymonds commented 7 years ago

See if df320dd answers your questions. It adds some more documentation.

halfrost commented 6 years ago

@dsymonds After I thought hard and finally understand , sincerely thank you !