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

Ellipse Hole in a Shape can wreck the Shape. #12921

Closed
3 of 14 tasks
MagnuzBinder opened this issue Dec 20, 2017 · 5 comments
Closed
3 of 14 tasks

Ellipse Hole in a Shape can wreck the Shape. #12921

MagnuzBinder opened this issue Dec 20, 2017 · 5 comments

Comments

@MagnuzBinder
Copy link
Contributor

MagnuzBinder commented Dec 20, 2017

Description of the problem

If a Hole in the form of one or more absarc/absellipse/arc/ellipse segments forming a full circle or ellipse is added to a 2D Shape and rendered as a ShapeBufferGeometry, this can wreck the Shape, anything from removing two triangles between one vertice in the outer Path and the three vertices around an absarc/absellipse/arc/ellipse start/end, to not rendering the Shape at all, depending on the Shape outer Path geometry. It gives the warning "THREE.ShapeUtils: Unable to triangulate polygon! in triangulate()". This does not happen if the Hole is a single closed bezierCurveTo or any other Curve segments, independent of what Curve segments make up the outer Path of the Shape. It does however give a warning for each Path start/end "THREE.ShapeUtils: Duplicate point".

All vertices are correctly added to the geometry position attribute, but the problem seems to be with vertex indices, which are added oddly, incomplete or not at all, depending on the Shape outer Path. The fact that the glitch always seems to emanate around an absarc/absellipse/arc/ellipse start or end indicates that it may be the wrapping there working incorrectly. The actual reason seems to be that the start and end points at the angles 0 and 2 * Math.PI can become unequal, because Math.sin( 0 ) === 0 but Math.sin( 2 * Math.PI ) === -2.4492935982947064e-16, so the "duplicate" end point is not removed following an equal comparison (ShapeUtils.js line 38-40). In some cases, this still works, because rounding errors compensate for the sin error.

There are several ways to solve this. The probably most correct one, to handle any kind of rounding errors, would be to replace the straightforward function

Vector3.equals = function ( v ) {
    return ( ( v.x === this.x ) && ( v.y === this.y ) && ( v.z === this.z ) );
}

with a more complex function similar to

Vector3.approximates = function ( v, re ) {
    // re = permitted relative error
    return (
        Math.abs( v.x - this.x ) / ( 1 + Math.abs( v.x ) + Math.abs( this.x ) ) <= re &&
        Math.abs( v.y - this.y ) / ( 1 + Math.abs( v.y ) + Math.abs( this.y ) ) <= re &&
        Math.abs( v.z - this.z ) / ( 1 + Math.abs( v.z ) + Math.abs( this.z ) ) <= re
    );
}

or a joined function, keeping (most of) the simplicity and speed in the simple case

Vector3.equals = function ( v, re ) {
    // re = permitted relative error, optional
    if ( v.x === this.x && v.y === this.y && v.z === this.z )
        return true;
    if ( re === undefined || re <= 0 )
        return false;
    return (
        Math.abs( v.x - this.x ) <= re * ( 1 + Math.abs( v.x ) + Math.abs( this.x ) ) &&
        Math.abs( v.y - this.y ) <= re * ( 1 + Math.abs( v.y ) + Math.abs( this.y ) ) &&
        Math.abs( v.z - this.z ) <= re * ( 1 + Math.abs( v.z ) + Math.abs( this.z ) )
    );
}

(edited function to handle origin position case and add joined function)

Three.js version
  • Dev
  • r89
  • ...
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
  • Safari
OS
  • All of them
  • Windows
  • macOS
  • Linux
  • Android
  • iOS
Hardware Requirements (graphics card, VR Device, ...)
@WestLangley
Copy link
Collaborator

A fiddle would be beneficial. See "How to report a bug" in the guidelines.

@MagnuzBinder
Copy link
Contributor Author

It seems this is fixed in r89. It didn't work at first, probably owing to the viewer using a cached r88. Mea culpa. Closing this.

@MagnuzBinder
Copy link
Contributor Author

MagnuzBinder commented Dec 20, 2017

I did try to set up a fiddle, but didn't get the dev version to load properly. Probably because it was the first time ever I tried to use jsfiddle and I need some more practice. I'll go back to fixing typos.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 20, 2017

R89 has a new polygon triangulation algorithm that is way more robust than the old one, see #12661. I LOVE this code 😊 ❤️

@mrdoob
Copy link
Owner

mrdoob commented Dec 20, 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

4 participants