GeoJSON support

[mramato]

1. Add GeoJsonDataSource which as the name implies is a DataSource object which supports GeoJSON. The only aspect of the specification that we do not currently support is holes in polygons. This is a limitation of the DynamicScene layer which must be corrected first (I'll be writing an issue for it).
2. Hook it up to viewerDragDropMixin and the Cesium Viewer source query parameter.
3. Add a ConstantProperty which makes it easy to specify static values for DynamicScene objects.
4. Since GeoJSON does not include styling information, I provide user-modifiable default styling. People with an eye for such things may want to check it out and offer suggestions.

[mramato]

@shunter, you know you are dying to review this. 😉

@emackey and @hpinkos, you have the best eyes for style around here. Any comments on the default styling?

[emackey]

Does this work even if suffix is longer than str?

[mramato]

Yes. The value of the second argument can be positive or negative and if suffix is longer it's impossible for this function to equal anything other than -1. However, your question brings up a good point, I don't need str.length - suffix.length because simply -suffix.length will work exactly the same.

[emackey]

Since you're asking for bike shed color recommendations, allow me to recommend Color.CYAN instead of the primary Blue you're using (along with rgba : [0, 255, 255, 25.5] for polygons). Generally, cyan survives poor-quality video links better than dark blue, and I find it easier on the eyes.

[mramato]

It's not bike shedding for things that actually matter. I'll try out CYAN. What about the opacity of the polygons? Does that look good to you (too opaque or too translucent?)

[emackey]

The opacity looks good. I don't think the casual observer will find a difference between 25 and 25.5, if you want to trim two bytes 😉

[mramato]

Okay, those changes are in. Are we happy with just using a Point primitive for points? Should we use a fancy placemark-like billboard instead?

[shunter]

indexOf doesn't take a negative argument. From the ES5 spec, start is min(max(pos, 0), len), which means that passing -suffix.length is equivalent to passing 0.

[mramato]

MDN lied to me, which is where I was looking. But you are correct. I double-checked the official spec PDF just to be sure. I'll undo my previous changes.

[emackey]

The default point is fine, people can configure it. Taking a second look at the OSM globe, it's very cyan already. Let's switch to Color.YELLOW (255, 255, 0). Sorry for changing my mind after you coded it.

[mramato]

@emackey I made it yellow, and I agree it's the most clear. That being said, keep in mind that anyone using Cesium Viewer will get these default values. I think we need a set of good looking defaults for Cesium Viewer (and I don't know why these wouldn't match the general defaults). That's why I think a custom placemark and easy on the eyes coloring is important. Yellow is visible, but not very pretty.

[emackey]

Yeah, the goal of a default color is just to be visible-by-default in as many different situations as possible. You can't really change your defaults based on which imagery layer is selected etc, but you can make educated guesses that the OSM layer might be popular with GeoJSON users.

A custom placemark can be nice. The upside-down raindrop shape allows more of the target area to be seen. Would we use a raster image for this, or just code a shader?

[mramato]

We would just base encode a png into the source, just like we do with the Bing logo. If we go with the standard placemark change, I would also change the bitmap origin so that the botton of the placemark is the actual position in the file.

[emackey]

Sure, non-scalable raster images can be useful. You could also use a GLSL shader.

    float len = length(st + vec2(0.0, -2.0));
    float circle = smoothstep(0.80, 0.85, len);
    float curve = (1.0 - cos(st.t * PI * 0.5)) * 0.5 * 0.80;
    float lower = 1.0 - (1.0 - smoothstep(curve, curve + 0.05, abs(st.s))) * (1.0 - smoothstep(0.95, 1.0, abs(st.t - 1.0)));
    float inscription = smoothstep(0.30, 0.35, len);
    float val = (1.0 - circle * lower) * inscription;

    gl_FragColor = color * val;

[mramato]

Except we have no way to express such things in the DynamicScene layer and we'd also be burdening the video card with shading all of those things that could just easily be a single static texture.

On a related topic, I was also thinking we could incorporate the Cesium logo into the placemark somehow, like instead of a black hole in the center, have the logo.

[shunter]

Arguably these are RuntimeErrors, not DeveloperErrors, since they're caused by data loaded at runtime.

[mramato]

That makes sense, I almost never use RuntimeError so it didn't even occur to me. I suppose you could argue that it's a DeveloperError when using load and a RuntimeError when using loadUrl, but then we are really splitting hairs. I'll change them to RuntimeError.

[shunter]

It's not clear to me what you're trying to do here. This looks like a no-op to me:

• adapt crsFunction to make it a promise if it's not already one
• when that promise resolves, make a new promise that is already resolved to the crsFunction value.

[mramato]

Okay, all review changes so far are in. Anything else?

[emackey]

Good with me 👍 @shunter please merge when ready.

[shunter]

Why check this.hasOwnProperty(property) here? Shouldn't you merge other[property] into this regardless?

[mramato]

Probably. I simplified it to:

for ( var property in other) {
    if (other.hasOwnProperty(property)) {
        this[property] = defaultValue(this[property], other[property]);
    }
}

[shunter]

Probably worth being consistent between geoJson and geojson throughout.

[shunter]

Worth factoring out this function to convert an array of coordinates to an array of positions?

[mramato]

Okay, I think we're close. Anything else?

[shunter]

Looks good, merging.