Polygon layer fix (visgl#424)

1) Update the data file and default values for the GeoJson layer sample 2) Update the accessor names of GeoJson layer and Polygon layer 3) Change the behavior of "extruded", "wireframe" and "stroked" props. Now "wireframe" only affects the extruded polygon and "stroked" only affects the non-extruded polygon 5) Add calls to super.updateState() to all layers so that data change leads to attribute update 6) Added a polygonWireframeLayer to GeoJSON layer and Polygon layer 7) Fixed other bugs mentioned by the review 8) Move uniform settings to the draw function from the updateState function in SolidPolygon layer
aravindiiitb · Mar 14, 2017 · 08b8d05 · 08b8d05
1 parent e8e4cf5
commit 08b8d05
Show file tree

Hide file tree

Showing 15 changed files with 133 additions and 72 deletions.
diff --git a/examples/layer-browser/data/sample.geo.json b/examples/layer-browser/data/sample.geo.json
@@ -4,7 +4,7 @@
     "type": "Feature",
     "properties": {
       "marker-color": "#ff0000",
-      "marker-size": "small",
+      "marker-size": "medium",
       "marker-symbol": ""
     },
     "geometry": {
@@ -24,7 +24,9 @@
     }
   }, {
     "type": "Feature",
-    "properties": {},
+    "properties": {
+      "marker-size": "small"
+    },
     "geometry": {
       "type": "MultiPoint",
       "coordinates": [

diff --git a/examples/layer-browser/src/examples/core-layers.js b/examples/layer-browser/src/examples/core-layers.js
@@ -24,9 +24,9 @@ import dataSamples from '../immutable-data-samples';
 import {parseColor, setOpacity} from '../utils/color';
 
 const MARKER_SIZE_MAP = {
-  small: 2,
-  medium: 5,
-  large: 10
+  small: 200,
+  medium: 500,
+  large: 1000
 };
 
 const LIGHT_SETTINGS = {
@@ -84,7 +84,7 @@ const GeoJsonLayerExample = {
       return setOpacity(color, opacity);
     },
     getWidth: f => f.properties['stroke-width'],
-    getHeight: f => Math.random() * 1000,
+    getElevation: f => 500,
     widthScale: 10,
     widthMinPixels: 1,
     pickable: true
@@ -96,7 +96,7 @@ const GeoJsonLayerExtrudedExample = {
   getData: () => dataSamples.choropleths,
   props: {
     id: 'geojsonLayer-extruded',
-    getHeight: f => get(f, 'properties.ZIP_CODE') * 10 % 127 * 10,
+    getElevation: f => get(f, 'properties.ZIP_CODE') * 10 % 127 * 10,
     getFillColor: f => [0, 255, get(f, 'properties.ZIP_CODE') * 23 % 100 + 155],
     getColor: f => [200, 0, 80],
     drawPolygons: true,

diff --git a/src/layers/core/arc-layer/arc-layer.js b/src/layers/core/arc-layer/arc-layer.js
@@ -89,7 +89,12 @@ export default class ArcLayer extends Layer {
   }
 
   updateState({props, oldProps, changeFlags}) {
-    this.updateModel({props, oldProps, changeFlags});
+    super.updateState({props, oldProps, changeFlags});
+    // Re-generate model if geometry changed
+    if (props.fp64 !== oldProps.fp64) {
+      const {gl} = this.context;
+      this.setState({model: this._getModel(gl)});
+    }
     this.updateAttribute({props, oldProps, changeFlags});
   }
 

diff --git a/src/layers/core/geojson-layer/geojson-layer.js b/src/layers/core/geojson-layer/geojson-layer.js
@@ -86,49 +86,54 @@ export default class GeoJsonLayer extends CompositeLayer {
     let {} = this.props;
     const drawPoints = pointFeatures && pointFeatures.length > 0;
     const drawLines = lineFeatures && lineFeatures.length > 0;
-    const drawPolygons = stroked && polygonOutlineFeatures && polygonOutlineFeatures.length > 0;
-    const fillPolygons = filled && polygonFeatures && polygonFeatures.length > 0;
+    const hasPolygonOutline = polygonOutlineFeatures && polygonOutlineFeatures.length > 0;
+    const hasPolygon = polygonFeatures && polygonFeatures.length > 0;
 
     const onHover = this._onHoverSubLayer.bind(this);
     const onClick = this._onClickSubLayer.bind(this);
 
     // Filled Polygon Layer
-    const polygonFillLayer = fillPolygons && new SolidPolygonLayer(Object.assign({}, this.props, {
-      id: `${id}-polygon-fill`,
-      data: polygonFeatures,
-      extruded,
-      wireframe: false,
-      getPolygon: getCoordinates,
-      getElevation,
-      getColor: getFillColor,
-      updateTriggers: {
-        getElevation: updateTriggers.getElevation,
-        getColor: updateTriggers.getFillColor
-      },
-      onHover,
-      onClick
-    }));
+    const polygonFillLayer = filled &&
+      hasPolygon &&
+      new SolidPolygonLayer(Object.assign({}, this.props, {
+        id: `${id}-polygon-fill`,
+        data: polygonFeatures,
+        extruded,
+        wireframe: false,
+        getPolygon: getCoordinates,
+        getElevation,
+        getColor: getFillColor,
+        updateTriggers: {
+          getElevation: updateTriggers.getElevation,
+          getColor: updateTriggers.getFillColor
+        },
+        onHover,
+        onClick
+      }));
 
-    // Polygon outline or wireframe
-    let polygonOutlineLayer = null;
-    if (drawPolygons && extruded && wireframe) {
-      polygonOutlineLayer = new SolidPolygonLayer(Object.assign({}, this.props, {
+    const polygonWireframeLayer = wireframe &&
+      extruded &&
+      hasPolygon &&
+      new SolidPolygonLayer(Object.assign({}, this.props, {
         id: `${id}-polygon-wireframe`,
         data: polygonFeatures,
-        extruded: true,
+        extruded,
         wireframe: true,
         getPolygon: getCoordinates,
         getElevation,
         getColor,
         updateTriggers: {
           getElevation: updateTriggers.getElevation,
-          getColor: updateTriggers.getColor
+          getColor: updateTriggers.getFillColor
         },
         onHover,
         onClick
       }));
-    } else if (drawPolygons) {
-      polygonOutlineLayer = new PathLayer(Object.assign({}, this.props, {
+
+    const polygonOutlineLayer = !extruded &&
+      stroked &&
+      hasPolygonOutline &&
+      new PathLayer(Object.assign({}, this.props, {
         id: `${id}-polygon-outline`,
         data: polygonOutlineFeatures,
         getPath: getCoordinates,
@@ -141,7 +146,6 @@ export default class GeoJsonLayer extends CompositeLayer {
         onHover,
         onClick
       }));
-    }
 
     const lineLayer = drawLines && new PathLayer(Object.assign({}, this.props, {
       id: `${id}-line-paths`,
@@ -173,6 +177,7 @@ export default class GeoJsonLayer extends CompositeLayer {
 
     return [
       polygonFillLayer,
+      polygonWireframeLayer,
       polygonOutlineLayer,
       lineLayer,
       pointLayer

diff --git a/src/layers/core/grid-cell-layer/grid-cell-layer.js b/src/layers/core/grid-cell-layer/grid-cell-layer.js
@@ -116,7 +116,11 @@ export default class GridCellLayer extends Layer {
 
   updateState({props, oldProps, changeFlags}) {
     super.updateState({props, oldProps, changeFlags});
-    this.updateModel({props, oldProps, changeFlags});
+    // Re-generate model if geometry changed
+    if (props.fp64 !== oldProps.fp64) {
+      const {gl} = this.context;
+      this.setState({model: this._getModel(gl)});
+    }
     this.updateAttribute({props, oldProps, changeFlags});
     this.updateUniforms();
   }

diff --git a/src/layers/core/hexagon-cell-layer/hexagon-cell-layer.js b/src/layers/core/hexagon-cell-layer/hexagon-cell-layer.js
@@ -143,7 +143,10 @@ export default class HexagonCellLayer extends Layer {
 
   updateState({props, oldProps, changeFlags}) {
     super.updateState({props, oldProps, changeFlags});
-    this.updateModel({props, oldProps, changeFlags});
+    if (props.fp64 !== oldProps.fp64) {
+      const {gl} = this.context;
+      this.setState({model: this._getModel(gl)});
+    }
     this.updateAttribute({props, oldProps, changeFlags});
 
     const viewportChanged = changeFlags.viewportChanged;

diff --git a/src/layers/core/icon-layer/icon-layer.js b/src/layers/core/icon-layer/icon-layer.js
@@ -103,6 +103,8 @@ export default class IconLayer extends Layer {
   }
 
   updateState({oldProps, props, changeFlags}) {
+    super.updateState({props, oldProps, changeFlags});
+
     const {iconAtlas} = props;
 
     if (oldProps.iconAtlas !== iconAtlas) {
@@ -121,7 +123,10 @@ export default class IconLayer extends Layer {
       }
     }
 
-    this.updateModel({props, oldProps, changeFlags});
+    if (props.fp64 !== oldProps.fp64) {
+      const {gl} = this.context;
+      this.setState({model: this._getModel(gl)});
+    }
     this.updateAttribute({props, oldProps, changeFlags});
 
   }

diff --git a/src/layers/core/line-layer/line-layer.js b/src/layers/core/line-layer/line-layer.js
@@ -88,7 +88,12 @@ export default class LineLayer extends Layer {
   }
 
   updateState({props, oldProps, changeFlags}) {
-    this.updateModel({props, oldProps, changeFlags});
+    super.updateState({props, oldProps, changeFlags});
+
+    if (props.fp64 !== oldProps.fp64) {
+      const {gl} = this.context;
+      this.setState({model: this._getModel(gl)});
+    }
     this.updateAttribute({props, oldProps, changeFlags});
   }
 

diff --git a/src/layers/core/path-layer/path-layer.js b/src/layers/core/path-layer/path-layer.js
@@ -81,9 +81,14 @@ export default class PathLayer extends Layer {
   }
 
   updateState({oldProps, props, changeFlags}) {
+    super.updateState({props, oldProps, changeFlags});
+
     const {getPath} = this.props;
     const {attributeManager} = this.state;
-    this.updateModel({props, oldProps, changeFlags});
+    if (props.fp64 !== oldProps.fp64) {
+      const {gl} = this.context;
+      this.setState({model: this._getModel(gl)});
+    }
     this.updateAttribute({props, oldProps, changeFlags});
 
     if (changeFlags.dataChanged) {
@@ -94,7 +99,6 @@ export default class PathLayer extends Layer {
       this.setState({paths, numInstances});
       attributeManager.invalidateAll();
     }
-
   }
 
   draw({uniforms}) {

diff --git a/src/layers/core/point-cloud-layer/point-cloud-layer.js b/src/layers/core/point-cloud-layer/point-cloud-layer.js
@@ -95,7 +95,11 @@ export default class PointCloudLayer extends Layer {
   }
 
   updateState({props, oldProps, changeFlags}) {
-    this.updateModel({props, oldProps, changeFlags});
+    super.updateState({props, oldProps, changeFlags});
+    if (props.fp64 !== oldProps.fp64) {
+      const {gl} = this.context;
+      this.setState({model: this._getModel(gl)});
+    }
     this.updateAttribute({props, oldProps, changeFlags});
   }
 

diff --git a/src/layers/core/polygon-layer/polygon-layer.js b/src/layers/core/polygon-layer/polygon-layer.js
@@ -83,28 +83,45 @@ export default class PolygonLayer extends CompositeLayer {
     const {data, id, stroked, filled, extruded, wireframe} = this.props;
     const {paths, onHover, onClick} = this.state;
 
-    const strokePolygons = stroked && data && data.length > 0;
-    const fillPolygons = filled && data && data.length > 0;
+    const hasData = data && data.length > 0;
 
     // Filled Polygon Layer
-    const polygonFillLayer = fillPolygons && new SolidPolygonLayer(Object.assign({},
+    const polygonLayer = filled && hasData && new SolidPolygonLayer(Object.assign({},
       this.props, {
         id: `${id}-fill`,
         data,
         getElevation,
         getColor: getFillColor,
         extruded,
-        wireframe,
+        wireframe: false,
         updateTriggers: {
           getElevation: updateTriggers.getElevation,
           getColor: updateTriggers.getFillColor
         }
       }));
 
+    const polygonWireframeLayer = extruded &&
+      wireframe &&
+      hasData &&
+      new SolidPolygonLayer(Object.assign({},
+      this.props, {
+        id: `${id}-wireframe`,
+        data,
+        getElevation,
+        getColor,
+        extruded: true,
+        wireframe: true,
+        updateTriggers: {
+          getElevation: updateTriggers.getElevation,
+          getColor: updateTriggers.getColor
+        }
+      }));
+
     // Polygon outline layer
-    let polygonOutlineLayer = null;
-    if (strokePolygons) {
-      polygonOutlineLayer = new PathLayer(Object.assign({}, this.props, {
+    const polygonOutlineLayer = !extruded &&
+      stroked &&
+      hasData &&
+      new PathLayer(Object.assign({}, this.props, {
         id: `${id}-stroke`,
         data: paths,
         getPath: x => x.path,
@@ -117,10 +134,10 @@ export default class PolygonLayer extends CompositeLayer {
           getColor: updateTriggers.getColor
         }
       }));
-    }
 
     return [
-      polygonFillLayer,
+      polygonLayer,
+      polygonWireframeLayer,
       polygonOutlineLayer
     ].filter(Boolean);
   }

diff --git a/src/layers/core/scatterplot-layer/scatterplot-layer.js b/src/layers/core/scatterplot-layer/scatterplot-layer.js
@@ -95,7 +95,10 @@ export default class ScatterplotLayer extends Layer {
   }
 
   updateState({props, oldProps, changeFlags}) {
-    this.updateModel({props, oldProps, changeFlags});
+    if (props.fp64 !== oldProps.fp64) {
+      const {gl} = this.context;
+      this.setState({model: this._getModel(gl)});
+    }
     this.updateAttribute({props, oldProps, changeFlags});
   }
 

diff --git a/src/layers/core/screen-grid-layer/screen-grid-layer.js b/src/layers/core/screen-grid-layer/screen-grid-layer.js
@@ -63,6 +63,7 @@ export default class ScreenGridLayer extends Layer {
   }
 
   updateState({oldProps, props, changeFlags}) {
+    super.updateState({props, oldProps, changeFlags});
     const cellSizeChanged =
       props.cellSizePixels !== oldProps.cellSizePixels;