This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 8177fa1  [bugfix] deck.gl grid&hex don't respond to zoom&drag (#6363)
8177fa1 is described below

commit 8177fa17fa9bdc78c1ff6866293f204642cce847
Author: Maxime Beauchemin <maximebeauche...@gmail.com>
AuthorDate: Fri Nov 16 17:12:19 2018 -0800

    [bugfix] deck.gl grid&hex don't respond to zoom&drag (#6363)
    
    * [bugfix] deck.gl grid&hex don't respond to zoom&drag
    
    Following recent changes, Hex & Grid component wouldn't respond to
    viewport interactions. This fixes the issue.
    
    This PR also greatly improve rendering performance by only recomputing
    the deck.gl layers when needed, as opposed to every time the viewport
    changes, which is very inneficient.
    
    Note that most other deck.gl layers seem to be systematically
    recomputing layers, we'll have to fix this in another PR.
    
    * Addressing comments
---
 superset/assets/package.json                       |  2 +-
 .../src/visualizations/deckgl/DeckGLContainer.jsx  | 12 ++-
 .../assets/src/visualizations/deckgl/factory.jsx   | 86 +++++++++++++++-------
 superset/assets/yarn.lock                          | 42 +++++------
 4 files changed, 89 insertions(+), 53 deletions(-)

diff --git a/superset/assets/package.json b/superset/assets/package.json
index a0aa9e8..9ee1ed4 100644
--- a/superset/assets/package.json
+++ b/superset/assets/package.json
@@ -79,7 +79,7 @@
     "d3-tip": "^0.9.1",
     "datamaps": "^0.5.8",
     "datatables.net-bs": "^1.10.15",
-    "deck.gl": "^5.3.4",
+    "deck.gl": "^5.3.5",
     "distributions": "^1.0.0",
     "dnd-core": "^2.6.0",
     "dompurify": "^1.0.3",
diff --git a/superset/assets/src/visualizations/deckgl/DeckGLContainer.jsx 
b/superset/assets/src/visualizations/deckgl/DeckGLContainer.jsx
index 199506e..9ad2f70 100644
--- a/superset/assets/src/visualizations/deckgl/DeckGLContainer.jsx
+++ b/superset/assets/src/visualizations/deckgl/DeckGLContainer.jsx
@@ -3,8 +3,9 @@ import PropTypes from 'prop-types';
 import MapGL from 'react-map-gl';
 import DeckGL from 'deck.gl';
 import 'mapbox-gl/dist/mapbox-gl.css';
+import { isEqual } from 'lodash';
 
-const TICK = 1000;  // milliseconds
+const TICK = 2000;  // milliseconds
 
 const propTypes = {
   viewport: PropTypes.object.isRequired,
@@ -23,12 +24,13 @@ const defaultProps = {
 export default class DeckGLContainer extends React.Component {
   constructor(props) {
     super(props);
+    this.tick = this.tick.bind(this);
+    this.onViewportChange = this.onViewportChange.bind(this);
+    // This has to be placed after this.tick is bound to this
     this.state = {
       previousViewport: props.viewport,
       timer: setInterval(this.tick, TICK),
     };
-    this.tick = this.tick.bind(this);
-    this.onViewportChange = this.onViewportChange.bind(this);
   }
   static getDerivedStateFromProps(nextProps, prevState) {
     if (nextProps.viewport !== prevState.viewport) {
@@ -53,7 +55,9 @@ export default class DeckGLContainer extends React.Component {
   }
   tick() {
     // Limiting updating viewport controls through Redux at most 1*sec
-    if (this.state && this.state.previousViewport !== this.props.viewport) {
+    // Deep compare is needed as shallow equality doesn't work here, viewport 
object
+    // changes id at every change
+    if (this.state && !isEqual(this.state.previousViewport, 
this.props.viewport)) {
       const setCV = this.props.setControlValue;
       const vp = this.props.viewport;
       if (setCV) {
diff --git a/superset/assets/src/visualizations/deckgl/factory.jsx 
b/superset/assets/src/visualizations/deckgl/factory.jsx
index 48e29f6..6a33f61 100644
--- a/superset/assets/src/visualizations/deckgl/factory.jsx
+++ b/superset/assets/src/visualizations/deckgl/factory.jsx
@@ -1,5 +1,7 @@
 import React from 'react';
 import PropTypes from 'prop-types';
+import { isEqual } from 'lodash';
+
 import DeckGLContainer from './DeckGLContainer';
 import CategoricalDeckGLContainer from './CategoricalDeckGLContainer';
 import { fitViewport } from './layers/common';
@@ -18,36 +20,66 @@ const defaultProps = {
 };
 
 export function createDeckGLComponent(getLayer, getPoints) {
-  function Component(props) {
-    const {
-      formData,
-      payload,
-      setControlValue,
-      onAddFilter,
-      setTooltip,
-      viewport: originalViewport,
-    } = props;
-
-    const viewport = formData.autozoom
-      ? fitViewport(originalViewport, getPoints(payload.data.features))
-      : originalViewport;
-
-    const layer = getLayer(formData, payload, onAddFilter, setTooltip);
-
-    return (
-      <DeckGLContainer
-        mapboxApiAccessToken={payload.data.mapboxApiKey}
-        viewport={viewport}
-        layers={[layer]}
-        mapStyle={formData.mapbox_style}
-        setControlValue={setControlValue}
-      />
-    );
+  // Higher order component
+  class Component extends React.PureComponent {
+    constructor(props) {
+      super(props);
+      const originalViewport = props.viewport;
+      const viewport = props.formData.autozoom
+        ? fitViewport(originalViewport, getPoints(props.payload.data.features))
+        : originalViewport;
+      this.state = {
+        viewport,
+        layer: this.computeLayer(props),
+      };
+      this.onViewportChange = this.onViewportChange.bind(this);
+    }
+    componentWillReceiveProps(nextProps) {
+      // Only recompute the layer if anything BUT the viewport has changed
+      const nextFdNoVP = { ...nextProps.formData, viewport: null };
+      const currFdNoVP = { ...this.props.formData, viewport: null };
+      if (
+        !isEqual(nextFdNoVP, currFdNoVP) ||
+        nextProps.payload !== this.props.payload
+      ) {
+        this.setState({ layer: this.computeLayer(nextProps) });
+      }
+    }
+    onViewportChange(viewport) {
+      this.setState({ viewport });
+    }
+    computeLayer(props) {
+      const {
+        formData,
+        payload,
+        onAddFilter,
+        setTooltip,
+      } = props;
+      return getLayer(formData, payload, onAddFilter, setTooltip);
+    }
+    render() {
+      const {
+        formData,
+        payload,
+        setControlValue,
+      } = this.props;
+      const {
+        layer,
+        viewport,
+      } = this.state;
+      return (
+        <DeckGLContainer
+          mapboxApiAccessToken={payload.data.mapboxApiKey}
+          viewport={viewport}
+          layers={[layer]}
+          mapStyle={formData.mapbox_style}
+          setControlValue={setControlValue}
+          onViewportChange={this.onViewportChange}
+        />);
+    }
   }
-
   Component.propTypes = propTypes;
   Component.defaultProps = defaultProps;
-
   return Component;
 }
 
diff --git a/superset/assets/yarn.lock b/superset/assets/yarn.lock
index 3f28667..e4742cb 100644
--- a/superset/assets/yarn.lock
+++ b/superset/assets/yarn.lock
@@ -307,30 +307,30 @@
     d3-array "^1.2.0"
     prop-types "^15.5.10"
 
-"@deck.gl/core@^5.3.3":
-  version "5.3.3"
-  resolved 
"https://registry.yarnpkg.com/@deck.gl/core/-/core-5.3.3.tgz#a13c07e5fa3e22297fd450d6da8ab9aac334b1f0";
+"@deck.gl/core@^5.3.5":
+  version "5.3.5"
+  resolved 
"https://registry.yarnpkg.com/@deck.gl/core/-/core-5.3.5.tgz#24e6b3164a36b89b05cd020c2f7bcb5d08e3c266";
   dependencies:
-    luma.gl "^5.3.0"
+    luma.gl "^5.3.1"
     math.gl "^1.2.1"
     mjolnir.js "^1.0.0"
     probe.gl "^1.0.0"
     seer "^0.2.4"
     viewport-mercator-project "^5.1.0"
 
-"@deck.gl/layers@^5.3.4":
-  version "5.3.4"
-  resolved 
"https://registry.yarnpkg.com/@deck.gl/layers/-/layers-5.3.4.tgz#ab3de1bf8bb68d67772642acbb4e0f87f4f11300";
+"@deck.gl/layers@^5.3.5":
+  version "5.3.5"
+  resolved 
"https://registry.yarnpkg.com/@deck.gl/layers/-/layers-5.3.5.tgz#79c19be42961b909772e9dcd4eef5416d226d58e";
   dependencies:
-    "@deck.gl/core" "^5.3.3"
+    "@deck.gl/core" "^5.3.5"
     d3-hexbin "^0.2.1"
     earcut "^2.0.6"
 
-"@deck.gl/react@^5.3.3":
-  version "5.3.3"
-  resolved 
"https://registry.yarnpkg.com/@deck.gl/react/-/react-5.3.3.tgz#e7352934f6742d3ce672a394cbff312aab5ccaa0";
+"@deck.gl/react@^5.3.5":
+  version "5.3.5"
+  resolved 
"https://registry.yarnpkg.com/@deck.gl/react/-/react-5.3.5.tgz#caace72d4afc6531103fbb87292400de7c6f292a";
   dependencies:
-    "@deck.gl/core" "^5.3.3"
+    "@deck.gl/core" "^5.3.5"
     prop-types "^15.6.0"
 
 "@mapbox/geojson-area@0.2.2":
@@ -3679,13 +3679,13 @@ decimal.js@9.0.1:
   version "9.0.1"
   resolved 
"https://registry.yarnpkg.com/decimal.js/-/decimal.js-9.0.1.tgz#1cc8b228177da7ab6498c1cc06eb130a290e6e1e";
 
-deck.gl@^5.3.4:
-  version "5.3.4"
-  resolved 
"https://registry.yarnpkg.com/deck.gl/-/deck.gl-5.3.4.tgz#35e5a7087ef0d8ca7811d06a721ea289edbe7c24";
+deck.gl@^5.3.5:
+  version "5.3.5"
+  resolved 
"https://registry.yarnpkg.com/deck.gl/-/deck.gl-5.3.5.tgz#a30b8a6ee6caba1133167de461edc15ee3bb0f8b";
   dependencies:
-    "@deck.gl/core" "^5.3.3"
-    "@deck.gl/layers" "^5.3.4"
-    "@deck.gl/react" "^5.3.3"
+    "@deck.gl/core" "^5.3.5"
+    "@deck.gl/layers" "^5.3.5"
+    "@deck.gl/react" "^5.3.5"
 
 decode-uri-component@^0.2.0:
   version "0.2.0"
@@ -7241,9 +7241,9 @@ lru-cache@^4.0.1, lru-cache@^4.1.1, lru-cache@^4.1.3:
     pseudomap "^1.0.2"
     yallist "^2.1.2"
 
-luma.gl@^5.3.0:
-  version "5.3.0"
-  resolved 
"https://registry.yarnpkg.com/luma.gl/-/luma.gl-5.3.0.tgz#a93b2f34489d8230eb6d8c871335800d9b83ee67";
+luma.gl@^5.3.1:
+  version "5.3.1"
+  resolved 
"https://registry.yarnpkg.com/luma.gl/-/luma.gl-5.3.1.tgz#d380d554fe4c9de0b883e5d75f58d2a2142cfa05";
   dependencies:
     math.gl "^1.1.0"
     probe.gl "^1.0.0"

Reply via email to