Picking broken for geometry types with back-face culling that have no volume

[hpinkos]

As of b23, picking no longer works for geometry types that have no volume (CircleGeometry, ExtentGeometry, PolygonGeometry...) when the appearance has the option 'closed : true'. If the geometry is extruded and has the same appearance, picking works.

Sandcastle Example: picking works for green and blue circle but not red circle.

require(['Cesium'], function(Cesium) {
    "use strict";

    var viewer = new Cesium.Viewer('cesiumContainer');
    var scene = viewer.scene;
    var primitives = scene.getPrimitives();
    var ellipsoid = viewer.centralBody.getEllipsoid();

    // Red circle
    var redCircleInstance = new Cesium.GeometryInstance({
        geometry : new Cesium.CircleGeometry({
            center : ellipsoid.cartographicToCartesian(Cesium.Cartographic.fromDegrees(-95.0, 43.0)),
            radius : 250000.0,
            vertexFormat : Cesium.PerInstanceColorAppearance.VERTEX_FORMAT
        }),
        id : 'flat circle closed',
        attributes : {
            color : Cesium.ColorGeometryInstanceAttribute.fromColor(new Cesium.Color(1.0, 0.0, 0.0, 0.5))
        }
    });

    // Green extruded circle
    var greenCircleInstance = new Cesium.GeometryInstance({
        geometry : new Cesium.CircleGeometry({
            center : ellipsoid.cartographicToCartesian(Cesium.Cartographic.fromDegrees(-87.0, 45.0)),
            radius : 250000.0,
            extrudedHeight: 300000.0,
            vertexFormat : Cesium.PerInstanceColorAppearance.VERTEX_FORMAT
        }),
        id : 'extruded circle',
        attributes : {
            color : Cesium.ColorGeometryInstanceAttribute.fromColor(new Cesium.Color(0.0, 1.0, 0.0, 0.5))
        }
    });

   // Blue circle
    var blueCircleInstance = new Cesium.GeometryInstance({
        geometry : new Cesium.CircleGeometry({
            center : ellipsoid.cartographicToCartesian(Cesium.Cartographic.fromDegrees(-90.0, 40.0)),
            radius : 250000.0,
            vertexFormat : Cesium.PerInstanceColorAppearance.VERTEX_FORMAT
        }),
        id : 'flat circle not closed',
        attributes : {
            color : Cesium.ColorGeometryInstanceAttribute.fromColor(new Cesium.Color(0.0, 0.0, 1.0, 0.5))
        }
    });

    // Add instances to primitives 
    primitives.add(new Cesium.Primitive({
        geometryInstances: [redCircleInstance, greenCircleInstance],
        appearance: new Cesium.PerInstanceColorAppearance({
            closed: true
        })
    }));

    primitives.add(new Cesium.Primitive({
        geometryInstances: blueCircleInstance,
        appearance: new Cesium.PerInstanceColorAppearance()
    }));

    var handler = new Cesium.ScreenSpaceEventHandler(scene.getCanvas());
    handler.setInputAction(
        function (movement) {
            var pickedObject = scene.pick(movement.position);
            if (Cesium.defined(pickedObject)) {
                console.log(pickedObject.id);
            } else {
                console.log('undefined');
            }
        }, Cesium.ScreenSpaceEventType.LEFT_CLICK);


    Sandcastle.finishedLoading();
});

[pjcozzi]

@bagnell no rush here but let's look at this at some point since I wouldn't expect these to get culled in the pick pass but not the color pass.

[mramato]

This is going to be a bigger issue when dynamicScene-refactor comes in, since CZML polygons and other geometry will be rendered with Primitive.

[pjcozzi]

@mramato as a workaround, use closed : false, which is really the right setting anyway.

[mramato]

The problem is I'm batching both flat polygons and extruded polygons into the same primitive. Won't closed : true make the extruded polygons render incorrectly? If so, then I would have to further sub-divide my batches based on the whether or not they are extruded. If that's really the right answer, then I suppose I can do it, it will just require another layer of bookeeping on my part.

[pjcozzi]

Yes, I would sort by these; non-extruded polygons are not closed, and if they are drawn at altitude, they will disappear when looking at them from the ground so we will have to sort anyway.

[mramato]

Well that sucks. I'll add it to my to-do.

[mramato]

Would you be against adding a closed property to the Geometry interface? The geometry knows whether it's closed or not when it's computed (right?) so it could just set the value and have it be a read-only property. Then higher level code can use that value for batching. I can open a PR to do this separately from my DynamicScene work if you're okay with it.

[pjcozzi]

That is OK with me.