Conversation
| return math.Max(i.Lo, math.Min(i.Hi, p)) | ||
| } | ||
|
|
||
| func (i Interval) Project(p float64) float64 { |
There was a problem hiding this comment.
Document all public methods. You can take inspiration from the cpp implementation at https://github.com/google/s2geometry/blob/ac448e2e939863dfefdeb6500ea74fda92d8187c/src/s2/r1interval.h#L172, but make sure you follow https://go.dev/doc/comment#func.
s2/r2rect.go
Outdated
| return r.CapBound().CellUnionBound() | ||
| } | ||
|
|
||
| // func (r R2Rect) ContainsPoint(p Point) bool { |
There was a problem hiding this comment.
Why are these commented out?
s2/r2rect.go
Outdated
| return r.CapBound().CellUnionBound() | ||
| } | ||
|
|
||
| // func (r R2Rect) ContainsPoint(p Point) bool { |
jmr
left a comment
jmr
left a comment
There was a problem hiding this comment.
Before I get too far into the details, why do you want this?
Are you just checking things off the "done" list, or do you have a use case?
This class isn't really used much. There was once an idea that the geometry library would handle both planar and spherical geometry, but the planar part isn't well developed.
The comments say it's a "stopgap measure" (which is admittedly now 20 years old).
| } | ||
| } | ||
|
|
||
| func (r Rect) Vertex(k int) Point { |
There was a problem hiding this comment.
All public functions should be directly tested.
| return Point{r.X.ClampPoint(p.X), r.Y.ClampPoint(p.Y)} | ||
| } | ||
|
|
||
| func (r Rect) Project(p Point) Point { |
There was a problem hiding this comment.
This function doesn't come from C++ as far as I can see.
|
Tests are broken, but should be fixed by #255. |
Hi @jmr, I don't have a specific use case, just checking off indeed. If you want to put this on hold or drop it, it's fine by me. |
| // We use the rectangle's center in (s,t)-space as the cap axis. This | ||
| // doesn't yield the minimal cap but it's pretty close. | ||
| cap := CapFromPoint(toPoint(r.Center())) | ||
| for k := 0; k < 4; k++ { |
| } | ||
|
|
||
| u, v := validFaceXYZToUV(0, p.Vector) | ||
| return r.Rect.ContainsPoint(r2.Point{X: u, Y: v}) |
There was a problem hiding this comment.
Where's the UVtoST call and why is no test failing?
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Please don't change unrelated files.
3 participants
Fully implement
S2R2Rectwith testing