[Bug fix] box points ordering

[wybryan]

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

This PR tries to fix bounding box point ordering issue I reported at https://github.com/open-mmlab/mmocr/issues/1143

Modification

bug fix

BC-breaking (Optional)

Don't think so.

[gaotongxiao]

Thanks for your PR! However, the tests did not seem to pass.

In fact, we have long been aware of the bug of sort_points and the existence of its alternative in mmocr.datasets.pipelines.box_utils.sort_vertex, but did not have time to compare their implementation and eliminate the duplicate. Could you try sort_vertex as well see if it can solve your issue?

[wybryan]

Thanks for your PR! However, the tests did not seem to pass.

In fact, we have long been aware of the bug of sort_points and the existence of its alternative in mmocr.datasets.pipelines.box_utils.sort_vertex, but did not have time to compare their implementation and eliminate the duplicate. Could you try sort_vertex as well see if it can solve your issue?

Thanks for your PR! However, the tests did not seem to pass.

In fact, we have long been aware of the bug of sort_points and the existence of its alternative in mmocr.datasets.pipelines.box_utils.sort_vertex, but did not have time to compare their implementation and eliminate the duplicate. Could you try sort_vertex as well see if it can solve your issue?

I'll try sort_vertext.

regarding the test failure, I notice there is another error in the test itself: look at code

points = np.array([[1, 1], [1, -1], [-1, 1], [-1, -1]]) target = np.array([[-1, -1], [-1, 1], [1, 1], [1, -1]]) assert np.allclose(target, sort_points(points))

Isn't the target is wrong? it is anti-clockwise, not clock-wise. This is the reason the proposed solution 'fail' the test, where the test reference is not right...

[wybryan]

Hi, the test is now passing when incorrect test reference gets fixed now.

Do you know the test scripts for sort_vertex so I can test it also?

It is my impression that sort_vertex implementation seems quite cumbersome even if it is correct, therefore I'd like to use same test suite to test my proposal so see if it all works, I see there is opportunity to optimize such point ordering function for mmocr once for all.

[gaotongxiao]

@wybryan The target might or might not be wrong depending on the coordinate system of the external library that uses the polygon. We need a quick investigation before we set up a gold standard for all the relevant implementations. And sort_vertex doesn't come with a test suite.