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

Triangulation does not preserve original edges #74

Open
zzfoo opened this issue Dec 8, 2016 · 18 comments
Open

Triangulation does not preserve original edges #74

zzfoo opened this issue Dec 8, 2016 · 18 comments

Comments

@zzfoo
Copy link

zzfoo commented Dec 8, 2016

earcut

In the above image, the two neighbor triangles (where the red arrow points) are not fully connected, while all other neighbor triangles are.

(please let me know if I didn't make myself clear.)

@zzfoo
Copy link
Author

zzfoo commented Dec 12, 2016

@mikelmaron @hjanetzek @zugaldia @incanus @springmeyer
sorry to disturb, but this issue is important to me. can any of you guys answers it for me? thanks in advance

@finscn
Copy link

finscn commented Dec 12, 2016

This issue troubles me too.

@finscn
Copy link

finscn commented Dec 12, 2016

@mikelmaron @hjanetzek @zugaldia @incanus @springmeyer @zzfoo

I changed pointInTriangle() to

// check if a point lies within a convex triangle
function pointInTriangle(ax, ay, bx, by, cx, cy, px, py) {
    return (cx - px) * (ay - py) - (ax - px) * (cy - py) > 0 &&
           (ax - px) * (by - py) - (bx - px) * (ay - py) > 0 &&
           (bx - px) * (cy - py) - (cx - px) * (by - py) > 0;
}

change >= to > .

It could solve this problem , but I don't know whether it is the best way.

My test case :

var testPoints = [
    [
        [10, 10], [300, 10], [10, 300]
    ],
    [
        [50, 50], [100, 50], [50, 100]
    ],
    [
        [150, 50], [200, 50], [150, 100]
    ],
];

Just replace the testPoints in viz.js

.

@mourner
Copy link
Member

mourner commented Dec 12, 2016

Is this behavior causing any particular issues? It seems like a result I would expect.

@finscn
Copy link

finscn commented Dec 12, 2016

@mourner

Current result of earcut is :
2016-12-13 12 16 26

I would expect is :
2016-12-13 12 16 39

I think the result of Polygon triangulation should be a mesh, One border could most belong to two triangles.

@finscn
Copy link

finscn commented Dec 12, 2016

If one border shared to many triangles , the triangulation will lost the value in the real-world(e.g. : NavMesh Pathfinding , Mesh Transform )

NavMesh Pathfinding:
ccs-8549-0-63277200-1309293937

Mesh Transform:
b6f313e54d2c51d05c7304e676ee5965

Maybe earcut would not be used in those use case , but it's the base of Polygon triangulation,
I hope it could support this feature.

@finscn
Copy link

finscn commented Dec 12, 2016

And , there is result of current version earcut :
2016-12-13 12 42 48

You look the triangles below two rect holes , they are correct,
But the triangles above two rect holes are wrong.

@finscn
Copy link

finscn commented Dec 12, 2016

@mourner , there is a document about Triangulation :
https://www.geometrictools.com/Documentation/TriangulationByEarClipping.pdf
It said that :

2016-12-13 1 14 04

The key point : "Each vertex shares exactly two edges. The only place where edges are allowed to intersect are at the vertices."

In the famous & classic book "Geometric Tools for Computer Graphics" , there is similar claim :

2016-12-13 1 21 19

The key point : "The edges are only allowed to intersect at the vertices. "

.

In fact, by definition, the result of current version earcut in my test case is not Triangulation at all.

.
I'm a Chinese guy , with bad english , I hope you could understand me.
If my words is impolite , I hope you could forgive me . It's not because I'm impolite , just because my poor english , I don't know how to express my opinion in the right way.

And the most words above are translated by google.

.

@finscn finscn mentioned this issue Dec 12, 2016
@mourner
Copy link
Member

mourner commented Dec 12, 2016

I see the issue now — thanks for the explanation. I don't have immediate suggestions to fix this — this problem might stem from the original tactics from https://www.cosy.sbg.ac.at/~held/projects/triang/triang.html to deal with degenerate cases by filtering out collinear points at various stages of the algorithm. Needs investigation.

@finscn
Copy link

finscn commented Dec 12, 2016

Thank you , @mourner .

And there is a better test case :

//  replace the testPoints in viz.js.
var testPoints = [
    [
        [10, 10],
        [300, 10],
        [300, 300],
        [10, 300]
    ],
    [
        [50, 50],
        [100, 50],
        [100, 100],
        [50, 100]
    ],

    [
        [150, 50],
        [200, 50],
        [200, 100],
        [150, 100]
    ],

    [
        [50, 150],
        [100, 150],
        [100, 200],
        [50, 200]
    ],

    [
        [150, 150],
        [200, 150],
        [200, 200],
        [150, 200]
    ],
];

2016-12-13 2 48 01

My solution can't solve this bug in this test case, So I think this test case is more representative.

@finscn
Copy link

finscn commented Dec 12, 2016

@mourner , I think the bug at eliminateHoles().

The node linkList that return value of eliminateHoles in my test case is this :
link-bug

I think the correct one should be like this :
link-right

And , whatever , 10 ,11 ,12 will be in one line , the filterPoints will remove 11.

@zzfoo
Copy link
Author

zzfoo commented Dec 13, 2016

@mourner I'm using earcut.js to divide the walkable area of our game map into triangles. I was expecting all the triangles are fully connected with their neighbors such that game characters can walk from one triangle area to another unobstructedly as long as they are walking through the common edge.
anyway, if this bug can't be fixed in the near future, then I have to find another solution. thanks a lot for your reply.

finscn pushed a commit to finscn/earcut that referenced this issue Dec 15, 2016
To fix mapbox#74

Because the algorithm has been changed , the `expectedTriangles` & `expectedDeviation` in the test cases will have to changed.
So the current test case will not valid for this PR.
@mourner mourner changed the title should this be considered a bug? Triangulation does not preserve original edges Dec 15, 2016
@finscn
Copy link

finscn commented Dec 24, 2016

@mourner , I create a new version at : https://github.com/finscn/earcut/blob/master/src/earcut.js
It just can't pass only 3 test cases : 'steiner', 'water-huge', 'water-huge2'.


And for steiner test , the current earcut can't get correct result too.
It's missing the blue line.

2016-12-25 3 59 06

@finscn
Copy link

finscn commented May 12, 2017

Hi @mourner , Is there any news about this issue? thanks.

@mourner
Copy link
Member

mourner commented May 13, 2017

No updates on this yet unfortunately. If anyone has an idea on how to fix the issue without breaking existing test cases, I'll be glad to see a PR!

@finscn
Copy link

finscn commented Dec 3, 2017

I'm still waiting ... :'(

I wish it could be fixed some day

finscn added a commit to finscn/earcut that referenced this issue Dec 9, 2017
Only `water-huge2` failed .
the log says : 
```
not ok 18 4464 triangles when expected 4461
```

I think maybe  something is wrong in the test-case.
@finscn finscn mentioned this issue Dec 9, 2017
@finscn
Copy link

finscn commented Feb 9, 2019

Is there any news about this issue ?

@finscn
Copy link

finscn commented Mar 19, 2019

I think this is a serious problem , Is there any plan for fixing it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants