Improve CoplanarPolygonGeometry normal calculation

[hpinkos]

Fixes #7185

Flips the normal of the polygon if it is pointing roughly towards the center of the earth instead of away from it. (Also flip the tangent so it points towards the right instead of the left). This then fixes the texture coordinate calculation so images don't appear mirrored in some cases.

var positions = new Cesium.PolygonHierarchy(Cesium.Cartesian3.fromDegreesArrayHeights([
            110,30,0, 
            110,40,0, 
            120,40,0, 
            120,30,0]));

var primitive = scene.primitives.add(new Cesium.Primitive({
    geometryInstances : [new Cesium.GeometryInstance({
        geometry : new Cesium.CoplanarPolygonGeometry({
            polygonHierarchy : positions,
            vertexFormat : Cesium.MaterialAppearance.MaterialSupport.TEXTURED.vertexFormat
        })
    })],
    appearance : new Cesium.MaterialAppearance({
        material : Cesium.Material.fromType('Image', {
            image: '../images/Cesium_Logo_Color.jpg'
        }),
        closed : false,
        translucent : false
    })
}));

[cesium-concierge]

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.

🌍 🌎 🌏

[hpinkos]

@OmarShehata can you review?

[OmarShehata]

If using winding order to compute normal is going to cause problems, I think this could work. Can we add a flag that says something like flipNormal ? Then the doc can say that the polygon will always face away from the ellipsoid, unless you set that.

That way it resolves the use case Scott brought up here #7185 (comment) , and still gives the user explicit control over this without either them or us having to worry about winding order.

What do you think @hpinkos ?

I definitely think it makes more sense for the polygon to have a consistent direction (facing away from the ellipsoid) as opposed to the behavior in master where it depends on where it's defined.

Something I just thought of: Does it still work if the globe is turned off the polygon is around the center of the earth?

[mramato]

I don't think a flipNormal makes sense here, since we don't do it for other geometry. Cesium can certainly do better in this area, but I think staying consistent with other geometry is the best short-term approach and we can revisit if it becomes a problem.

[OmarShehata]

@mramato isn't this the only flat geometry that won't always be on the surface? That's why I thought this would make sense. If not, then I agree doing this as a separate issue for all relevant geometry types is better.

[hpinkos]

@OmarShehata yeah, we can consider adding something like that in the future if people ask for it. It's just not really necessary for this PR.

[hpinkos]

This is ready for review

[mramato]

@mramato isn't this the only flat geometry that won't always be on the surface?

No, rectangle, polygon, ellipse, etc... all have height properties and can be in the air.

@OmarShehata please bump for @bagnell to give a final lookover and merge once you're happy.

[OmarShehata]

Code looks good! It resolves the original issue as well as the issue here https://groups.google.com/d/msg/cesium-dev/aTbKq5Kn8Tc/R3RDMvd_BwAJ.

For CHANGES.md:

Fixed texture coordinate calculation for polygon entities with perPositionHeight #7187

This links to the wrong PR. Shouldn't it be more like "Fixed inconsistent normal calculation for polygon entities with perPositionHeight" ?

[hpinkos]

@OmarShehata the normal was the underlying problem, but the incorrect texture was the bug that someone saw and reported.

[OmarShehata]

I'm fine with that description, but just wanted to clarify that the link still goes to a different PR than this one.

[hpinkos]

Thanks @OmarShehata!

@bagnell please review and merge

[cesium-concierge]

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/d/msg/cesium-dev/aTbKq5Kn8Tc/R3RDMvd_BwAJ

If this issue affects any of these threads, please post a comment like the following:

The issue at #7188 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.

---

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

🌍 🌎 🌏