From 1d7294dc926da51198beab1d73c2f78ce71e5d5d Mon Sep 17 00:00:00 2001 From: lukasmartinelli Date: Sat, 10 Sep 2016 17:31:45 +0200 Subject: [PATCH] Use OrderedMap for layers for perf --- src/layers/list.jsx | 40 +++++++++------------------------------- src/map.jsx | 7 ++++--- src/stylestore.js | 28 ++++++++++++++++++++++------ 3 files changed, 35 insertions(+), 40 deletions(-) diff --git a/src/layers/list.jsx b/src/layers/list.jsx index 4146ebd..8185ddf 100644 --- a/src/layers/list.jsx +++ b/src/layers/list.jsx @@ -7,7 +7,7 @@ import scrollbars from '../scrollbars.scss' // List of collapsible layer editors export class LayerList extends React.Component { static propTypes = { - layers: React.PropTypes.instanceOf(Immutable.List), + layers: React.PropTypes.instanceOf(Immutable.OrderedMap), onLayersChanged: React.PropTypes.func.isRequired } @@ -16,47 +16,25 @@ export class LayerList extends React.Component { } onLayerDestroyed(deletedLayer) { - //TODO: That's just horrible... - // Can we use a immutable ordered map to look up and guarantee order - // at the same time? - let deleteIdx = -1 - for (let entry of this.props.layers.entries()) { - let [i, layer] = entry - if(layer.get('id') == deletedLayer.get('id')) { - deleteIdx = i - break - } - } - - this.props.onLayersChanged(this.props.layers.delete(deleteIdx)) + const remainingLayers = this.props.layers.delete(deletedLayer.get('id')) + this.props.onLayersChanged(remainingLayers) } - onLayerChanged(changedLayer) { - //TODO: That's just horrible... - let changeIdx = -1 - for (let entry of this.props.layers.entries()) { - let [i, layer] = entry - if(layer.get('id') == changedLayer.get('id')) { - changeIdx = i - break - } - } - - const changedLayers = this.props.layers.set(changeIdx, changedLayer) + onLayerChanged(layer) { + const changedLayers = this.props.layers.set(layer.get('id'), layer) this.props.onLayersChanged(changedLayers) } render() { var layerPanels = [] - - for(let layer of this.props.layers) { - layerPanels.push( { + return ) - } + /> + }).toIndexedSeq() return
diff --git a/src/map.jsx b/src/map.jsx index 9dd52f0..fa471a9 100644 --- a/src/map.jsx +++ b/src/map.jsx @@ -2,6 +2,7 @@ import React from 'react' import MapboxGl from 'mapbox-gl'; import diffStyles from 'mapbox-gl-style-spec/lib/diff' import { fullHeight } from './theme.js' +import { styleToJS } from './stylestore.js' import Immutable from 'immutable' export class Map extends React.Component { @@ -18,7 +19,7 @@ export class Map extends React.Component { const mapIdChanged = this.props.mapStyle.get('id') !== nextProps.mapStyle.get('id') if(mapIdChanged || tokenChanged) { - this.state.map.setStyle(nextProps.mapStyle.toJS()) + this.state.map.setStyle(styleToJS(nextProps.mapStyle)) return } @@ -28,7 +29,7 @@ export class Map extends React.Component { //TODO: Write own diff algo that operates on immutable collections // Should be able to improve performance since we can only compare // by reference - const changes = diffStyles(this.props.mapStyle.toJS(), nextProps.mapStyle.toJS()) + const changes = diffStyles(styleToJS(this.props.mapStyle), styleToJS(nextProps.mapStyle)) changes.forEach(change => { this.state.map[change.command].apply(this.state.map, change.args); }); @@ -44,7 +45,7 @@ export class Map extends React.Component { const map = new MapboxGl.Map({ container: this.container, - style: this.props.mapStyle.toJS(), + style: styleToJS(this.props.mapStyle), }); map.on("style.load", (...args) => { diff --git a/src/stylestore.js b/src/stylestore.js index 07f0257..25204cd 100644 --- a/src/stylestore.js +++ b/src/stylestore.js @@ -8,7 +8,7 @@ const storageKeys = { } // Empty style is always used if no style could be restored or fetched -const emptyStyle = ensureOptionalStyleProps(Immutable.fromJS({ +const emptyStyle = ensureOptionalStyleProps(makeStyleImmutable({ version: 8, sources: {}, layers: [], @@ -16,6 +16,20 @@ const emptyStyle = ensureOptionalStyleProps(Immutable.fromJS({ const defaultStyleUrl = "https://raw.githubusercontent.com/osm2vectortiles/mapbox-gl-styles/master/styles/basic-v9-cdn.json" +// TODO: Stop converting around so much.. we should make a module containing the immutable style stuff +export function styleToJS(mapStyle) { + const jsonStyle = mapStyle.toJS() + jsonStyle.layers = mapStyle.get('layers').toIndexedSeq().toJS() + return jsonStyle +} + +function makeStyleImmutable(mapStyle) { + if(mapStyle instanceof Immutable.Map) return mapStyle + const style = Immutable.fromJS(mapStyle) + const orderdLayers = Immutable.OrderedMap(mapStyle.layers.map(l => [l.id, Immutable.fromJS(l)])) + return style.set('layers', orderdLayers) +} + // Fetch a default style via URL and return it or a fallback style via callback export function loadDefaultStyle(cb) { var request = new XMLHttpRequest(); @@ -23,7 +37,7 @@ export function loadDefaultStyle(cb) { request.onload = () => { if (request.status >= 200 && request.status < 400) { - cb(Immutable.fromJS(JSON.parse(request.responseText))) + cb(makeStyleImmutable(JSON.parse(request.responseText))) } else { cb(emptyStyle) } @@ -102,20 +116,22 @@ export class StyleStore { // Find the last edited style latestStyle() { - if(this.mapStyles.length == 0) return emptyStyle + if(this.mapStyles.length === 0) return emptyStyle const styleId = window.localStorage.getItem(storageKeys.latest) const styleItem = window.localStorage.getItem(styleKey(styleId)) - return Immutable.fromJS(JSON.parse(styleItem)) + + if(styleItem) return makeStyleImmutable(JSON.parse(styleItem)) + return memptyStyle } // Save current style replacing previous version save(mapStyle) { if(!(mapStyle instanceof Immutable.Map)) { - mapStyle = Immutable.fromJS(mapStyle) + mapStyle = makeStyleImmutable(mapStyle) } mapStyle = ensureOptionalStyleProps(mapStyle) const key = styleKey(mapStyle.get('id')) - window.localStorage.setItem(key, JSON.stringify(mapStyle.toJS())) + window.localStorage.setItem(key, JSON.stringify(styleToJS(mapStyle))) window.localStorage.setItem(storageKeys.latest, mapStyle.get('id')) return mapStyle }