Skip to content

With buffer: 0, geometry coincident with a tile edge is clipped inconsistently#102

Merged
mourner merged 3 commits intomasterfrom
bufferless-tests
May 3, 2018
Merged

With buffer: 0, geometry coincident with a tile edge is clipped inconsistently#102
mourner merged 3 commits intomasterfrom
bufferless-tests

Conversation

@jfirebaugh
Copy link
Copy Markdown
Contributor

When using geojson-vt to generate bufferless tiles, a LineString geometry that is coincident with a tile edge is clipped inconsistently:

  • LineString geometry that is coincident with a horizontal (top/bottom) tile edge appears only in the top tile (along its bottom edge).
  • LineString geometry that is coincident with a vertical (left/right) tile edge appears in both tiles that share that edge.

It should be consistent one way or the other -- although possibly different from either of these two behaviors. For bufferless tiles in VT3, we're thinking of formally specifying that tiles are effectively inclusive of their top and left edges, and exclusive of their bottom and right edges, such that the correct behavior in this case is to include this type of coincident geometry in only the bottom and right tiles.

cc @mourner @anandthakker

@mourner
Copy link
Copy Markdown
Member

mourner commented May 2, 2018

The issue here is that geojson-vt currently clips inclusively (which happens in the passing test), but the second test fails because the clipping line (0.25) doesn't exactly match the projected y coordinate, which due to numerical errors equals 0.24999999999999983.

A fix for that will be to clip one-directionally — inclusive on one side and exclusive on the other, just like the VT3 proposal. I'll push a fix and update the tests to reflect this.

@mourner
Copy link
Copy Markdown
Member

mourner commented May 2, 2018

@jfirebaugh check this out now. Note that in the tests, one getTile returns null while another one {features: []}, which is an unrelated issue (geojson-vt returns "empty" tiles instead of null when the parent tile has non-empty children).

@jfirebaugh
Copy link
Copy Markdown
Contributor Author

Looks good to me, and it passes the tests in gl-js that were failing before. 👍

@mourner mourner merged commit 7226705 into master May 3, 2018
@mourner mourner deleted the bufferless-tests branch May 3, 2018 07:03
@mourner
Copy link
Copy Markdown
Member

mourner commented May 3, 2018

@jfirebaugh released as v3.1.1.

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.

2 participants