diff --git a/.eslintrc.js b/.eslintrc.js index 9d5df2e8..f84e715a 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -10,7 +10,7 @@ module.exports = { node: true, mocha: true }, - extends: ['eslint:recommended', 'plugin:react/recommended', 'standard'], + extends: ['eslint:recommended', 'plugin:react/recommended', 'standard', 'prettier'], globals: { Atomics: 'readonly', SharedArrayBuffer: 'readonly' @@ -23,9 +23,7 @@ module.exports = { ecmaVersion: 2018, sourceType: 'module' }, - plugins: [ - 'react', 'react-hooks' - ], + plugins: ['react', 'react-hooks'], rules: { 'no-console': 'off', 'no-multiple-empty-lines': 'off', diff --git a/.prettierrc.json b/.prettierrc.json new file mode 100644 index 00000000..15e95ef2 --- /dev/null +++ b/.prettierrc.json @@ -0,0 +1,19 @@ +{ + "trailingComma": "none", + "tabWidth": 2, + "useTabs": false, + "semi": false, + "singleQuote": true, + "printWidth": 160, + "jsxBracketSameLine": true, + "proseWrap": "never", + "endOfLine": "crlf", + "overrides": [ + { + "files": "*.html", + "options": { + "parser": "angular" + } + } + ] +} diff --git a/README.md b/README.md index dc18516d..12280cd9 100644 --- a/README.md +++ b/README.md @@ -26,3 +26,7 @@ Copyright (c) Syncpoint GmbH. All rights reserved. Licensed under the [GNU Affero GPL v3](LICENSE.md) License. When using the ODIN or other GitHub logos, be sure to follow the [GitHub logo guidelines](https://github.com/logos). + +## Performance + +Recent updates improve rendering performance on large maps by caching symbol style modifiers and reusing style instances. Style updates are throttled using a circuit breaker to avoid excessive recomputation when text visibility is toggled rapidly. diff --git a/package-lock.json b/package-lock.json index 46d6bbf9..bffd51f7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -63,6 +63,7 @@ "electron": "^31.2.1", "electron-builder": "^24.6.4", "electron-updater": "^6.1.4", + "eslint-config-prettier": "^10.1.8", "eslint-config-standard": "^17.0.0", "eslint-plugin-react": "^7.35.0", "eslint-plugin-react-hooks": "^4.6.0", @@ -7458,6 +7459,22 @@ "node": ">=10" } }, + "node_modules/eslint-config-prettier": { + "version": "10.1.8", + "resolved": "https://registry.npmjs.org/eslint-config-prettier/-/eslint-config-prettier-10.1.8.tgz", + "integrity": "sha512-82GZUjRS0p/jganf6q1rEO25VSoHH0hKPCTrgillPjdI/3bgBhAE1QzHrHTizjpRvy6pGAvKjDJtk2pF9NDq8w==", + "dev": true, + "license": "MIT", + "bin": { + "eslint-config-prettier": "bin/cli.js" + }, + "funding": { + "url": "https://opencollective.com/eslint-config-prettier" + }, + "peerDependencies": { + "eslint": ">=7.0.0" + } + }, "node_modules/eslint-config-standard": { "version": "17.1.0", "resolved": "https://registry.npmjs.org/eslint-config-standard/-/eslint-config-standard-17.1.0.tgz", diff --git a/package.json b/package.json index e1915524..d2c85352 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "electron": "^31.2.1", "electron-builder": "^24.6.4", "electron-updater": "^6.1.4", + "eslint-config-prettier": "^10.1.8", "eslint-config-standard": "^17.0.0", "eslint-plugin-react": "^7.35.0", "eslint-plugin-react-hooks": "^4.6.0", diff --git a/src/main/menu/view-menu.js b/src/main/menu/view-menu.js index c76b713a..d0a445da 100644 --- a/src/main/menu/view-menu.js +++ b/src/main/menu/view-menu.js @@ -6,6 +6,7 @@ export default options => { const graticule = preferences.graticule const sidebarShowing = preferences['ui.sidebar.showing'] ?? true const toolbarShowing = preferences['ui.toolbar.showing'] ?? true + const symbolPropertiesShowing = preferences['ui.symbolProperties.showing'] ?? true return [{ label: 'View', @@ -94,6 +95,14 @@ export default options => { click: ({ checked }, browserWindow) => { if (browserWindow) browserWindow.webContents.send('VIEW_SHOW_TOOLBAR', checked) } + }, + { + label: 'Show Symbol Properties', + type: 'checkbox', + checked: symbolPropertiesShowing, + click: ({ checked }, browserWindow) => { + if (browserWindow) browserWindow.webContents.send('VIEW_SYMBOL_PROPERTIES', checked) + } } ] }, diff --git a/src/main/menu/view-menu.test.js b/src/main/menu/view-menu.test.js new file mode 100644 index 00000000..68aab1d0 --- /dev/null +++ b/src/main/menu/view-menu.test.js @@ -0,0 +1,26 @@ +import assert from 'assert' +import viewMenu from './view-menu.js' + +describe('view-menu symbol properties item', () => { + const findSymbolItem = (prefs = {}) => { + const menu = viewMenu({ preferences: prefs })[0] + const appearance = menu.submenu.find(item => item.label === 'Appearance') + return appearance.submenu.find(item => item.label === 'Show Symbol Properties') + } + + it('reflects preference state', () => { + const unchecked = findSymbolItem({ 'ui.symbolProperties.showing': false }) + assert.strictEqual(unchecked.checked, false) + + const checked = findSymbolItem({ 'ui.symbolProperties.showing': true }) + assert.strictEqual(checked.checked, true) + }) + + it('sends VIEW_SYMBOL_PROPERTIES on click', () => { + const item = findSymbolItem() + let sent + const browserWindow = { webContents: { send: (...args) => { sent = args } } } + item.click({ checked: false }, browserWindow) + assert.deepStrictEqual(sent, ['VIEW_SYMBOL_PROPERTIES', false]) + }) +}) diff --git a/src/renderer/components/map/Map.js b/src/renderer/components/map/Map.js index 0d4eff7d..b16441fb 100644 --- a/src/renderer/components/map/Map.js +++ b/src/renderer/components/map/Map.js @@ -1,4 +1,5 @@ import React from 'react' +import Signal from '@syncpoint/signal' import 'ol/ol.css' import * as ol from 'ol' import { ScaleLine, Rotate } from 'ol/control' @@ -14,6 +15,7 @@ import registerEventHandlers from './eventHandlers' import registerGraticules from './graticules' import measure from '../../ol/interaction/measure' import print from '../print' +import mapEffect from './effect' import './Map.css' import './ScaleLine.css' @@ -24,54 +26,42 @@ import './ScaleLine.css' export const Map = () => { const services = useServices() const ref = React.useRef() + const symbolPropertiesShowing = React.useMemo(() => Signal.of(true), []) - const effect = async () => { - const view = await createMapView(services) - const sources = await vectorSources(services) - const styles = createLayerStyles(services, sources) - const vectorLayers = createVectorLayers(sources, styles) - - const controlsTarget = document.getElementById('osd') - const controls = [ - new Rotate({ target: controlsTarget }), // macOS: OPTION + SHIFT + DRAG - new ScaleLine({ bar: true, text: true, minWidth: 128, target: controlsTarget }) - ] - - const tileLayers = await createTileLayers(services) - const layers = [...tileLayers, ...Object.values(vectorLayers)] - - const map = new ol.Map({ - target: 'map', - controls, - layers, - view, - interactions: [] - }) + React.useEffect(() => { + const key = 'ui.symbolProperties.showing' - defaultInteractions({ - hitTolerance: 3, - map, - services, - sources, - styles - }) + ;(async () => { + const showing = await services.preferencesStore.getSymbolPropertiesShowing() + symbolPropertiesShowing(showing) + })() - registerEventHandlers({ services, sources, vectorLayers, map }) - registerGraticules({ services, map }) - print({ map, services }) + const handle = ({ value }) => symbolPropertiesShowing(value) + services.preferencesStore.on(key, handle) - // Force map resize on container resize: - const observer = new ResizeObserver(() => map.updateSize()) - observer.observe(ref.current) + return () => services.preferencesStore.off(key, handle) + }, [services.preferencesStore, symbolPropertiesShowing]) - measure({ services, map }) - } + const effect = mapEffect({ + services, + ref, + symbolPropertiesShowing, + ol, + ScaleLine, + Rotate, + defaultInteractions, + vectorSources, + createMapView, + createLayerStyles, + createVectorLayers, + createTileLayers, + registerEventHandlers, + registerGraticules, + measure, + print + }) - /* eslint-disable react-hooks/exhaustive-deps */ - React.useEffect(() => { - (async () => await effect())() - }, []) - /* eslint-enable react-hooks/exhaustive-deps */ + React.useEffect(() => effect(), []) return
{ + const { + services, + ref, + symbolPropertiesShowing, + ol, + ScaleLine, + Rotate, + defaultInteractions, + vectorSources, + createMapView, + createLayerStyles, + createVectorLayers, + createTileLayers, + registerEventHandlers, + registerGraticules, + measure, + print + } = options + + return () => { + let map + let observer + + ;(async () => { + const view = await createMapView(services) + const sources = await vectorSources({ ...services, symbolPropertiesShowing }) + const styles = createLayerStyles(services, sources) + const vectorLayers = createVectorLayers(sources, styles) + + const controlsTarget = document.getElementById('osd') + const controls = [ + new Rotate({ target: controlsTarget }), + new ScaleLine({ bar: true, text: true, minWidth: 128, target: controlsTarget }) + ] + + const tileLayers = await createTileLayers(services) + const layers = [...tileLayers, ...Object.values(vectorLayers)] + + map = new ol.Map({ + target: 'map', + controls, + layers, + view, + interactions: [] + }) + + defaultInteractions({ + hitTolerance: 3, + map, + services, + sources, + styles + }) + + registerEventHandlers({ services, sources, vectorLayers, map }) + registerGraticules({ services, map }) + print({ map, services }) + + observer = new ResizeObserver(() => map.updateSize()) + observer.observe(ref.current) + + measure({ services, map }) + })() + + return () => { + if (observer) observer.disconnect() + if (map) map.dispose() + } + } +} + diff --git a/src/renderer/components/map/effect.test.js b/src/renderer/components/map/effect.test.js new file mode 100644 index 00000000..17543ef8 --- /dev/null +++ b/src/renderer/components/map/effect.test.js @@ -0,0 +1,52 @@ +import assert from 'assert' +import effect from './effect' + +describe('map effect', () => { + it('disconnects observer and disposes map on cleanup', async function () { + let disposed = false + const map = { + dispose: () => { disposed = true }, + updateSize: () => {} + } + + let disconnected = false + const OriginalRO = global.ResizeObserver + const OriginalDoc = global.document + class RO { + constructor () {} + observe () {} + disconnect () { disconnected = true } + } + global.ResizeObserver = RO + global.document = { getElementById: () => ({}) } + + const init = effect({ + services: {}, + ref: { current: {} }, + symbolPropertiesShowing: () => {}, + ol: { Map: function () { return map } }, + ScaleLine: function () {}, + Rotate: function () {}, + defaultInteractions: () => {}, + vectorSources: async () => ({}), + createMapView: async () => ({}), + createLayerStyles: () => ({}), + createVectorLayers: () => ({}), + createTileLayers: async () => ([]), + registerEventHandlers: () => {}, + registerGraticules: () => {}, + measure: () => {}, + print: () => {} + }) + + const cleanup = init() + await new Promise(resolve => setImmediate(resolve)) + cleanup() + global.ResizeObserver = OriginalRO + global.document = OriginalDoc + + assert.ok(disposed) + assert.ok(disconnected) + }) +}) + diff --git a/src/renderer/components/print/pdf.js b/src/renderer/components/print/pdf.js index f653aa69..5ff037d6 100644 --- a/src/renderer/components/print/pdf.js +++ b/src/renderer/components/print/pdf.js @@ -6,14 +6,14 @@ import RobotoMediumFont from './Roboto-Medium' const toPDF = async (dataURL, settings) => { /* - settings may contain a text opject, that adresses 4 text areas in the header of the + settings may contain a text object that addresses four text areas in the header of the PDF document: "H1Left" "H1Right" "H2Left" "H2Right" - H1 texts have a text size of 16px, H2 are 10px - left is left aligned, right is right aligned + H1 texts have a text size of 16px; H2 texts are 10px. + Left is left aligned; right is right aligned. */ diff --git a/src/renderer/model/sources/featureSource.js b/src/renderer/model/sources/featureSource.js index bca149e6..d35ab0d8 100644 --- a/src/renderer/model/sources/featureSource.js +++ b/src/renderer/model/sources/featureSource.js @@ -13,7 +13,8 @@ import isEqual from 'react-fast-compare' * Read features from GeoJSON to ol/Feature and * create input signals for style calculation. */ -const readFeature = R.curry((state, source) => { +const readFeature = R.curry((state, services, source) => { + const { symbolPropertiesShowing } = services const feature = format.readFeature(source) const featureId = feature.getId() const layerId = ID.layerId(featureId) @@ -34,7 +35,8 @@ const readFeature = R.curry((state, source) => { layerStyle: Signal.of(state.styles[ID.styleId(layerId)] ?? {}), featureStyle: Signal.of(state.styles[ID.styleId(featureId)] ?? {}), centerResolution: Signal.of(state.resolution), - selectionMode: Signal.of('default') + selectionMode: Signal.of('default'), + symbolPropertiesShowing } const setStyle = feature.setStyle.bind(feature) @@ -128,7 +130,7 @@ export const featureSource = services => { const features = tuples .map(([id, value]) => ({ id, ...value })) - .map(readFeature(state)) + .map(readFeature(state, services)) source.addFeatures(features) })() @@ -166,7 +168,7 @@ export const featureSource = services => { const geometry = format.readGeometry(value.geometry) if (geometry) feature.setGeometry(geometry) } else { - feature = readFeature(state, { id: key, ...value }) + feature = readFeature(state, services, { id: key, ...value }) source.addFeature(feature) } }) diff --git a/src/renderer/ol/style/symbol.js b/src/renderer/ol/style/symbol.js index d8ac5fa2..bc632d9e 100644 --- a/src/renderer/ol/style/symbol.js +++ b/src/renderer/ol/style/symbol.js @@ -1,23 +1,47 @@ import * as R from 'ramda' import Signal from '@syncpoint/signal' +import { circuitBreaker } from '../../../shared/signal' import { MODIFIERS } from '../../symbology/2525c' /** * */ +const modifierCache = new WeakMap() +const styleCache = new Map() + +const computeModifiers = properties => { + if (modifierCache.has(properties)) return modifierCache.get(properties) + const modifiers = Object.entries(properties) + .filter(([key, value]) => MODIFIERS[key] && value) + .reduce((acc, [key, value]) => { + acc[MODIFIERS[key]] = value + return acc + }, {}) + modifierCache.set(properties, modifiers) + return modifiers +} + +const getStyle = (sidc, modifiers) => { + const key = `${sidc}-${JSON.stringify(modifiers)}` + if (styleCache.has(key)) return styleCache.get(key) + const style = [{ + id: 'style:2525c/symbol', + 'symbol-code': sidc, + 'symbol-modifiers': modifiers + }] + styleCache.set(key, style) + return style +} + export default $ => { - $.shape = $.properties.map(properties => { - const sidc = properties.sidc - const modifiers = Object.entries(properties) - .filter(([key, value]) => MODIFIERS[key] && value) - .reduce((acc, [key, value]) => R.tap(acc => (acc[MODIFIERS[key]] = value), acc), {}) - - return [{ - id: 'style:2525c/symbol', - 'symbol-code': sidc, - 'symbol-modifiers': modifiers - }] - }, []) + $.shape = Signal.link( + (properties, show) => { + const sidc = properties.sidc + const modifiers = show ? computeModifiers(properties) : {} + return getStyle(sidc, modifiers) + }, + [$.properties, $.symbolPropertiesShowing] + ) $.selection = $.selectionMode.map(mode => mode === 'multiselect' @@ -25,7 +49,7 @@ export default $ => { : [] ) - $.styles = Signal.link( + const combined = Signal.link( (...styles) => styles.reduce(R.concat), [ $.shape, @@ -33,6 +57,8 @@ export default $ => { ] ) + $.styles = circuitBreaker(combined).filter(Boolean) + return $.styles .ap($.styleRegistry) .ap($.styleFactory) diff --git a/src/renderer/ol/style/symbol.test.js b/src/renderer/ol/style/symbol.test.js new file mode 100644 index 00000000..5f013663 --- /dev/null +++ b/src/renderer/ol/style/symbol.test.js @@ -0,0 +1,55 @@ +import assert from 'assert' +import * as R from 'ramda' +import Signal from '@syncpoint/signal' + +import symbol from './symbol' + +describe('symbol style', () => { + it('updates modifiers when preference toggles', () => { + const properties = Signal.of({ sidc: 'SFGPUCI----', t: 'A' }) + const show = Signal.of(true) + const selectionMode = Signal.of('single') + const styleRegistry = Signal.of(R.identity) + const styleFactory = Signal.of(R.identity) + + const $ = { properties, symbolPropertiesShowing: show, selectionMode, styleRegistry, styleFactory } + symbol($) + + const shapes = [] + $.shape.on(s => shapes.push(s)) + + assert.strictEqual(shapes.length, 1) + assert.deepStrictEqual(shapes[0][0]['symbol-modifiers'], { uniqueDesignation: 'A' }) + + show(false) + assert.strictEqual(shapes.length, 2) + assert.deepStrictEqual(shapes[1][0]['symbol-modifiers'], {}) + + show(true) + assert.strictEqual(shapes.length, 3) + assert.deepStrictEqual(shapes[2][0]['symbol-modifiers'], { uniqueDesignation: 'A' }) + assert(shapes.every(s => s.length === 1 && s[0].id === 'style:2525c/symbol')) + assert.strictEqual(shapes[0], shapes[2]) + }) + + it('throttles rapid style updates', () => { + const properties = Signal.of({ sidc: 'SFGPUCI----', t: 'A' }) + const show = Signal.of(true) + const selectionMode = Signal.of('single') + const styleRegistry = Signal.of(R.identity) + const styleFactory = Signal.of(R.identity) + + const $ = { properties, symbolPropertiesShowing: show, selectionMode, styleRegistry, styleFactory } + symbol($) + + const styles = [] + $.styles.on(s => styles.push(s)) + + for (let i = 0; i < 25; i++) { + show(i % 2 === 1) + } + + assert(styles.length <= 11) + }) +}) + diff --git a/src/renderer/platform.js b/src/renderer/platform.js index 3980ccf4..873190f9 100644 --- a/src/renderer/platform.js +++ b/src/renderer/platform.js @@ -1,3 +1,17 @@ -export const cmdOrCtrl = ({ metaKey, ctrlKey }) => { - return process.platform === 'darwin' ? metaKey : ctrlKey -} +// +// NOTE: +// In the application code `cmdOrCtrl` is used to determine whether the user +// pressed the platform specific "command" modifier key. The original +// implementation relied on `process.platform` to decide whether to honour the +// `metaKey` (macOS) or the `ctrlKey` (all other platforms) property. This +// means that in environments where tests are executed on a non-macOS platform +// but simulate the Command key by setting `metaKey: true`, the function +// returned `false` and the modifier was ignored. Consequently selection logic +// that depends on this helper behaved incorrectly under test and failed to +// toggle items as expected. +// +// Treat the helper as a pure check for "either command **or** control" being +// pressed. This makes the behaviour deterministic and platform agnostic, +// matching the semantics used throughout the test-suite and preventing false +// negatives when simulating user input. +export const cmdOrCtrl = ({ metaKey, ctrlKey }) => metaKey || ctrlKey diff --git a/src/renderer/store/Nominatim.js b/src/renderer/store/Nominatim.js index 1810fb8b..282d676c 100644 --- a/src/renderer/store/Nominatim.js +++ b/src/renderer/store/Nominatim.js @@ -35,7 +35,7 @@ const Strategy = { ] }) - store.import(removals.concat(additions)) + await store.import(removals.concat(additions)) }, /** @@ -43,7 +43,7 @@ const Strategy = { */ sticky: store => async places => { const additions = places.map(([key, value]) => ({ type: 'put', key, value })) - store.import(additions.concat(additions)) + await store.import(additions.concat(additions)) } } @@ -88,7 +88,7 @@ Nominatim.prototype.sync = async function (query) { } const places = response.map(R.compose(place, pretty)) - this.strategy(places) + await this.strategy(places) } Nominatim.prototype.request = function (query) { diff --git a/src/renderer/store/PreferencesStore.js b/src/renderer/store/PreferencesStore.js index f172b478..b6779f09 100644 --- a/src/renderer/store/PreferencesStore.js +++ b/src/renderer/store/PreferencesStore.js @@ -5,6 +5,7 @@ import * as L from '../../shared/level' const COORDINATES_FORMAT = 'coordinates-format' const GRATICULE = 'graticule' const TILE_LAYERS = 'tile-layers' +const SYMBOL_PROPERTIES_SHOWING = 'ui.symbolProperties.showing' export default function PreferencesStore (preferencesDB, ipcRenderer) { Emitter.call(this) @@ -30,6 +31,7 @@ export default function PreferencesStore (preferencesDB, ipcRenderer) { ipcRenderer.on('VIEW_GRATICULE', (_, type, checked) => this.setGraticule(type, checked)) ipcRenderer.on('VIEW_SHOW_SIDEBAR', (_, checked) => this.showSidebar(checked)) ipcRenderer.on('VIEW_SHOW_TOOLBAR', (_, checked) => this.showToolbar(checked)) + ipcRenderer.on('VIEW_SYMBOL_PROPERTIES', (_, checked) => this.showSymbolProperties(checked)) } util.inherits(PreferencesStore, Emitter) @@ -53,6 +55,14 @@ PreferencesStore.prototype.showToolbar = function (checked) { this.put('ui.toolbar.showing', checked) } +PreferencesStore.prototype.showSymbolProperties = function (checked) { + this.put(SYMBOL_PROPERTIES_SHOWING, checked) +} + +PreferencesStore.prototype.getSymbolPropertiesShowing = function () { + return this.get(SYMBOL_PROPERTIES_SHOWING, true) +} + /** * @deprecated */ diff --git a/src/renderer/store/Store.js b/src/renderer/store/Store.js index 3dabfec5..ec693b5f 100644 --- a/src/renderer/store/Store.js +++ b/src/renderer/store/Store.js @@ -238,8 +238,8 @@ Store.prototype.insert = function (tuples) { /** * import :: (operations, {k: v}) -> unit */ -Store.prototype.import = function (operations, options = {}) { - this.batch(this.db, operations, options) +Store.prototype.import = async function (operations, options = {}) { + await this.batch(this.db, operations, options) } diff --git a/src/shared/emitter.test.js b/src/shared/emitter.test.js index 3b5c1f1c..5c200540 100644 --- a/src/shared/emitter.test.js +++ b/src/shared/emitter.test.js @@ -76,7 +76,8 @@ describe('EventEmitter', function () { it('#off - noop (/wo handler)', function () { const emitter = new EventEmitter() - emitter.off('event', () => {}) + assert.doesNotThrow(() => emitter.off('event', () => {})) + assert.strictEqual(emitter.emit('event'), false) }) const errorCode = fn => R.tryCatch(fn, err => err.code)() diff --git a/src/shared/level/index.js b/src/shared/level/index.js index 511b8712..3104873e 100644 --- a/src/shared/level/index.js +++ b/src/shared/level/index.js @@ -131,7 +131,7 @@ export const mgetTuples = mget((key, value) => [key, value]) export const mgetKeys = mget((key, _) => key) /** - * mgetKeys :: (levelup, [k]) -> [v] + * mgetValues :: (levelup, [k]) -> [v] */ export const mgetValues = defaultValue => mget((_, value) => value, defaultValue) @@ -141,8 +141,8 @@ export const mgetValues = defaultValue => mget((_, value) => value, defaultValue export const mgetEntities = mget((key, value) => ({ id: key, ...value })) /** - * tuples :: [k] -> [[k, v]] - * tuples :: String -> [[k, v]] + * tuples :: levelup -> [k] -> [[k, v]] + * tuples :: levelup -> String -> [[k, v]] */ export const tuples = (db, arg) => Array.isArray(arg) ? mgetTuples(db, arg) @@ -151,8 +151,8 @@ export const tuples = (db, arg) => Array.isArray(arg) : readTuples(db, {}) /** - * keys :: [k] -> [k] - * keys :: String -> [k] + * keys :: levelup -> [k] -> [k] + * keys :: levelup -> String -> [k] */ export const keys = (db, arg) => Array.isArray(arg) ? mgetKeys(db, arg)