Switched SurfacePolygon to Polygon in GeoJSONParser.js

[lowswaplab]

Description of the Change

Switch SurfacePolygon to Polygon so 3D polygons can be drawn on globe.

Why Should This Be In Core?

So 3D polygons can be drawn on globe.

Benefits

3D polygons can be drawn on globe.

Potential Drawbacks

Polygons locked to the surface may not work properly.

Applicable Issues

[ComBatVision]

Hi @lowswaplab,

What about to have following logic: If at least one source point has altitude, then Polygon is created, otherwise SurfacePolygon?

What potential issues do you see in this case?

Or nay be we can control this behavior by altitudeMode?

[lowswaplab]

Setting altitudeMode to CLAMP_TO_GROUND, ABSOLUTE, etc works as expected.

[ComBatVision]

Could you, please, show some screen shots how it looks like in both variants ABSOLUYE and CLAMP_TO_GROUND and tell me when PR be ready to merge from your point of view.

I think it is better to use Surface Polygon for ClampToGround, because tracing terrain in vector Polygon is more resource ineffective.

[lowswaplab]

Where can I upload pictures? I think the PR is ready to merge.

[ComBatVision]

There is a button upload image in comments.

What about SurfacePolygon? I see only Polygon in the code. What is your thoughts about using SurfacePolygon in clamp to ground mode as it was before your enhancement?

[lowswaplab]

I updated the code to check for presence of altitude. If no altitudes are present, it uses a SurfacePolygon. If an altitude is present, it uses a Polygon.

[lowswaplab]

[ComBatVision]

One last question. Should we keep altitude presence check or may be we can use altitudeMode = PlainToGround as indicator for surface polygon usage?

[lowswaplab]

How about this?

If altitudeMode is CLAMP_TO_GROUND, then use SurfacePolygon.

Else, then check altitude. If altitude: Polygon, if no altitude: SurfacePolygon

[ComBatVision]

I thought about it little bit more... Do we have situation when we need force use of vector Polygon and trace terrain specially for it instead of SurfacePolygon?

If we remain only altitudeMode check, then it will be more flexible and allow us to create vector polygon which is following the ground.

How do you think, do we need such flexibility?

Difference between SurfacePolygon and Polygon with zero altitude is that first one draws as raster line on texture canvas and automatically follow terrain with texture. Second one has special additional logic to trace altitude map for each polygon. It may draw lines through texture peaks and it will looks like a dashed line.

[ComBatVision]

I found another corner case! It will be not possible to draw Polygon at zero altitude in case of surface is below sea level.

So I propose to use only clamp_to_ground as indicator. It will definetely determine that user wants to follow surface. Zero altitude do not means surface level in absolute mode.

[lowswaplab]

I switched it to check only CLAMP_TO_GROUND and ran some tests and it worked.

[ComBatVision]

I have changed code little bit: 1) SurfacePolygon has no altitudeMode attribute, so it is not necessary to set it. 2) I have used Position in both cases and simplified logic with Locations.

                    if (configuration && configuration.altitudeMode == WorldWind.CLAMP_TO_GROUND) {
                      shape = new SurfacePolygon(
                        pBoundaries,
                        configuration && configuration.attributes ? configuration.attributes : null);
                    } else {
                      shape = new Polygon(
                        pBoundaries,
                        configuration && configuration.attributes ? configuration.attributes : null);
                      if (configuration && configuration.altitudeMode) {
                        shape.altitudeMode = configuration.altitudeMode;
                      } else {
                        shape.altitudeMode = WorldWind.ABSOLUTE;
                      }
                    }

@lowswaplab Could you please test this feature in develop branch.

[lowswaplab]

Looks good!

I wasn't sure about the Location vs Position thing...

Next I want to do the same thing to SurfacePolyline and Path.

[ComBatVision]

Position is child of Location, but to be honest it is little bit memory ineffective solution, but code is simpler.

Ok. Create new PR.