[postgis-tickets] [PostGIS] #4183: Invalid output of ST_ClipByBox2D with an invalid polygon

[PostGIS]

#4183: Invalid output of ST_ClipByBox2D with an invalid polygon
-------------------------+--------------------------
  Reporter:  Algunenano  |      Owner:  Algunenano
      Type:  defect      |     Status:  assigned
  Priority:  medium      |  Milestone:  PostGIS GEOS
 Component:  postgis     |    Version:  trunk
Resolution:              |   Keywords:
-------------------------+--------------------------

Comment (by Algunenano):

 I've been going deeper into this issue and ended up reviewing the code in
 Geos that does the clipping.

 Here is the geometry that exemplifies this issue:
 ![image](https://user-
 images.githubusercontent.com/664253/45883295-f4937b80-bdb0-11e8-8456-c24ccbdfee6b.png)

 If you take the order used by the orange arrow (right hand order) then
 when the clipping happens you'll get the correct part of the geometry; on
 the other hand, if you get the green arrow (left hand order) you will get
 the opposite (the whole box minus that part of the geometry). The issue
 comes from the fact that this geometry is invalid so the part of the
 geometry getting clipped is in the wrong winding order (left hand).

 I've been thinking about how to fix it and the way I've come up with is to
 get the points where the geometries cross and check whether the
 intermediate point (or the corner of the box if the points are in
 different sides of the box) is inside the geometry or not.
 Doing this involves a Point in Polygon check which is not fast, and
 forcing this check for all geometries (even the ones that are valid) would
 mean a big hit in performance in a function whose main purpose is to be a
 fast and dirty alternative to `St_Intersection` when the second geometry
 is a box.

 With this in mind, I've decided to explore an alternative solution: check
 if the resulting geometry's bbox is inside the bbox of the geometry (as it
 should normally) and, if not, try to fix the geometry and relaunch again.
 This should have a negligible impact in performance for the most common
 case (valid input) and fix this issue.

 My main concern is whether to do this only for `St_AsMVTGeom` or in
 `lwgeom_clip_by_rect` itself If done only in St_AsMVTGeom we could,
 instead of fixing the geometry, just drop it (as done by Mapnik); and if
 done in `lwgeom_clip_by_rect` it would help everyone relying on that
 function (`St_SnaptoGrid`) with the caveat of hiding geometry changes to
 the user, which is something that was discussed during 2.5 and postponed
 to 3.0.

 I'm open to ideas, but I think that for now (and backporting to 2.4 and
 2.5) I'd take the easy way out (drop that geometry in St_AsMVTGeom) and,
 whenever some decision is taken as to how to handle invalid geometries
 inside Postgis, adapt St_AsMVTGeom (or `lwgeom_clip_by_rect`) do follow
 that behaviour.

-- 
Ticket URL: <https://trac.osgeo.org/postgis/ticket/4183#comment:4>
PostGIS <http://trac.osgeo.org/postgis/>
The PostGIS Trac is used for bug, enhancement & task tracking, a user and developer wiki, and a view into the subversion code repository of PostGIS project.