geos icon indicating copy to clipboard operation
geos copied to clipboard

Optimizations to RingClipper implementation

Open dbaston opened this issue 3 years ago • 3 comments

  • Inline trivial methods
  • Use std::vector instead of CoordinateSequence internally
  • Avoid some Coordinate copies

dbaston avatar Dec 05 '21 21:12 dbaston

It seems to cause an infinite loop in MSVC for some reason. (https://github.com/libgeos/geos/runs/4477323616?check_suite_focus=true)

dbaston avatar Jan 10 '22 23:01 dbaston

Just that one version too, very impressive.

pramsey avatar Jan 10 '22 23:01 pramsey

The "infinite loop" turned out to be the result of MSVC trying to pop up an "assertion failed" dialog box on an Azure server somewhere with nobody to click "OK." I fixed that error but can no longer verify a performance improvement so I'm holding this for now.

dbaston avatar Feb 07 '22 14:02 dbaston

Shouldn't this and #605 be overlaid.

robe2 avatar Nov 22 '22 01:11 robe2

I can revisit this if/after #721 is merged.

dbaston avatar Nov 22 '22 01:11 dbaston

If we wanted to optimize this (and I'm not sure it's really warranted), I think the time is probably better directed bringing in a more efficient algorithm. Some interesting ones are implemented by QGIS in https://github.com/qgis/QGIS/blob/84c3613cf7f4fae1c6b872956a7784a83a135653/src/core/qgsclipper.h

dbaston avatar Mar 01 '23 00:03 dbaston