Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turf tangents fails in some cases again #785

Closed
dhivehi opened this issue Jun 11, 2017 · 8 comments
Closed

Turf tangents fails in some cases again #785

dhivehi opened this issue Jun 11, 2017 · 8 comments
Assignees
Labels
Milestone

Comments

@dhivehi
Copy link
Collaborator

dhivehi commented Jun 11, 2017

Sample data for test:
screenshot-dhivehi mv-2017-06-11-15-14-10

var point = {"type":"Feature","geometry":{"type":"Point","coordinates":[73.5755476513307,0.4323399237340624]},"properties":null};

var poly  = {"type":"Feature","geometry":{"type":"Polygon","coordinates":[[[73.57502193836378,0.43239356638694915],[73.57526333717512,0.43281197906664204],[73.57561738876508,0.43281197906664204],[73.57560129551099,0.4325384015479159],[73.57543499855207,0.4324686661003909],[73.57551010040449,0.43223263842709514],[73.57579441456006,0.43239893065221224],[73.57609482196972,0.4324364805090113],[73.57643278030561,0.4323184666727826],[73.57594998268293,0.4322380026924719],[73.5754779138963,0.4318464113095217],[73.5751184978883,0.4320448891362787],[73.57516141323254,0.4322862810807635],[73.57500048069166,0.43227018828469],[73.57502193836378,0.43239356638694915]]]},"properties":null};

tangents = turf.polygonTangents(point,poly);
@DenisCarriere
Copy link
Member

@rowanwins any thoughts?

@rowanwins
Copy link
Member

Gday @dhivehi

Thanks for the test case, I'll take a look into it and see what I can find.

@DenisCarriere
Copy link
Member

👍 @rowanwins's PR fixes this issue.

image

DenisCarriere added a commit that referenced this issue Jun 17, 2017
Fix for @turf/polygon-tangents - Resolves #785
DenisCarriere pushed a commit that referenced this issue Jun 18, 2017
* added fiji + resolute-bay tests to translate

* added fiji + resolute-bay test to rotate

* Add new modules to Turf core

* v4.4.0

* Fix tests for blank dependencies

* Add inside tests

* New @turf/isolines based on MarchingSquares.js (#781)

* replaced conrec with marchingsquare

* fixed script and tests

* updated bench

* updated readme

* updated package.json

* updated index.d.ts

* introduced suggested changes

* added zProperty to index.d.ts

* minor doc change

* Updates to Isolines
- Divide properties={} to two params
- Simplify Catch error logic (index.js)
- Update Typescript definition
- Add types.ts to test Typescript definition
- Add single task to benchmark
- Update Yarn.lock
- Simplify bench & test logic
- Save fixtures out to GeoJSON
CC: @stebogit

* perIsoline overlaps properties from toAllIsolines

* Update Readme

* Buffer (#786)

Buffer - Drop circle buffer operation

* removed turf-isolines old test files

* copied files from  repo

* refactored index and test

* updated README

* updated index.d.ts and bench

* added yarn.lock

* removed Polygon as accepted input; added array test;

* added masking feature to turf-grid

* added throws tests

* updated yarn.lock

* added contributor

* Resolves #785

* Add test fixture

* Update complex unkink-polygon

* Update name to boolean-clockwise

* Declare input as line instead of Feature
@dhivehi
Copy link
Collaborator Author

dhivehi commented Oct 20, 2017

image

Again failed in this case also:

var point = {"type":"Feature","geometry":{"type":"Point","coordinates":[73.2258944382542,0.6673910301706059]},"properties":null};
var poly = {"type":"Feature","geometry":{"type":"Polygon","coordinates":[[[73.222696781158,0.672929312864639],[73.223141775131,0.6728221177272928],[73.223743934789,0.6726381749213459],[73.224029146338,0.6725036554756088],[73.224338813812,0.6724074327878782],[73.224860170232,0.6721935244108295],[73.225509744573,0.6720420390183648],[73.226084583476,0.6717472629325556],[73.226458847279,0.6715981449324744],[73.226650858421,0.6715436544821927],[73.227258045226,0.6714184949730679],[73.227445127964,0.6713529476262039],[73.227678178196,0.6712050062728139],[73.228031831012,0.6708426157704537],[73.228146526963,0.6706682403694231],[73.228457655945,0.6700969051734518],[73.228657754266,0.6698443347108025],[73.22884159707,0.6696673096285082],[73.228996763994,0.6695767853640717],[73.229106436541,0.6695525110064722],[73.229149940014,0.6695025217523778],[73.229169917106,0.6693944039229791],[73.229167029882,0.6693046080477387],[73.229084080861,0.6691702157268935],[73.229041067618,0.6691383134431845],[73.228922636334,0.6690945712344814],[73.22885162189,0.6690290941501615],[73.228795706928,0.6690004793602498],[73.228683117007,0.6689698982356589],[73.228615358483,0.6689786536104236],[73.228546747123,0.6690229904581599],[73.228490368271,0.669096495729903],[73.228443036787,0.6694061168332297],[73.228223152757,0.6698214354820493],[73.228165577041,0.6698999456480976],[73.228024344452,0.670025254964898],[73.227903388313,0.6702371606698421],[73.227836480364,0.6703046163993349],[73.227119555512,0.6707099538733416],[73.22668329446,0.6710793258441896],[73.226559966198,0.6711543446100734],[73.226424521696,0.6712119748989664],[73.226177066949,0.6712730394161497],[73.225950830598,0.67127542067891],[73.225655714655,0.6712369330303574],[73.22555065155,0.6711909636509574],[73.225483278402,0.6711332409315816],[73.225406283817,0.6710095253332611],[73.225390645993,0.6709432706487064],[73.225397197529,0.6708261252031065],[73.225457941492,0.6706975144888645],[73.225963781675,0.6703145215167154],[73.226412547476,0.669790217338786],[73.226639346285,0.6696012463551142],[73.226701757055,0.6695209006273473],[73.226727089028,0.6694367056162207],[73.226723397567,0.6693697679729524],[73.226656628151,0.6691877016677381],[73.226566725674,0.669003250939312],[73.226551314195,0.6689166059413054],[73.226568804017,0.668865677272052],[73.226787260285,0.6685663053432478],[73.226824706351,0.668476577946393],[73.226843488025,0.6683734115614755],[73.226823012034,0.668252158733111],[73.226760301619,0.6681455247222345],[73.226612638682,0.6680583776521587],[73.226456208552,0.6680175865484159],[73.226209720597,0.6680410701986119],[73.226154902776,0.668062388064314],[73.226070207392,0.6681289116730795],[73.225768983364,0.6684395167627741],[73.225699240456,0.6684703579247326],[73.22541866541,0.6684446126130723],[73.225309299131,0.6684913460441066],[73.225291081448,0.6684570449487524],[73.225256291015,0.668222172880931],[73.225226394924,0.6681784489895506],[73.225180595009,0.6681526846102344],[73.224978322105,0.6681633512328347],[73.224772984242,0.6682065624897859],[73.224741499018,0.668201968184249],[73.22471701051,0.668173830077734],[73.22470184438,0.66807906378871],[73.224710363746,0.6679383479043537],[73.224809658364,0.6676155759821256],[73.224856696195,0.6673534849849148],[73.224970303437,0.6670680268227471],[73.22512649074,0.6668365082036303],[73.225212097168,0.6667288667304803],[73.225274791718,0.6666840121064155],[73.225625809829,0.6666438269910628],[73.225738127231,0.666611311729028],[73.226002620068,0.6663804677868512],[73.226105900095,0.6662014314291156],[73.226124983675,0.6660370141104295],[73.226040943446,0.6658116107705752],[73.225815226237,0.6655119071691757],[73.225710327836,0.6654167649301144],[73.225627314532,0.6653661538076818],[73.225537743419,0.6653534272697499],[73.225433878899,0.6653717924575773],[73.225318119124,0.665562515457168],[73.225255542633,0.6657391979264418],[73.224956187609,0.6660264320867242],[73.224782510517,0.6663571798607677],[73.224617868401,0.6665229776088211],[73.224439704818,0.6668353595225085],[73.224300938349,0.6672443056588548],[73.22419991164,0.6676913082846596],[73.22406436316,0.6681564543334417],[73.223883887404,0.6690056083385798],[73.22377935234,0.6692590564152425],[73.223676624298,0.6694553729287946],[73.223596730253,0.6695715464665994],[73.223541862338,0.6696966344853763],[73.223345681691,0.6704022315888665],[73.223258191582,0.670645296332097],[73.222985252214,0.6715456004246505],[73.22285381536,0.6718248328859886],[73.222686846515,0.6721174107883314],[73.222454455099,0.6724137943929662],[73.222098727673,0.6728239052963403],[73.222041913757,0.6729435103004278],[73.222061824868,0.6729818899183755],[73.222099328875,0.6729992722487737],[73.222397657256,0.6729767617070905],[73.222696781158,0.672929312864639]]]}};
tangents = turf.polygonTangents(point,poly);

@dhivehi dhivehi changed the title Turf tangents fails in some cases Turf tangents fails in some cases again Oct 20, 2017
@DenisCarriere DenisCarriere reopened this Oct 24, 2017
@DenisCarriere DenisCarriere added this to the 5.1.0 milestone Oct 24, 2017
@rowanwins
Copy link
Member

Ok @dhivehi

So I've done a bit of digging. It looks like the fix I previously applied broke the module for other cases, and the previous implementation was more accurate.

It looks like there is a more fundamental problem with the algorithm when your input point is inside the concave hull of the polygon and lower than the first segment. My head is still grappling with the maths but I'll see what we can work out, in the meantime I suggest we revert back to the original code

@DenisCarriere
Copy link
Member

Interesting... I'm good with reverting it back 👍 The more test cases we have the more robust the library will become.

@rowanwins
Copy link
Member

This page (where the orig implementation came from) also lists a sweepline approach, apparently this is faster but Im not sure if it'll produce different results.

@rowanwins
Copy link
Member

Think I've now got this resolved by #1174 , has been released as v5.1.5 to npm

@DenisCarriere DenisCarriere modified the milestones: 5.2.0, 5.1.0 Dec 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants