diff --git a/atlas/map.go b/atlas/map.go index aab8e79de..cf6ef5da5 100644 --- a/atlas/map.go +++ b/atlas/map.go @@ -295,7 +295,8 @@ func (m Map) encodeMVTTile(ctx context.Context, tile *slippy.Tile, params provid // with the adoption of the new make valid routine. once implemented, the clipRegion // calculation will need to be in the same coordinate space as the geometry the // make valid function will be operating on. - geo = mvt.PrepareGeo(geo, tile.Extent3857(), float64(mvt.DefaultExtent)) + ext, _ := ptile.Extent() + geo = mvt.PrepareGeo(geo, ext, float64(mvt.DefaultExtent)) // TODO: remove this geom conversion step once the validate function uses geom types sg, err = convert.ToTegola(geo) diff --git a/atlas/map_test.go b/atlas/map_test.go index 699df275a..e7a6370eb 100644 --- a/atlas/map_test.go +++ b/atlas/map_test.go @@ -345,7 +345,7 @@ func TestEncode(t *testing.T) { }, }, }, - tile: slippy.NewTile(2, 3, 4), + tile: slippy.NewTile(2, 3, 3), expected: vectorTile.Tile{ Layers: []*vectorTile.Tile_Layer{ { @@ -403,7 +403,7 @@ func TestEncode(t *testing.T) { }, }, }, - tile: slippy.NewTile(2, 3, 4), + tile: slippy.NewTile(2, 3, 3), expected: vectorTile.Tile{ Layers: []*vectorTile.Tile_Layer{ { diff --git a/cmd/tegola/cmd/cache/seed_purge.go b/cmd/tegola/cmd/cache/seed_purge.go index f826b72db..92ba32c88 100644 --- a/cmd/tegola/cmd/cache/seed_purge.go +++ b/cmd/tegola/cmd/cache/seed_purge.go @@ -7,12 +7,12 @@ import ( "strings" "github.com/go-spatial/cobra" + "github.com/go-spatial/geom" "github.com/go-spatial/geom/slippy" "github.com/go-spatial/tegola/atlas" "github.com/go-spatial/tegola/internal/build" gdcmd "github.com/go-spatial/tegola/internal/cmd" "github.com/go-spatial/tegola/internal/log" - "github.com/go-spatial/tegola/maths" "github.com/go-spatial/tegola/observability" "github.com/go-spatial/tegola/provider" ) @@ -203,46 +203,30 @@ func generateTilesForBounds(ctx context.Context, bounds [4]float64, zooms []uint channel: make(chan *slippy.Tile), } + webmercatorGrid, err := slippy.NewGrid(3857) + if err != nil { + tce.setError(fmt.Errorf("Could not create Web Mercator grid (3857): %s", err)) + tce.Close() + return tce + } + go func() { defer tce.Close() - for _, z := range zooms { - // get the tiles at the corners given the bounds and zoom - corner1 := slippy.NewTileLatLon(z, bounds[1], bounds[0]) - corner2 := slippy.NewTileLatLon(z, bounds[3], bounds[2]) - - // x,y initials and finals - _, xi, yi := corner1.ZXY() - _, xf, yf := corner2.ZXY() - maxXYatZ := uint(maths.Exp2(uint64(z))) - 1 - - // ensure the initials are smaller than finals - // this breaks at the anti meridian: https://github.com/go-spatial/tegola/issues/500 - if xi > xf { - xi, xf = xf, xi - } - if yi > yf { - yi, yf = yf, yi - } + var extent geom.Extent = bounds + for _, z := range zooms { - // prevent seeding out of bounds - xf = maths.Min(xf, maxXYatZ) - yf = maths.Min(yf, maxXYatZ) - - MainLoop: - for x := xi; x <= xf; x++ { - // loop columns - for y := yi; y <= yf; y++ { - select { - case tce.channel <- slippy.NewTile(z, x, y): - case <-ctx.Done(): - // we have been cancelled - break MainLoop - } + tiles := slippy.FromBounds(webmercatorGrid, &extent, z) + for _, tile := range tiles { + t := tile + select { + case tce.channel <- &t: + case <-ctx.Done(): + // we have been cancelled + return } } } - tce.Close() }() return tce } diff --git a/cmd/tegola/cmd/cache/seed_purge_generator_test.go b/cmd/tegola/cmd/cache/seed_purge_generator_test.go index 9db38e637..a0d762150 100644 --- a/cmd/tegola/cmd/cache/seed_purge_generator_test.go +++ b/cmd/tegola/cmd/cache/seed_purge_generator_test.go @@ -134,11 +134,7 @@ func TestGenerateTilesForBounds(t *testing.T) { zooms: []uint{1}, bounds: [4]float64{180.0, 90.0, 0.0, 0.0}, tiles: sTiles{ - /* - * Note that the test case for this from the original had the tile being - * produced as 1/1/0 and not 1/1/1 but the code is identical, so not sure - * what the difference is. - */ + slippy.NewTile(1, 1, 0), slippy.NewTile(1, 1, 1), }, }, diff --git a/cmd/tegola/cmd/cache/slippy.go b/cmd/tegola/cmd/cache/slippy.go new file mode 100644 index 000000000..ffb6ed3c2 --- /dev/null +++ b/cmd/tegola/cmd/cache/slippy.go @@ -0,0 +1,43 @@ +package cache + +import ( + "math" + + "github.com/go-spatial/geom/slippy" +) + +// rangeFamilyAt runs the given callback on every related tile at the given zoom. This could include +// the provided tile itself (if the same zoom is provided), the parent (overlapping tile at a lower zoom +// level), or children (overlapping tiles at a higher zoom level). +// +// Copied from go-spatial/geom (dc1d50720ee77122d0) since the function by this name doesn't do +// the same thing anymore. (In geom, it no longer returns ancestors or self, only descendants. +// It's also buggy because grid.ToNative/FromNative are buggy.) +// +// This function should be removed once the one in geom is updated to work as expected. +func rangeFamilyAt(t *slippy.Tile, zoom uint, f func(*slippy.Tile) error) error { + // handle ancestors and self + if zoom <= t.Z { + mag := t.Z - zoom + arg := slippy.NewTile(zoom, t.X>>mag, t.Y>>mag) + return f(arg) + } + + // handle descendants + mag := zoom - t.Z + delta := uint(math.Exp2(float64(mag))) + + leastX := t.X << mag + leastY := t.Y << mag + + for x := leastX; x < leastX+delta; x++ { + for y := leastY; y < leastY+delta; y++ { + err := f(slippy.NewTile(zoom, x, y)) + if err != nil { + return err + } + } + } + + return nil +} diff --git a/cmd/tegola/cmd/cache/slippy_test.go b/cmd/tegola/cmd/cache/slippy_test.go new file mode 100644 index 000000000..38d21380e --- /dev/null +++ b/cmd/tegola/cmd/cache/slippy_test.go @@ -0,0 +1,112 @@ +package cache + +import ( + "testing" + + "github.com/go-spatial/geom/slippy" +) + +func TestRangeFamilyAt(t *testing.T) { + type coord struct { + z, x, y uint + } + + type tcase struct { + tile *slippy.Tile + zoomAt uint + expected []coord + } + + isIn := func(arr []coord, c coord) bool { + for _, v := range arr { + if v == c { + return true + } + } + + return false + } + + fn := func(tc tcase) func(t *testing.T) { + return func(t *testing.T) { + + coordList := make([]coord, 0, len(tc.expected)) + rangeFamilyAt(tc.tile, tc.zoomAt, func(tile *slippy.Tile) error { + z, x, y := tile.ZXY() + c := coord{z, x, y} + + coordList = append(coordList, c) + + return nil + }) + + if len(coordList) != len(tc.expected) { + t.Fatalf("coordinate list length, expected %d, got %d: %v \n\n %v", len(tc.expected), len(coordList), tc.expected, coordList) + } + + for _, v := range tc.expected { + if !isIn(coordList, v) { + t.Logf("coordinates: %v", coordList) + t.Fatalf("coordinate exists, expected %v, got missing", v) + } + } + + } + } + + testcases := map[string]tcase{ + "children 1": { + tile: slippy.NewTile(0, 0, 0), + zoomAt: 1, + expected: []coord{ + {1, 0, 0}, + {1, 0, 1}, + {1, 1, 0}, + {1, 1, 1}, + }, + }, + "children 2": { + tile: slippy.NewTile(8, 3, 5), + zoomAt: 10, + expected: []coord{ + {10, 12, 20}, + {10, 12, 21}, + {10, 12, 22}, + {10, 12, 23}, + // + {10, 13, 20}, + {10, 13, 21}, + {10, 13, 22}, + {10, 13, 23}, + // + {10, 14, 20}, + {10, 14, 21}, + {10, 14, 22}, + {10, 14, 23}, + // + {10, 15, 20}, + {10, 15, 21}, + {10, 15, 22}, + {10, 15, 23}, + }, + }, + "parent 1": { + tile: slippy.NewTile(1, 0, 0), + zoomAt: 0, + expected: []coord{ + {0, 0, 0}, + }, + }, + "parent 2": { + tile: slippy.NewTile(3, 3, 5), + zoomAt: 1, + expected: []coord{ + {1, 0, 1}, + }, + }, + } + + for name, tc := range testcases { + t.Run(name, fn(tc)) + } +} diff --git a/cmd/tegola/cmd/cache/tile_list.go b/cmd/tegola/cmd/cache/tile_list.go index f745d105a..3f7825fb7 100644 --- a/cmd/tegola/cmd/cache/tile_list.go +++ b/cmd/tegola/cmd/cache/tile_list.go @@ -102,6 +102,7 @@ func generateTilesForTileList(ctx context.Context, tilelist io.Reader, explicit tce := &TileChannel{ channel: make(chan *slippy.Tile), } + go func() { defer tce.Close() @@ -134,7 +135,7 @@ func generateTilesForTileList(ctx context.Context, tilelist io.Reader, explicit for _, zoom := range zooms { // range will include the original tile. - err = tile.RangeFamilyAt(zoom, func(tile *slippy.Tile) error { + err = rangeFamilyAt(tile, zoom, func(tile *slippy.Tile) error { select { case tce.channel <- tile: case <-ctx.Done(): diff --git a/cmd/tegola/cmd/cache/tile_name.go b/cmd/tegola/cmd/cache/tile_name.go index 41c82db49..250ee61b2 100644 --- a/cmd/tegola/cmd/cache/tile_name.go +++ b/cmd/tegola/cmd/cache/tile_name.go @@ -82,6 +82,7 @@ func generateTilesForTileName(ctx context.Context, tile *slippy.Tile, explicit b tce := &TileChannel{ channel: make(chan *slippy.Tile), } + go func() { defer tce.Close() if tile == nil { @@ -98,7 +99,7 @@ func generateTilesForTileName(ctx context.Context, tile *slippy.Tile, explicit b } for _, zoom := range zooms { // range will include the original tile. - err := tile.RangeFamilyAt(zoom, func(tile *slippy.Tile) error { + err := rangeFamilyAt(tile, zoom, func(tile *slippy.Tile) error { select { case tce.channel <- tile: case <-ctx.Done(): diff --git a/provider/hana/hana.go b/provider/hana/hana.go index a75a39f11..31d62abd5 100644 --- a/provider/hana/hana.go +++ b/provider/hana/hana.go @@ -55,11 +55,11 @@ func (c connectionPoolCollector) QueryContextWithBBox(ctx context.Context, query return nil, err } - strLL, _ := wkt.Encode(ll) + strLL, _ := wkt.EncodeString(ll) lobLL := new(driver.Lob) lobLL.SetReader(strings.NewReader(strLL)) - strUR, _ := wkt.Encode(ur) + strUR, _ := wkt.EncodeString(ur) lobUR := new(driver.Lob) lobUR.SetReader(strings.NewReader(strUR)) diff --git a/provider/provider.go b/provider/provider.go index 6813892cb..4cd7b9187 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -29,6 +29,18 @@ const ( TypeAll = TypeStd & TypeMvt ) +var ( + webmercatorGrid slippy.Grid +) + +func init() { + var err error + webmercatorGrid, err = slippy.NewGrid(3857) + if err != nil { + log.Fatal("Could not create Web Mercator grid (3857): ", err) + } +} + func (pt providerType) Prefix() string { if pt == TypeMvt { return "mvt_" @@ -67,6 +79,9 @@ func (pf providerFilter) Is(ps ...providerType) bool { // the geom port is mostly done as part of issue #499 (removing the // Tile interface in this package) // TODO(@ear7h) remove this atrocity from the code base +// NOTE(@jchamberlain) leaving said atrocity for now because several places +// in the code require extent+srid, (which said atrocity supplies), +// not just extent. type tile_t struct { slippy.Tile buffer uint @@ -86,12 +101,22 @@ func NewTile(z, x, y, buf, srid uint) Tile { // Extent returns the extent of the tile func (tile *tile_t) Extent() (ext *geom.Extent, srid uint64) { - return tile.Extent3857(), 3857 + if ext, ok := slippy.Extent(webmercatorGrid, &tile.Tile); ok { + return ext, 3857 + } + + log.Error("Could not generate valid extent for tile.", tile) + return &geom.Extent{}, 3857 } // BufferedExtent returns a the extent of the tile, with the define buffer func (tile *tile_t) BufferedExtent() (ext *geom.Extent, srid uint64) { - return tile.Extent3857().ExpandBy(slippy.Pixels2Webs(tile.Z, tile.buffer)), 3857 + ext, _ = tile.Extent() + // WARNING: slippy.PixelsToNative() is misnamed. It doesn't convert the given number of pixels into + // anything--the pixels arguments isn't even used (hence set to 0 here). Instead, slippy.PixelsToNative() + // return the ratio of pixels to projected units at the given zoom. We multiply its return value by + // the pixel count in tile.buffer to get the expected conversion. + return ext.ExpandBy(slippy.PixelsToNative(webmercatorGrid, tile.Z, 0) * float64(tile.buffer)), 3857 } // Tile is an interface used by Tiler, it is an unnecessary abstraction and is @@ -153,7 +178,9 @@ type pfns struct { var providers map[string]pfns // Register the provider with the system. This call is generally made in the init functions of the provider. -// the clean up function will be called during shutdown of the provider to allow the provider to do any cleanup. +// +// the clean up function will be called during shutdown of the provider to allow the provider to do any cleanup. +// // The init function can not be nil, the cleanup function may be nil func Register(name string, init InitFunc, cleanup CleanupFunc) error { if init == nil { @@ -176,7 +203,9 @@ func Register(name string, init InitFunc, cleanup CleanupFunc) error { } // MVTRegister the provider with the system. This call is generally made in the init functions of the provider. -// the clean up function will be called during shutdown of the provider to allow the provider to do any cleanup. +// +// the clean up function will be called during shutdown of the provider to allow the provider to do any cleanup. +// // The init function can not be nil, the cleanup function may be nil func MVTRegister(name string, init MVTInitFunc, cleanup CleanupFunc) error { if init == nil { diff --git a/server/handle_map_layer_zxy.go b/server/handle_map_layer_zxy.go index 39345a3b0..652116396 100644 --- a/server/handle_map_layer_zxy.go +++ b/server/handle_map_layer_zxy.go @@ -8,6 +8,8 @@ import ( "strconv" "strings" + "github.com/go-spatial/geom" + "github.com/go-spatial/proj" "github.com/go-spatial/tegola/observability" "github.com/go-spatial/tegola/provider" @@ -20,6 +22,18 @@ import ( "github.com/go-spatial/tegola/maths" ) +var ( + webmercatorGrid slippy.Grid +) + +func init() { + var err error + webmercatorGrid, err = slippy.NewGrid(3857) + if err != nil { + log.Fatal("Could not create Web Mercator grid (3857): ", err) + } +} + type HandleMapLayerZXY struct { // required mapName string @@ -148,9 +162,26 @@ func (req HandleMapLayerZXY) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Check to see that the zxy is within the bounds of the map. // TODO(@ear7h): use a more efficient version of Intersect that doesn't // make a new extent - textent := tile.Extent4326() - if _, intersect := m.Bounds.Intersect(textent); !intersect { - msg := fmt.Sprintf("map (%v -- %v) does not contains tile at %v/%v/%v -- %v", req.mapName, m.Bounds, req.z, req.x, req.y, textent) + ext3857, ok := slippy.Extent(webmercatorGrid, tile) + if !ok { + msg := fmt.Sprintf("map (%v -- %v) does not contains tile at %v/%v/%v. Unable to generate extent.", req.mapName, m.Bounds, req.z, req.x, req.y) + log.Debug(msg) + http.Error(w, msg, http.StatusNotFound) + return + } + + points4326, err := proj.Inverse(proj.WebMercator, ext3857[:]) + if err != nil { + msg := fmt.Sprintf("Unable to convert 3857 to 4326 for map (%v -- %v) and tile %v/%v/%v -- %v.", req.mapName, m.Bounds, req.z, req.x, req.y, ext3857) + log.Error(msg) + http.Error(w, msg, http.StatusNotFound) + return + } + + ext4326 := &geom.Extent{} + copy(ext4326[:], points4326) + if _, intersect := m.Bounds.Intersect(ext4326); !intersect || !ok { + msg := fmt.Sprintf("map (%v -- %v) does not contains tile at %v/%v/%v -- %v", req.mapName, m.Bounds, req.z, req.x, req.y, ext4326) log.Debug(msg) http.Error(w, msg, http.StatusNotFound) return