Opened 5 years ago

Closed 5 years ago

#4311 closed enhancement (fixed)

MVT: Introduce wagyu to clip and validate polygon

Reported by: Algunenano Owned by: pramsey
Priority: medium Milestone: PostGIS 3.0.0
Component: postgis Version: master
Keywords: Cc:

Description

After I commented on the possibility of introducing wagyu to validate MVT polygons inside Postgis [1], I came up with several improvements that I've already committed and I have a final PR [3] that only contains the changes to introduce Wagyu as an option during configure (--with-wagyu).

The sort summary is that on the worst case, Wagyu is as fast as GEOS and it the best test case, it's 34x faster. The main caveat is that some small polygons might be dropped due to how it handles tolerance (but I haven't seen any visual impact for it).

It's currently bringing wagyu master as-is. Even though I made some changes for performance and to add interruptions, they have been accepted so the code is kept unmodified from upstream.

Next steps:

  • I plan to move uthash.h to the new deps folders to make it clear that the code comes from other project and it's covered by a different license. We should also check if we want to apply an update based on upstream.
  • Similarly the .proto files could be moved.
  • At some point in the future I want to give it a try and enable wagyu as default and see how bots react to it (it requires a C++11 compiler and I'm not fully confident on the mix with autotools). It works in my machine…

References:

1- Initial email: https://lists.osgeo.org/pipermail/postgis-devel/2018-December/027613.html

2- Thread: https://lists.osgeo.org/pipermail/postgis-devel/2018-December/thread.html#27613

3 - Github PR: https://github.com/postgis/postgis/pull/356

Change History (3)

comment:1 by robe, 5 years ago

@Alguenano, It looks like we are including wagyu code in our code base. Is that correct or am I misreading? If so what's the reasoning behind that instead just having a wget call or something.

I guess my main concern is it seems to easy to fork from the original wagyu project, unless we consider it so stable that forking is a necessity.

comment:2 by Algunenano, 5 years ago

It looks like we are including wagyu code in our code base. Is that correct or am I misreading? If so what's the reasoning behind that instead just having a wget call or something.

A little of backstory: Initially I considered the idea of using system packages for it, but only some distro's distribute it (no OSX, Windows…) and even the ones that do won't have the latest changes that I had to do to integrate it with Postgis. Those changes are now merged upstream but there isn't a release with it.

My second option would have been to use git submodules; I'm not a big fan of them but they are available wherever you have git. The equivalent for svn seems to be svn:externals but I certainly don't want to deal with mixing Wagyu's git with our svn.

Now that all the changes are merged upstream, using something like wget is an option. But thinking about it, it means downloading 2 projects (it uses geometry.hpp and wagyu), validate them (shaXX) and keeping them up to date when you run either autogen.sh or configure. I don't expect many updates from upstream, but some might happen and doing package / source validation using autotools in a portable way seems extremely hard.

The last option I could come up with was to push the sources into a directory in Postgis. It isn't great since, as you say, it's easy to deviate from the main project but it will work for everyone no matter what OS they're running. The nice part is that to update the libraries, at least right now, you just need to copy and paste the include folder from the 2 projects into deps/wagyu and you are done.

comment:3 by Raul Marin, 5 years ago

Resolution: fixed
Status: newclosed

In 17231:

Integrate wagyu to validate MVT polygons

Closes https://github.com/postgis/postgis/pull/356
Closes #4311

Note: See TracTickets for help on using tickets.