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 rectangles at north and south poles #7452

Merged
merged 6 commits into from
Jan 4, 2019
Merged

Fix rectangles at north and south poles #7452

merged 6 commits into from
Jan 4, 2019

Conversation

hpinkos
Copy link
Contributor

@hpinkos hpinkos commented Jan 2, 2019

Fixes #7406
Fixes #4760

When the rectangle north is the north pole, it comes to a single point instead of a row of points at the top edge. RectangleGeometry was creating a ton of duplicate points and every other triangle at the north pole had zero area, which was causing things to go weird further up the geometry pipeline (specifically when splitLongitude was trying to use barycentric coordinates to compute attributes).

Similarly for the south pole and the bottom edge.

This change creates a single north pole position and/or south pole position and triangulates accordingly.

For an example, this rectangle crashes in master, but is fixed in this branch:

var viewer = new Cesium.Viewer('cesiumContainer');
viewer.scene.primitives.add(new Cesium.Primitive({
    geometryInstances : new Cesium.GeometryInstance({
        geometry : new Cesium.RectangleGeometry({
            rectangle : Cesium.Rectangle.fromDegrees(-180.0, -90.0, -179.0, 90.0),
            vertexFormat : Cesium.VertexFormat.POSITION_AND_NORMAL
        }),
        attributes: {
            color: Cesium.ColorGeometryInstanceAttribute.fromColor(new Cesium.Color(0.0, 1.0, 0.0, 0.5))
        }
    }),
    appearance : new Cesium.PerInstanceColorAppearance({
        closed : true
    })
}));

@cesium-concierge
Copy link

Thanks for the pull request @hpinkos!

  • ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

@mramato
Copy link
Contributor

mramato commented Jan 3, 2019

Thanks @hpinkos! Please merge in master.

@bagnell I think you're most qualified to review this.

@bagnell
Copy link
Contributor

bagnell commented Jan 4, 2019

This doesn't fix #1297. It doesn't crash, but the artifact is still there. Here is the updated code example:

var viewer = new Cesium.Viewer('cesiumContainer');
viewer.entities.add({
    rectangle : {
        coordinates : Cesium.Rectangle.fromDegrees(-180.0, 45.0, -160.0, 90.0),
        outline : true,
        outlineColor : Cesium.Color.WHITE,
        material : Cesium.Color.BLUE,
        height : 0.0
    }
});

The only difference between this code and that in the issue is height : 0. So for some reason a rectangle clamped to the ground is different from a normal rectangle geometry?

@hpinkos
Copy link
Contributor Author

hpinkos commented Jan 4, 2019

Gotcha. I remembered to set height: 0 when i was testing out some other things but I must have forgot to set it when I was testing that example

@hpinkos
Copy link
Contributor Author

hpinkos commented Jan 4, 2019

@bagnell merged in master. Anything else?

@bagnell
Copy link
Contributor

bagnell commented Jan 4, 2019

No, this looks good. Merging.

@bagnell bagnell merged commit 98410b4 into master Jan 4, 2019
@bagnell bagnell deleted the fix-rectangles branch January 4, 2019 20:55
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.

splitLongitude produces NaN texture coordinates RectangleGeometry fails at latitude -90
4 participants