search

t-rex-tileserver / t-rex

t-rex is a vector tile server specialized on publishing MVT tiles from your own data
https://t-rex.tileserver.ch/
MIT License
545 stars 68 forks source link

Added extra ST_MakeValid step which fixes self intersection issues #252

Closed chau-intl closed 2 years ago

chau-intl commented 2 years ago

I'm having a situation where my source MULTIPOLYGON (PostGIS 2.4.4 and 2.5.1) is incorrectly processed when creating a vector tile from it:

MULTIPOLYGON(((536841.95361167204100639 6226165.36378674302250147, 536931.37260000000242144 6226001.91550000011920929, 537001.33200000005308539 6225925.69120000023394823, 537147.38219999999273568 6225752.12420000042766333, 537278.61580000002868474 6225531.99039999954402447, 537439.48279999999795109 6225222.95650000032037497, 537479.69960000005085021 6225115.00619999971240759, 537606.69979999994393438 6225239.88980000000447035, 537676.70709999999962747 6225282.6084000002592802, 537724.55709999997634441 6225045.64840000029653311, 537793.36930000002030283 6225065.83409999962896109, 537765.40930000005755574 6225506.00409999955445528, 537765.29330000001937151 6225506.25490000005811453, 537654.10710000002291054 6225478.39759999979287386, 537683.45709999999962747 6225307.57770000025629997, 537643.30059999995864928 6225280.90029999986290932, 537573.8473000000230968 6225234.59819999989122152, 537481.24300000001676381 6225165.1448999997228384, 537348.95109999994747341 6225442.95789999980479479, 537233.19559999997727573 6225657.93230000045150518, 537065.70070000004488975 6225889.50750000029802322, 537056.30099999997764826 6226307.04030000045895576, 536841.95361167204100639 6226165.36378674302250147)))

In the vector tile I only get a small part of the expected result:

MULTIPOLYGON (((537352.3088447893 6225413.76868618, 537505.1829013594 6225184.457601325, 537581.6199296446 6225260.89462961, 537505.1829013594 6225108.02057304, 537428.7458730743 6225260.89462961, 537352.3088447893 6225413.76868618)), ((537658.0569579297 6225260.89462961, 537658.0569579297 6225337.331657895, 537658.0569579297 6225490.205714465, 537734.493986215 6225490.205714465, 537810.9310145001 6225031.583544754, 537734.493986215 6225031.583544754, 537658.0569579297 6225260.89462961)))

It seems like ST_SnapToGrid can cause self intersections which the immediately following ST_Buffer doesn't handle well. When I run ST_IsValid on the result of ST_SnapToGrid it return false and the immediately following ST_Buffer does a suboptimal job on the invalid geometry.

Adding a ST_MakeValid step after ST_SnapToGrid and before the ST_Buffer fixes my self intersections and the result looks much better:

MULTIPOLYGON (((537658.0569579297 6225260.89462961, 537658.0569579297 6225337.331657895, 537658.0569579297 6225490.205714465, 537734.493986215 6225490.205714465, 537810.9310145001 6225031.583544754, 537734.493986215 6225031.583544754, 537658.0569579297 6225260.89462961)), ((536817.2496467929 6226178.138969033, 537046.5607316484 6226331.013025602, 537046.5607316484 6225872.390855892, 537199.4347882167 6225643.079771037, 537122.9977599336 6225719.5167993205, 536970.1237033632 6225948.827884176, 536893.686675078 6226025.264912462, 536817.2496467929 6226178.138969033)), ((537199.4347882167 6225643.079771037, 537275.8718165039 6225566.64274275, 537352.3088447911 6225413.768686177, 537352.308844789 6225413.768686179, 537199.4347882187 6225643.079771034, 537199.4347882167 6225643.079771037)), ((537352.3088447911 6225413.768686177, 537505.1829013594 6225184.457601324, 537581.6199296446 6225260.89462961, 537505.1829013594 6225108.02057304, 537428.7458730743 6225260.89462961, 537352.3088447911 6225413.768686177)))

I'm not sure if the intention of the ST_Buffer(..., 0.0) was to fix issues like mine and in that case might ST_Buffer(ST_MakeValid(...), 0.0) be overkill?

I currently don't have an environment to test/run the code so I haven't created a test case for this PR. But feel free to use the WKT's to add tests.

pka commented 2 years ago

You proposed adding ST_MakeVaild in #206 already. But as far as I remember, I've replaced it with ST_Buffer(0) after problems like https://github.com/PDOK/t-rex/pull/2

Obviously there are still unsolved cases, so we should try again adding ST_MakeVaild.

tobwen commented 2 years ago

Please, never ever use ST_Buffer(0): https://github.com/t-rex-tileserver/t-rex/issues/206#issuecomment-922138321

pka commented 2 years ago

Changed ST_Buffer to ST_MakeVaild in 97bab19

Could you please test with 0.14.1-pre8

pka commented 2 years ago

BTW, Is there a public site which can be added to https://github.com/t-rex-tileserver/t-rex#examples ?

pka commented 2 years ago

Discussion is going on in #206 ...

chau-intl commented 2 years ago

Changed ST_Buffer to ST_MakeVaild in 97bab19

Could you please test with 0.14.1-pre8

I think you are missing an ) in the current line:

https://github.com/t-rex-tileserver/t-rex/blob/362c1fc15478572d73290ee12a9cf2be258392a2/t-rex-core/src/datasource/postgis_ds.rs#L341

pka commented 2 years ago

You're right! I've tested with t_rex serve --dbconn, which uses make_valid=false by default. Fixed the SQL syntax in 0.14.1-pre9.

chau-intl commented 2 years ago

We have tested the issue with the latest pre release pre9 and we have no immediate visible issues so far.

Thanks for fixing it! Will you or should I close this PR?

pka commented 2 years ago

Fixed via 7a3ef68