Another self-intersection corner case handling

[sergiev]

Hello again! This time I faced problem that seem very similar to the one from my previous PR #961

The neighboring points placed here in such order that gives us another invalid shape.

But this time my previous fix hadn't helped:

If you try to get intersection of original shape, it leads to empty intersection, which is the least expected behaviour (for me, at least).
So this time the solution is a bit tradeoff: you're not getting original shape, but only convex hull of it. If there's more accurate solution, I'll be happy to see it implemented in SAHI.

Below are information required to reproduce the bug:
Input image shape: h=2060, w=3790
slice_coco args: slice_height=slice_width=1280, overlap_height_ratio=overlap_width_ratio=0.02

Example of slightly corrupted annotation

{
      "area": 54169.0,
      "attributes": {
        "occluded": false
      },
      "bbox": [
        3205.99,
        1765.29,
        360.21,
        294.71
      ],
      "category_id": 1,
      "id": 3346,
      "image_id": 460,
      "iscrowd": 0,
      "segmentation": [
        [
          3410.71,
          1808.52,
          3416.11,
          1784.5,
          3435.92,
          1776.1,
          3454.54,
          1765.29,
          3472.55,
          1776.1,
          3472.55,
          1791.71,
          3473.75,
          1810.32,
          3461.74,
          1826.53,
          3459.34,
          1841.54,
          3449.73,
          1859.55,
          3458.74,
          1872.76,
          3468.34,
          1893.77,
          3476.75,
          1908.18,
          3482.75,
          1933.39,
          3501.96,
          1930.39,
          3524.78,
          1936.4,
          3542.79,
          1943.6,
          3565.0,
          1952.61,
          3566.2,
          1967.01,
          3548.19,
          1983.22,
          3532.58,
          1998.83,
          3530.78,
          2022.85,
          3540.99,
          2037.86,
          3559.0,
          2046.86,
          3540.49,
          2060.0,
          3540.9,
          2060.0,
          3512.3,
          2060.0,
          3469.2,
          2060.0,
          3422.6,
          2060.0,
          3371.4,
          2060.0,
          3326.9,
          2060.0,
          3278.3,
          2060.0,
          3249.7,
          2060.0,
          3239.61,
          2052.87,
          3234.8,
          2034.26,
          3211.39,
          2023.45,
          3205.99,
          1998.23,
          3227.0,
          1976.02,
          3248.61,
          1974.22,
          3264.22,
          1958.61,
          3287.63,
          1957.41,
          3300.84,
          1935.2,
          3320.65,
          1915.98,
          3325.46,
          1897.37,
          3339.27,
          1886.57,
          3348.27,
          1879.36,
          3347.07,
          1864.95,
          3360.88,
          1856.55,
          3381.89,
          1857.15,
          3395.1,
          1844.54,
          3399.9,
          1823.53
        ]
      ]
    }

[devrimcavusoglu]

Hi @sergiev thank you for your contribution, I also had a chance to look at previous related PR. Could you maybe add some test data and extend the tests for shapely_utils ? I think that way we would've more control over these utilities.

[sergiev]

@devrimcavusoglu hi, thanks for the reply! Sorry but I sincerely do not understand what and where exactly you want me to put in... Could you expand the idea?

If it's the suggestion to extend test_shapelyutils.py with some kind of def test_naughty_polygon, I surely can do it with the same polygon presented above.
Please, approve that I've got you right (or wrong)

[devrimcavusoglu]

@devrimcavusoglu hi, thanks for the reply! Sorry but I sincerely do not understand what and where exactly you want me to put in... Could you expand the idea?

If it's the suggestion to extend test_shapelyutils.py with some kind of def test_naughty_polygon, I surely can do it with the same polygon presented above. Please, approve that I've got you right (or wrong)

Hi @sergiev. Yes, I exactly meant that, ofc. this cases are visible in PRs, but it would be nice also to track them in the code as well, so that people contributing to the package know that these cases are filed, and they may be able to add more to that test case.

[fcakyon]

Hey @sergiev, thanks for your fantastic contribution!

Do you plan to add some tests for your case into test_shapelyutils.py?

[sergiev]

Thank you @fcakyon for reminding! I plan to find time for it in a week, will update this PR branch before June, if you let me :)

[fcakyon]

what do you think about this implementation @sergiev? It fixes the same problem in a different way.

https://github.com/obss/sahi/pull/1047/files

[sergiev]

hi @fcakyon!
I've tested this different way on problematic data presented above - and it works smoothly. But what I think now is that both fixes - mine and Alias-z's one - are too downstream. What if we put the other author fix directly in utils/shapely.py to cure invalid multipolygons at the reading stage?

def get_shapely_multipolygon(coco_segmentation: List[List]) -> MultiPolygon:
    """
    Accepts coco style polygon coords and converts it to valid shapely multipolygon object
    """
    def filter_polygons(geometry):
        """
        Filters out and returns only Polygon or MultiPolygon components of a geometry.
        If geometry is a Polygon, it converts it into a MultiPolygon.
        If it's a GeometryCollection, it filters to create a MultiPolygon from any Polygons in the collection.
        Returns an empty MultiPolygon if no Polygon or MultiPolygon components are found.
        """
        if isinstance(geometry, Polygon):
            return MultiPolygon([geometry])
        elif isinstance(geometry, MultiPolygon):
            return geometry
        elif isinstance(geometry, GeometryCollection):
            polygons = [geom for geom in geometry.geoms if isinstance(geom, Polygon)]
            return MultiPolygon(polygons) if polygons else MultiPolygon()
        return MultiPolygon()
    
    polygon_list = []
    for coco_polygon in coco_segmentation:
        point_list = list(zip(coco_polygon[0::2], coco_polygon[1::2]))
        shapely_polygon = Polygon(point_list)
        polygon_list.append(shapely_polygon)
    shapely_multipolygon = MultiPolygon(polygon_list)
    
    if not shapely_multipolygon.is_valid:
        shapely_multipolygon = filter_polygons(make_valid(shapely_multipolygon))
    
    return shapely_multipolygon

[fcakyon]

hi @fcakyon! I've tested this different way on problematic data presented above - and it works smoothly. But what I think now is that both fixes - mine and Alias-z's one - are too downstream. What if we put the other author fix directly in utils/shapely.py to cure invalid multipolygons at the reading stage?

def get_shapely_multipolygon(coco_segmentation: List[List]) -> MultiPolygon:
    """
    Accepts coco style polygon coords and converts it to valid shapely multipolygon object
    """
    def filter_polygons(geometry):
        """
        Filters out and returns only Polygon or MultiPolygon components of a geometry.
        If geometry is a Polygon, it converts it into a MultiPolygon.
        If it's a GeometryCollection, it filters to create a MultiPolygon from any Polygons in the collection.
        Returns an empty MultiPolygon if no Polygon or MultiPolygon components are found.
        """
        if isinstance(geometry, Polygon):
            return MultiPolygon([geometry])
        elif isinstance(geometry, MultiPolygon):
            return geometry
        elif isinstance(geometry, GeometryCollection):
            polygons = [geom for geom in geometry.geoms if isinstance(geom, Polygon)]
            return MultiPolygon(polygons) if polygons else MultiPolygon()
        return MultiPolygon()
    
    polygon_list = []
    for coco_polygon in coco_segmentation:
        point_list = list(zip(coco_polygon[0::2], coco_polygon[1::2]))
        shapely_polygon = Polygon(point_list)
        polygon_list.append(shapely_polygon)
    shapely_multipolygon = MultiPolygon(polygon_list)
    
    if not shapely_multipolygon.is_valid:
        shapely_multipolygon = filter_polygons(make_valid(shapely_multipolygon))
    
    return shapely_multipolygon

That makes a lot of sense! Would you be open to creating a pull request for a more general solution based on your suggestion? @sergiev

[sergiev]

Hi @fcakyon! I will be happy to. Don't you mind if I put the suggestion in this PR?

[fcakyon]

Hi @fcakyon! I will be happy to. Don't you mind if I put the suggestion in this PR?

Yes would be great!

[sergiev]

Hi @fcakyon i've just added everything discussed above

but can't pass CI, though formatting with black. What I am missing?

[sergiev]

Hi @fcakyon!
CI checks passed
now only review is required.

[sergiev]

hi @devrimcavusoglu

CI checks passed
now only review is required.