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

Twin points in shape ? #12165

Closed
jeum opened this issue Sep 10, 2017 · 9 comments
Closed

Twin points in shape ? #12165

jeum opened this issue Sep 10, 2017 · 9 comments

Comments

@jeum
Copy link

jeum commented Sep 10, 2017

When I create a shape with a sequence (shape.moveTo ..) and this sequence contains a shape.absarc or shape.arc (haven't tried splines) a twin point is added before the arc into the Shape definition ?

In this sample the twin point is # 2 (into the console).

I don't know if it's a bug since it doesn't seems to alter following process (such as ShapeGeometry), but shape.extractPoints() retrieves these twin points ... and if I reuse the result to build something with THREE.ShapeUtils.triangulateShape() I will get the warning 'Unable to triangulate..' .. that's why I have to add this ugly patch :

	var loops = shape.extractPoints ( settings.curveResol );
	for (var l = 1 ; l < loops.shape.length ; l++) {
		if( Math.abs( loops.shape[l].x - loops.shape[l-1].x ) < 1E-10 && Math.abs( loops.shape[l].y - loops.shape[l-1].y ) < 1E-10 ) loops.shape.splice( l , 1 )
	}
@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 11, 2017

I think the problem is created by these two lines in Path.absellipse().

this.currentPoint is the last point used with .lineTo(). firstPoint is calculated via a sampling process of the given EllipseCurve. The problem is that these two points are equal
in the fewest cases because of floating point inaccuracies.

@jeum How do you define the points of your shape? Maybe it works if you use more decimal places?

Apart from this, instead of using .equals() it would make more sense to use something like .epsilonEquals() in .absellipse().

@mrdoob @WestLangley How about implementing such a method for all vector classes?

@jeum
Copy link
Author

jeum commented Sep 11, 2017

@Mugen87 : I can't do that, it's within a parser.

About a new method, indeed, this test is not unusual when you build things ...

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 30, 2017

Just a short update: With the new polygon triangulation algorithm (see #12661) the mentioned warning no longer appears. Will be available with R89. I'm still trying to figure out if it's possible to avoid the duplicated points.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 1, 2017

After some tests i think it's better to keep Path.absellipse() as it is. The current logic automatically ensures that a path is a sequence of consecutive curves without gaps. From my point of view that assumption should not be changed. Because of the mentioned floating point inaccuracies, a path might contain a curve (e.g. a line) that is actually not necessary. But the new and more robust polygon triangulation algorithm can handle this situation without problems.

Also #12379 and #12381 showed that it is not trivial to introduce a consistent epsilon value to handle floating point inaccuracies. For now, i don't think we have to go this way. We should stay with the current implementation (and its little limitation).

If there are no objections we can close this issue for now.

@jeum
Copy link
Author

jeum commented Dec 1, 2017

Ok thanks ! I will test it asap..

I'm still trying to figure out if it's possible to avoid the duplicated points.

May be with an optional argument "skip end point test" set to true when the point list is "ended" ? But not sure it's usefull. In my case (parsing) I realize that I could also try to use less decimal places to pass the test ..

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 1, 2017

Is it actually a problem for your application if you have these additional points in the contour? As i said before, the triangulation should not bother anymore. Or is it more some kind of cosmetic flaw for you? 😉

@jeum
Copy link
Author

jeum commented Dec 1, 2017

No, I worked (come back soon to this project) on a parser that extrudes potentially a hudge number of shapes ... so 1 point more in the shape generates 4 faces (+ artifacts if I remeber)... But as I said there should be workarounds such as reduce accurrency or epsilon test.

I'll try the new triangulation asap .. will it support quads and polys ?

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 1, 2017

The output is the same as before, a triangulated polygon from a series of vertices and holes. The implementation is based on https://github.com/mapbox/earcut

@jeum
Copy link
Author

jeum commented Dec 1, 2017

Yes, wrong question (of course it does, that't why I used it...).

I'll try and report if twin points are still a problem but I suppose that I can avoid to make it. Thanks, I close.

@jeum jeum closed this as completed Dec 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants