Vector tiles: increase the size of the envelope used to clip geometries#79030
Vector tiles: increase the size of the envelope used to clip geometries#79030iverase merged 6 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
|
@elasticmachine run elasticsearch-ci/packaging-tests-windows-sample |
1 similar comment
|
@elasticmachine run elasticsearch-ci/packaging-tests-windows-sample |
| final Envelope clipEnvelope = new Envelope(tileEnvelope); | ||
| clipEnvelope.expandBy(this.pixelPrecision, this.pixelPrecision); | ||
| // expand enough the clip envelope to prevent visual artefacts | ||
| clipEnvelope.expandBy(10 * this.pixelPrecision, 10 * this.pixelPrecision); |
There was a problem hiding this comment.
Not sure I quite understand where this 10 is coming from?
There was a problem hiding this comment.
That is a good point as it was mainly experimental. A quick search got me this info: https://blog.cyclemap.link/2020-01-25-tilebuffer/
In this blog, it is recommended depending on the zoom:
| zoom | Cell buffer size (pixels) |
|---|---|
| < 15 | extent / 64 |
| 15..17 | extent / 32 |
| > 17 | extent / 16 |
Much bigger than what I am proposing. Maybe @thomasneirynck can give a suggestion here?
There was a problem hiding this comment.
from the blog you linked, it seems to be focused on rendering the tiles to rasters client-side.
I'd like to try it out in Kibana Maps, but 90% sure we would not be running into these "line-connection" artifacts with mapbox with a small buffer. Fwiw - these artifacts from the blog arise due to line-connections, so we should try with a line dataset.
The main desired outcome (from kibana-maps perspective) is that the tile-outline is omitted from the actual tile-contents (since they would have negative tile-coords). Which seems to be the case with this PR.
Perhaps @jsanz @nickpeihl can weigh in: are we applying a similar buffer when generating the EMS tiles? if so, how large?
There was a problem hiding this comment.
Every layer on the basemap has its own buffer size that is used to compute the final tile buffer used by the PostGIS ST_AsMVTGeom function, based also on the zoom level as you can see in this line.
tile_buffer_size = int(self.extent * layer.buffer_size / self.pixel_width)The most common buffer size for the basemap layers is 4 but for point layers like points of interest or place names, the buffer is bigger, I understand to get the labels properly rendered.
Buffer sizes
There was a problem hiding this comment.
Thanks @thomasneirynck and @jsanz!
I am not sure I understand the formula above. For the discussion here it seems that the buffer size in that case is expressed in different system and pixel_width is used to transform between both systems (It does not seem to be dependent on zoom level).
I agree that what we want is to remove the tile outline by having negative coordinates on the polygons that cross the tile. I checked with line geometries and they are not affected.
Therefore my proposal is to set a default buffer size of 5 pixels and later on we can expose this parameter so users can have control over it. Any thoughts?
imotov
left a comment
There was a problem hiding this comment.
Could you add a comment in the code with TODO list explaining our thinking here and how we came up with this magic number. It might be also good to pull it out as a constant.
|
I created a static variable to hold the default buffer size in pixels and added a TODO. |
…es (elastic#79030) Removes the tile outline from polygons that cross contiguous tiles.
…es (elastic#79030) Removes the tile outline from polygons that cross contiguous tiles.
* upstream/master: (34 commits) Add extensionName() to security extension (elastic#79329) More robust and consistent allowAll indicesAccessControl (elastic#79415) Fix circuit breaker leak in MultiTerms aggregation (elastic#79362) guard geoline aggregation from parents aggegator that emit empty buckets (elastic#79129) Vector tiles: increase the size of the envelope used to clip geometries (elastic#79030) Revert "[ML] Add queue_capacity setting to start deployment API (elastic#79369)" (elastic#79374) Convert token service license object to LicensedFeature (elastic#79284) [TEST] Fix ShardPathTests for MDP (elastic#79393) Fix fleet search API with no checkpints (elastic#79400) Reduce BWC version for transient settings (elastic#79396) EQL: Rename a test class for eclipse (elastic#79254) Use search_coordination threadpool in field caps (elastic#79378) Use query param instead of a system property for opting in for new cluster health response code (elastic#79351) Add new kNN search endpoint (elastic#79013) Disable BWC tests Convert auditing license object to LicensedFeature (elastic#79280) Update BWC versions after backport of elastic#78551 Enable InstantiatingObjectParser to pass context as a first argument (elastic#79206) Move xcontent filtering tests (elastic#79298) Update links to Fleet/Agent docs (elastic#79303) ...
While testing we realise we have some visual artefacts when displaying big polygons. These artefacts are noticed because you can see the edges of the tiles, for example:
This is due to the size of the clipping envelope that is not big enough. Therefore we propose to increase from
1 pixel precisionto10 pixel precision. After this change the artefacts are gone: