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

Fix ellipse triangulation #3326

Merged
merged 8 commits into from
Dec 17, 2015
Merged

Fix ellipse triangulation #3326

merged 8 commits into from
Dec 17, 2015

Conversation

hpinkos
Copy link
Contributor

@hpinkos hpinkos commented Dec 10, 2015

Fixes #3078

This also fixes a crash if you tried to create an ellipse with VertexFormat.ALL. The tangents and binormals weren't normalized.

Before:
image

After:
image

@hpinkos
Copy link
Contributor Author

hpinkos commented Dec 10, 2015

normals/tangents/binormals and texture coordiantes should be correct:

image

image

@bagnell
Copy link
Contributor

bagnell commented Dec 10, 2015

Try this example code that doesn't generate that many triangles:

var viewer = new Cesium.Viewer('cesiumContainer');
var scene = viewer.scene;

scene.primitives.add(new Cesium.Primitive({
    geometryInstances : new Cesium.GeometryInstance({
        geometry : Cesium.GeometryPipeline.toWireframe(Cesium.EllipseGeometry.createGeometry(new Cesium.EllipseGeometry({
            center : Cesium.Cartesian3.fromDegrees(-100.0, 40.0),
            semiMinorAxis : 300000.0,
            semiMajorAxis : 400000.0,
            vertexFormat : Cesium.PerInstanceColorAppearance.VERTEX_FORMAT,
            granularity : Cesium.Math.PI / 32
        }))),
        attributes: {
            color: Cesium.ColorGeometryInstanceAttribute.fromColor(Cesium.Color.RED)
        }
    }),
    appearance : new Cesium.PerInstanceColorAppearance({
        translucent : false,
        closed : true
    }),
    asynchronous : false
}));

image

It looks like there are 22 triangles and 16 vertices. If you put a breakpoint in EllipseGeometry.createGeometry, there are 24 triangles and 18 vertices in the mesh.

@hpinkos
Copy link
Contributor Author

hpinkos commented Dec 10, 2015

@bagnell that's because there are duplicate positions at the points on the left and right end. It was that way before I changed the code. I'll see if I can fix it though.

@hpinkos
Copy link
Contributor Author

hpinkos commented Dec 11, 2015

@bagnell ready

@pjcozzi
Copy link
Contributor

pjcozzi commented Dec 15, 2015

@bagnell any update here?

@bagnell
Copy link
Contributor

bagnell commented Dec 17, 2015

Looks good. Thanks @hpinkos!

bagnell added a commit that referenced this pull request Dec 17, 2015
@bagnell bagnell merged commit 85264ad into master Dec 17, 2015
@bagnell bagnell deleted the fixEllipse branch December 17, 2015 17:24
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

Successfully merging this pull request may close these issues.

EllipseGeometryLibrary's computeEllipsePositions misses critical points
3 participants