Skip to content

Conversation

ear7h
Copy link
Contributor

@ear7h ear7h commented Aug 22, 2019

note: this pr had a different name because I originally wrote this as part of the changes in 46e9eeb

This pr has two major changes to remove the tegola.Tile from the encode function. The first is related to clipping and the second to the epsilon value. I've written the explicit changes below bc they can be hard to follow.

This pr also has new tests for Encode that test the clipping mechanism. This required a change in the test provider to be able to specify the features it contains.

clipping

tegola/tile.go

Lines 101 to 104 in 46e9eeb

t.bufpext = &geom.Extent{
0 - t.Buffer, 0 - t.Buffer,
t.Extent + t.Buffer, t.Extent + t.Buffer,
}

tegola/tile.go

Lines 192 to 194 in 46e9eeb

func (t *Tile) PixelBufferedBounds() (bounds [4]float64, err error) {
return t.bufpext.Extent(), nil
}

tegola/atlas/map.go

Lines 208 to 213 in 46e9eeb

pbb, err := tegolaTile.PixelBufferedBounds()
if err != nil {
return fmt.Errorf("err calculating tile pixel buffer bounds: %v", err)
}
clipRegion = geom.NewExtent([2]float64{pbb[0], pbb[1]}, [2]float64{pbb[2], pbb[3]})

and the new:

tegola/atlas/map.go

Lines 208 to 211 in 428bd2c

clipRegion = (&geom.Extent{
0, 0,
float64(m.TileExtent), float64(m.TileExtent),
}).ExpandBy(float64(m.TileBuffer))

epsilon

Current:

tegola/tile.go

Lines 209 to 225 in 46e9eeb

// This is from Leafty
func (t *Tile) ZEpislon() float64 {
if t.Z == MaxZ {
return 0
}
epi := t.Tolerance
if epi <= 0 {
return 0
}
ext := t.Extent
denom := (math.Exp2(float64(t.Z)) * ext)
e := epi / denom
return e
}

New:

// This is from Leafty
func ZEpislon(z uint, tileExtent float64) float64 {
if z == tegola.MaxZ {
return 0
}
epi := DefaultTolerence
if epi <= 0 {
return 0
}
denom := (math.Exp2(float64(z)) * tileExtent)
e := epi / denom
return e
}

@ear7h ear7h requested review from ARolek and gdey as code owners August 22, 2019 20:49
@coveralls
Copy link

coveralls commented Aug 22, 2019

Pull Request Test Coverage Report for Build 1795

  • 8 of 22 (36.36%) changed or added relevant lines in 3 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.9%) to 44.626%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/tegola/cmd/cache/worker.go 0 2 0.0%
atlas/map.go 8 11 72.73%
atlas/atlas.go 0 9 0.0%
Files with Coverage Reduction New Missed Lines %
atlas/atlas.go 1 5.06%
atlas/map.go 1 62.75%
Totals Coverage Status
Change from base Build 1787: -0.9%
Covered Lines: 24740
Relevant Lines: 55439

💛 - Coveralls

@ear7h ear7h changed the title mvt: fix bugs introduced from refactor into mvt atlas: remove usage of tegola.Tile Aug 22, 2019
@ear7h
Copy link
Contributor Author

ear7h commented Aug 22, 2019

Since it wasn't much, I also removed the last use of tegola.Tile from the atlas package and added it to this PR. However, I ran into some inconsistency between SeedMapTile and PurgeMapTile, I think they should both use a tile or both use a zxy.

func (a *Atlas) SeedMapTile(ctx context.Context, m Map, z, x, y uint) error {

func (a *Atlas) PurgeMapTile(m Map, tile *slippy.Tile) error {

I'm leaning towards the tile...

@ARolek
Copy link
Member

ARolek commented Aug 28, 2019

@ear7h let's use the tile for SeedMapTile.

Copy link
Member

@ARolek ARolek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good, thanks for tackling it. I have a few comments inline. Also, if tegola tile is no longer being used, is there any reason to not purge it with the commit?

@ARolek
Copy link
Member

ARolek commented Aug 30, 2019

@ear7h this looks great. Please clean up the commit history and then I will rebase it into v0.11.x

@ear7h
Copy link
Contributor Author

ear7h commented Sep 25, 2019

I had to rebase, but I was running into merge issues and simply applied the commit 3ff9ac6 to v0.11.x. GH automatically closed this because I accidentally pushed freshly created branch without applying the commit.

@ear7h ear7h reopened this Sep 25, 2019
* added tests for clipped geometries in Encode and fix buffer bug
* remove tegola.Tile from encode function
* remove tegola.Tile from map and cmd packages
* remove tile*.go files but keep default constants
* use tile instead of zxy params in SeedMapTile
Copy link
Member

@ARolek ARolek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just remove encode-cleanup.patch from the commit and then we're good.

@ARolek ARolek merged commit 0dddf97 into v0.11.x Sep 27, 2019
@ARolek ARolek deleted the encode-cleanup branch September 27, 2019 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants