othmane/ohif-viewer
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix: avoid-wasteful-renders (#1544)

Browse Source 
* Avoid un-necessary redux state update for frequent actions

* Avoid un-necessary re-render when no change in props

Co-authored-by: kedar.netelixir <kedar.choudhary@netelixir.com>
This commit is contained in:
Kedar 2020-03-24 00:01:39 +05:30 committed by GitHub
parent e374fe2118
commit e41d339f5f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 96 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
platform/core/package.json
View File

@ -45,6 +45,7 @@
"lodash.clonedeep": "^4.5.0", "lodash.clonedeep": "^4.5.0",
"lodash.merge": "^4.6.1", "lodash.merge": "^4.6.1",
"mousetrap": "^1.6.3", "mousetrap": "^1.6.3",
"validate.js": "^0.12.0" "validate.js": "^0.12.0",
"immer": "6.0.2"
} }
} }

40
platform/core/src/redux/reducers/viewports.js
View File

@ -1,5 +1,5 @@
import cloneDeep from 'lodash.clonedeep'; import cloneDeep from 'lodash.clonedeep';
import merge from 'lodash.merge'; import produce from 'immer';
import { import {
CLEAR_VIEWPORT, CLEAR_VIEWPORT,
@ -93,12 +93,13 @@ const viewports = (state = DEFAULT_STATE, action) => {
* @return {Object} New state. * @return {Object} New state.
*/ */
case SET_VIEWPORT_ACTIVE: { case SET_VIEWPORT_ACTIVE: {
const activeViewportIndex = getActiveViewportIndex( return produce(state, draftState => {
state.numRows, draftState.activeViewportIndex = getActiveViewportIndex(
state.numColumns, draftState.numRows,
action.viewportIndex draftState.numColumns,
); action.viewportIndex
return { ...state, activeViewportIndex }; );
});
} }
/** /**
@ -163,21 +164,20 @@ const viewports = (state = DEFAULT_STATE, action) => {
* @return {Object} New state. * @return {Object} New state.
*/ */
case SET_VIEWPORT: { case SET_VIEWPORT: {
const layout = cloneDeep(state.layout); return produce(state, draftState => {
draftState.viewportSpecificData[action.viewportIndex] =
draftState.viewportSpecificData[action.viewportIndex] || {};
let viewportSpecificData = cloneDeep(state.viewportSpecificData); Object.keys(action.viewportSpecificData).forEach(key => {
viewportSpecificData[action.viewportIndex] = merge( draftState.viewportSpecificData[action.viewportIndex][key] =
{}, action.viewportSpecificData[key];
viewportSpecificData[action.viewportIndex], });
action.viewportSpecificData
);
if (action.viewportSpecificData && action.viewportSpecificData.plugin) { if (action.viewportSpecificData && action.viewportSpecificData.plugin) {
layout.viewports[action.viewportIndex].plugin = draftState.layout.viewports[action.viewportIndex].plugin =
action.viewportSpecificData.plugin; action.viewportSpecificData.plugin;
} }
});
return { ...state, layout, viewportSpecificData };
} }
/** /**

11
platform/viewer/src/components/ViewportGrid/ConnectedViewportGrid.js
View File

@ -2,15 +2,20 @@ import ViewportGrid from './ViewportGrid.js';
import { MODULE_TYPES } from '@ohif/core'; import { MODULE_TYPES } from '@ohif/core';
import { connect } from 'react-redux'; import { connect } from 'react-redux';
import { extensionManager } from './../../App.js'; import { extensionManager } from './../../App.js';
import memoize from 'lodash/memoize';
const mapStateToProps = state => { const getAvailableViewportModules = memoize(viewportModules => {
const availableViewportModules = {}; const availableViewportModules = {};
const viewportModules = extensionManager.modules[MODULE_TYPES.VIEWPORT];
viewportModules.forEach(moduleDefinition => { viewportModules.forEach(moduleDefinition => {
availableViewportModules[moduleDefinition.extensionId] = availableViewportModules[moduleDefinition.extensionId] =
moduleDefinition.module; moduleDefinition.module;
}); });
return availableViewportModules;
});
const mapStateToProps = state => {
const viewportModules = extensionManager.modules[MODULE_TYPES.VIEWPORT];
const availableViewportModules = getAvailableViewportModules(viewportModules);
// TODO: Use something like state.plugins.defaultPlugin[MODULE_TYPES.VIEWPORT] // TODO: Use something like state.plugins.defaultPlugin[MODULE_TYPES.VIEWPORT]
let defaultPlugin; let defaultPlugin;

110
platform/viewer/src/components/ViewportGrid/ViewportGrid.js
View File

@ -39,66 +39,68 @@ const ViewportGrid = function(props) {
}); });
}, [viewportData]); }, [viewportData]);
const ViewportPanes = layout.viewports.map((layout, viewportIndex) => { const getViewportPanes = () =>
const displaySet = viewportData[viewportIndex]; layout.viewports.map((layout, viewportIndex) => {
const displaySet = viewportData[viewportIndex];
if (!displaySet) { if (!displaySet) {
return null; return null;
} }
const data = { const data = {
displaySet, displaySet,
studies, studies,
}; };
// JAMES TODO: // JAMES TODO:
// Use whichever plugin is currently in use in the panel // Use whichever plugin is currently in use in the panel
// unless nothing is specified. If nothing is specified // unless nothing is specified. If nothing is specified
// and the display set has a plugin specified, use that. // and the display set has a plugin specified, use that.
// //
// TODO: Change this logic to: // TODO: Change this logic to:
// - Plugins define how capable they are of displaying a SopClass // - Plugins define how capable they are of displaying a SopClass
// - When updating a panel, ensure that the currently enabled plugin // - When updating a panel, ensure that the currently enabled plugin
// in the viewport is capable of rendering this display set. If not // in the viewport is capable of rendering this display set. If not
// then use the most capable available plugin // then use the most capable available plugin
const pluginName = const pluginName =
!layout.plugin && displaySet && displaySet.plugin !layout.plugin && displaySet && displaySet.plugin
? displaySet.plugin ? displaySet.plugin
: layout.plugin; : layout.plugin;
const ViewportComponent = _getViewportComponent( const ViewportComponent = _getViewportComponent(
data, // Why do we pass this as `ViewportData`, when that's not really what it is? data, // Why do we pass this as `ViewportData`, when that's not really what it is?
viewportIndex, viewportIndex,
children, children,
availablePlugins, availablePlugins,
pluginName, pluginName,
defaultPluginName defaultPluginName
); );
return ( return (
<ViewportPane <ViewportPane
onDrop={({ onDrop={setViewportData}
viewportIndex, viewportIndex={viewportIndex} // Needed by `setViewportData`
StudyInstanceUID, className={classNames('viewport-container', {
displaySetInstanceUID, active: activeViewportIndex === viewportIndex,
}) => { })}
setViewportData({ key={viewportIndex}
viewportIndex, >
StudyInstanceUID, {ViewportComponent}
displaySetInstanceUID, </ViewportPane>
}); );
}} });
viewportIndex={viewportIndex} // Needed by `setViewportData`
className={classNames('viewport-container', { const ViewportPanes = React.useMemo(getViewportPanes, [
active: activeViewportIndex === viewportIndex, layout,
})} viewportData,
key={viewportIndex} studies,
> children,
{ViewportComponent} availablePlugins,
</ViewportPane> defaultPluginName,
); setViewportData,
}); activeViewportIndex,
]);
return ( return (
<div <div

8
platform/viewer/src/connectedComponents/ViewerMain.js
View File

@ -4,6 +4,10 @@ import { Component } from 'react';
import { ConnectedViewportGrid } from './../components/ViewportGrid/index.js'; import { ConnectedViewportGrid } from './../components/ViewportGrid/index.js';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import React from 'react'; import React from 'react';
import memoize from 'lodash/memoize';
import _values from 'lodash/values';
var values = memoize(_values);
class ViewerMain extends Component { class ViewerMain extends Component {
static propTypes = { static propTypes = {
@ -140,9 +144,7 @@ class ViewerMain extends Component {
render() { render() {
const { viewportSpecificData } = this.props; const { viewportSpecificData } = this.props;
const viewportData = viewportSpecificData const viewportData = values(viewportSpecificData);
? Object.values(viewportSpecificData)
: [];
return ( return (
<div className="ViewerMain"> <div className="ViewerMain">

5
yarn.lock
View File

@ -9922,6 +9922,11 @@ immer@1.10.0:
resolved "https://registry.yarnpkg.com/immer/-/immer-1.10.0.tgz#bad67605ba9c810275d91e1c2a47d4582e98286d" resolved "https://registry.yarnpkg.com/immer/-/immer-1.10.0.tgz#bad67605ba9c810275d91e1c2a47d4582e98286d"
integrity sha512-O3sR1/opvCDGLEVcvrGTMtLac8GJ5IwZC4puPrLuRj3l7ICKvkmA0vGuU9OW8mV9WIBRnaxp5GJh9IEAaNOoYg== integrity sha512-O3sR1/opvCDGLEVcvrGTMtLac8GJ5IwZC4puPrLuRj3l7ICKvkmA0vGuU9OW8mV9WIBRnaxp5GJh9IEAaNOoYg==
immer@6.0.2:
version "6.0.2"
resolved "https://registry.yarnpkg.com/immer/-/immer-6.0.2.tgz#5bc08dc4930c756d0749533a2afbd88c8de0cd19"
integrity sha512-56CMvUMZl4kkWJFFUe1TjBgGbyb9ibzpLyHD+RSKSVdytuDXgT/HXO1S+GJVywMVl5neGTdAogoR15eRVEd10Q==
immutable@>=3.6.0: immutable@>=3.6.0:
version "3.8.2" version "3.8.2"
resolved "https://registry.yarnpkg.com/immutable/-/immutable-3.8.2.tgz#c2439951455bb39913daf281376f1530e104adf3" resolved "https://registry.yarnpkg.com/immutable/-/immutable-3.8.2.tgz#c2439951455bb39913daf281376f1530e104adf3"