Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resizing on changeFlags.viewportChanged slower than storing viewState in application state #4504

Closed
ilan-gold opened this issue Apr 15, 2020 · 4 comments
Labels

Comments

@ilan-gold
Copy link
Collaborator

ilan-gold commented Apr 15, 2020

Hello, I am trying to make a resizable scale bar layer. To do this, I am trying to implement a custom layer that wraps a couple of LineLayers. However, the performance is a little jumpy. I have noticed that it is considerably less jumpy if, instead of calculating the bounding box in the renderLayers() call, I store the viewState in state, and use that instead to directly create a LineLayer. Am I missing some sort of data comparator or the like? I would totally believe that. Sorry if this is something silly!

UPDATE: I realized I wasn't totally clear. The idea is to resize as you zoom in and out, which is what is causing the update in the below screen-grab.

Here is a screen-grab - top left is using state and bottom left is using this.context.viewport for projection.

ezgif com-video-to-gif(1)

Here is some code for reproduction:

import React, {PureComponent} from 'react';
import {render} from 'react-dom';

import DeckGL, {OrthographicView, COORDINATE_SYSTEM, CompositeLayer} from 'deck.gl';
import {LineLayer} from '@deck.gl/layers';

const INITIAL_VIEW_STATE = {
  target: [0, 0, 0],
  zoom: 0,
  height: window.innerHeight,
  width: window.innerWidth
};

// Custom scale bar layer that resizes using `unproject` internally.
class ScaleBarLayer extends CompositeLayer {
  // This Layer reacts to viewport changes.
  // eslint-disable-next-line class-methods-use-this
  shouldUpdateState({changeFlags}) {
    return changeFlags.viewportChanged;
  }

  renderLayers() {
    const {viewport} = this.context;
    // Calculate the bounding box of the viewport and locate the bar in the bottom right, scaling as we zoom out.
    const boundingBox = [
      viewport.unproject([0, 0]),
      viewport.unproject([viewport.width, 0]),
      viewport.unproject([viewport.width, viewport.height]),
      viewport.unproject([0, viewport.height])
    ];
    const yCoord = boundingBox[2][1] - (boundingBox[2][1] - boundingBox[0][1]) * 0.1;
    const xLeftCoord = boundingBox[2][0] - (boundingBox[2][0] - boundingBox[0][0]) * 0.1;
    const numUnits = (boundingBox[2][0] - boundingBox[0][0]) * 0.05 * 2 ** -viewport.zoom;
    const lengthBar = new LineLayer({
      id: `scale-bar-length-CompositeLayer`,
      coordinateSystem: COORDINATE_SYSTEM.CARTESIAN,
      data: [[[xLeftCoord, yCoord], [xLeftCoord + numUnits, yCoord]]],
      getSourcePosition: d => d[0],
      getTargetPosition: d => d[1],
      getWidth: 2,
      getColor: [220, 220, 220]
    });
    return [lengthBar];
  }
}

ScaleBarLayer.layerName = 'ScaleBarLayer';

export default class App extends PureComponent {
  constructor(props) {
    super(props);
    this.state = {
      viewState: INITIAL_VIEW_STATE
    };
    this.onViewStateChange = this.onViewStateChange.bind(this);
  }

  // Hold the viewState in state to allow for direct creation of LineLayer.
  onViewStateChange({viewState}) {
    this.setState({viewState});
  }
  _renderLayers() {
    const {viewState} = this.state;
    // Calculate the bounding box of the viewport and locate the bar in the top right, scaling as we zoom out.
    const viewport = new OrthographicView().makeViewport({
      viewState,
      height: viewState.height,
      width: viewState.width
    });
    const boundingBox = [
      viewport.unproject([0, 0]),
      viewport.unproject([viewport.width, 0]),
      viewport.unproject([viewport.width, viewport.height]),
      viewport.unproject([0, viewport.height])
    ];
    const yCoord = boundingBox[0][1] + (boundingBox[2][1] - boundingBox[0][1]) * 0.1;
    const xLeftCoord = boundingBox[0][0] + (boundingBox[2][0] - boundingBox[0][0]) * 0.1;
    const numUnits = (boundingBox[2][0] - boundingBox[0][0]) * 0.05 * 2 ** -viewport.zoom;
    return [
      // Directly use bounding box calculated here.
      new LineLayer({
        id: `scale-bar-length-LineLayer`,
        coordinateSystem: COORDINATE_SYSTEM.CARTESIAN,
        data: [[[xLeftCoord, yCoord], [xLeftCoord + numUnits, yCoord]]],
        getSourcePosition: d => d[0],
        getTargetPosition: d => d[1],
        getWidth: 2,
        getColor: [220, 220, 220]
      }),
      // Custom composite layer.
      new ScaleBarLayer()
    ];
  }

  render() {
    return (
      <DeckGL
        views={[new OrthographicView({id: 'ortho'})]}
        layers={this._renderLayers()}
        viewState={this.state.viewState}
        controller={true}
        onViewStateChange={this.onViewStateChange}
      >
        {this._renderTooltip}
      </DeckGL>
    );
  }
}

export function renderToDOM(container) {
  render(<App />, container);
}
@Pessimistress
Copy link
Collaborator

dataOrPropChanged is triggered when the layers are updated; viewportChanged is triggered when a layer is being rendered. The latter does not update sublayers recursively, so you see them lagging one frame behind.

An easier way to do this is via multi-view:

new Deck({
  views: [
    new OrthographicView({id: 'main', controller: true}),
    new OrthographicView({id: 'legend', x: 0, y: 0, target: [0, 0]})
  ],
  layers: [
    new LineLayer({
      id: 'scale-bar',
      data: [[0, 0], [100, 0]],
      getSourcePosition: d => d[0],
      getTargetPosition: d => d[1],
      getWidth: 2,
      getColor: [220, 220, 220]
    })
  ],
  layerFilter: ({layer, viewport}) => {
    if ( layer.id === 'scale-bar' ) return viewport.id === 'legend';
    return true;
  }
})

@ilan-gold
Copy link
Collaborator Author

@Pessimistress Is there a way to force the recursive update? I was mimicking the TileLayer as documented here with respect to shouldUpdateState, although this layer doesn't have state. Even when shouldUpdateState was always set to return true, I didn't get any better performance. Maybe I am misunderstanding you though about propsOrDataChanged....

Also, for multiple views, is there a way to make the one on top transparent? If both the answers to those are negative, then I will probably just stick with storing the viewState in state and close this out.

@Pessimistress
Copy link
Collaborator

The way it's implemented now, you can't update layers recursively on viewport change without substantially affecting perf. We can revisit this in the next major release.

Multiple views can be on top of each other, and by default they just render their own content without clearing the region (aka transparent).

@ilan-gold
Copy link
Collaborator Author

Thank you for the clarification on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants