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.

Sign up for GitHub

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

DM-10830: Optimize the performance of multi-trace chart updates #397

Merged
merged 9 commits into from
Jun 29, 2017
Merged

DM-10830: Optimize the performance of multi-trace chart updates #397

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c8743ad
DM-10830: Optimize the performance of multi-trace chart updates
loitly Jun 23, 2017
d76da65
fixes highlighting selected, bug due to shared data
tgoldina Jun 27, 2017
eadf0f8
DM-10830: fixes made per review
loitly Jun 27, 2017
f1e8e66
DM-10830: additinal fixes
loitly Jun 27, 2017
f266326
- add back the non-supported symbols since we're not using GL.
loitly Jun 27, 2017
ab8f390
- fix wrong use of boolean function.
loitly Jun 27, 2017
464634f
- fixed FilterInfo.js fail when comparing sparsed array(rows).
loitly Jun 29, 2017
313a4f5
- remove old plotly.js file
loitly Jun 29, 2017
99d2125
Merge branch 'dev' into DM-10830_optimize_plotly_chart
loitly Jun 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/firefly/html/demo/plotly-concepts.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<link rel=”apple-touch-startup-image” href=”images/fftools-ipad_splash_768x1004.png”>
<title>Plotly Concept</title>

<script type="text/javascript" src="../plotly-1.27.1.min.js"></script>
<script type="text/javascript" src="../plotly-1.28.2.min.js"></script>
</head>

<body style="margin: 0; background-color: rgb(200,200,200)">
Expand Down
78 changes: 78 additions & 0 deletions src/firefly/html/plotly-1.28.2.min.js
View file
Open in desktop

Large diffs are not rendered by default.

34 changes: 29 additions & 5 deletions src/firefly/js/charts/ChartUtil.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* Utilities related to charts
* Created by tatianag on 3/17/16.
*/
import {get, uniqueId, isUndefined, omitBy, zip, isEmpty, range, set, isObject, isPlainObject, pick, cloneDeep, merge} from 'lodash';
import {get, uniqueId, isUndefined, omitBy, zip, isEmpty, range, set, isObject, pick, cloneDeep, merge} from 'lodash';
import shallowequal from 'shallowequal';

import {flux} from '../Firefly.js';
Expand All @@ -32,13 +32,15 @@ export const HISTOGRAM = 'histogram';

export const SELECTED_COLOR = '#ffc800';
export const SELECTED_PROPS = {
name: '__SELECTED',
marker: {color: SELECTED_COLOR},
error_x: {color: SELECTED_COLOR},
error_y: {color: SELECTED_COLOR}
};

export const HIGHLIGHTED_COLOR = '#ffa500';
export const HIGHLIGHTED_PROPS = {
name: '__HIGHLIGHTED',
marker: {color: HIGHLIGHTED_COLOR, line: {width: 2, color: '#ffa500'}}, // increase highlight marker with line border
error_x: {color: HIGHLIGHTED_COLOR},
error_y: {color: HIGHLIGHTED_COLOR}
Expand Down Expand Up @@ -278,7 +280,7 @@ export function getOptionsUI(chartId) {
const {data, fireflyData, activeTrace=0} = getChartData(chartId);
const type = get(data, [activeTrace, 'type'], 'scatter');
const dataType = get(fireflyData, [activeTrace, 'dataType'], '');
if (type === 'scatter') {
if (type.includes('scatter')) {
return ScatterOptions;
} else if (type === 'histogram') {
return HistogramOptions;
Expand All @@ -292,7 +294,7 @@ export function getOptionsUI(chartId) {
export function getToolbarUI(chartId, activeTrace=0) {
const {data, fireflyData} = getChartData(chartId);
const type = get(data, [activeTrace, 'type'], '');
if (type === 'scatter') {
if (type.includes('scatter')) {
return ScatterToolbar;
} else {
return BasicToolbar;
Expand Down Expand Up @@ -327,7 +329,7 @@ export function newTraceFrom(data, selIndexes, newTraceProps) {
});
}
deepReplace(sdata);
const flatprops = flattenObject(newTraceProps, isPlainObject);
const flatprops = flattenObject(newTraceProps);
Object.entries(flatprops).forEach(([k,v]) => set(sdata, k, v));
return sdata;
}
Expand Down Expand Up @@ -474,7 +476,7 @@ function makeTableSources(chartId, data=[], fireflyData=[]) {
const currentData = (data.length < fireflyData.length) ? fireflyData : data;

return currentData.map((d, traceNum) => {
const ds = data[traceNum] ? convertToDS(flattenObject(data[traceNum], isPlainObject)) : {}; //avoid flattening arrays
const ds = data[traceNum] ? convertToDS(flattenObject(data[traceNum])) : {}; //avoid flattening arrays
if (!ds.tbl_id) {
// table id can be a part of fireflyData
const tbl_id = get(fireflyData, `${traceNum}.tbl_id`);
Expand Down Expand Up @@ -548,6 +550,28 @@ export function applyDefaults(chartData={}) {
chartData.layout = merge(defaultLayout, chartData.layout);
}

// color-blind friendly colors
const TRACE_COLORS = [ '#333333', '#ff3333', '#00ccff','#336600',
'#9900cc', '#ff9933', '#009999', '#66ff33', '#cc9999',
'#333333', '#b22424', '#008fb2', '#244700',
'#6b008f', '#b26b24', '#006b6b', '#47b224', '8F6B6B'];
export function getNewTraceDefaults(type='', traceNum=0) {
if (type.includes(SCATTER)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, when the type is not SCATTER this function returns undefined, which is causing the type error, which I mentioned before (when we are trying to use the result as an array).

return {
[`data.${traceNum}.type`] : type, //make sure trace type is set
[`data.${traceNum}.marker.color`]: TRACE_COLORS[traceNum] || undefined,
[`data.${traceNum}.marker.line`]: 'none',
[`data.${traceNum}.showlegend`]: true,
['layout.xaxis.range'] : undefined, //clear out fixed range
['layout.yaxis.range'] : undefined, //clear out fixed range
};
} else {
return {
[`data.${traceNum}.marker.color`]: TRACE_COLORS[traceNum] || undefined,
[`data.${traceNum}.showlegend`]: true
};
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that the defaults behave differently for scatter and scattergl.
For example yaxis: {ticklen: 5 } shows the tick, but it overlaps with the tick label for scattergl. It looks like the middle of the tick label rather then the edge is positioned next to the tick. Looks like a Plotly bug. The workaround is not to show the axis line or ticks, and show the grid instead.

Also, in scattergl, the spikes (vertical and horizontal droplines meeting at the point) are shown by default, while they are not shown by default in scatter plot. For crowded charts these lines are convenient, but since the spikes do not extend all the way, the direction is wrong when axes are on the opposite sides. None of the spike attributes listed in the documentation take effect for scattergl at the moment. mirror axis attribute does not work either. So the workaround would be to remove the options for opposite axis location. (As well as for reverse.)




Expand Down
52 changes: 33 additions & 19 deletions src/firefly/js/charts/ChartsCntlr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {cloneDeep, has, get, isEmpty, isString, isUndefined, omit, omitBy, set}
import shallowequal from 'shallowequal';

import {flux} from '../Firefly.js';
import {updateSet, updateMerge, updateObject} from '../util/WebUtil.js';
import {updateSet, updateMerge, updateObject, toBoolean} from '../util/WebUtil.js';
import {getTblById} from '../tables/TableUtil.js';
import * as TablesCntlr from '../tables/TablesCntlr.js';
import {logError} from '../util/WebUtil.js';
Expand All @@ -20,6 +20,8 @@ export const CHART_SPACE_PATH = 'charts';
export const UI_PREFIX = `${CHART_SPACE_PATH}.ui`;
export const DATA_PREFIX = `${CHART_SPACE_PATH}.data`;

export const useScatterGL = toBoolean(sessionStorage.getItem('scatterGL')); // defaults to false
export const useChartRedraw = toBoolean(sessionStorage.getItem('chartRedraw')); // defaults to false

/*---------------------------- ACTIONS -----------------------------*/
export const CHART_ADD = `${DATA_PREFIX}/chartAdd`;
Expand Down Expand Up @@ -145,12 +147,13 @@ export function dispatchChartUpdate({chartId, changes, dispatcher=flux.process})
* @param {object} p parameter object
* @param {string} p.chartId required.
* @param {number} p.highlighted index of the current data array
* @param {number} [p.activeTrace]
* @param {number} [p.traceNum] - highlighted trace number
* @param {number} [p.traceName] - highlighted trace name
* @param {boolean} [p.chartTrigger] - action is triggered by chart
* @param {function} [p.dispatcher]
*/
export function dispatchChartHighlighted({chartId, highlighted, activeTrace, chartTrigger, dispatcher=flux.process}) {
dispatcher({type: CHART_HIGHLIGHT, payload: {chartId, highlighted, activeTrace, chartTrigger}});
export function dispatchChartHighlighted({chartId, highlighted, traceNum, traceName, chartTrigger, dispatcher=flux.process}) {
dispatcher({type: CHART_HIGHLIGHT, payload: {chartId, highlighted, traceNum, traceName, chartTrigger}});
}

/**
Expand Down Expand Up @@ -341,32 +344,35 @@ function chartHighlight(action) {
return (dispatch) => {
const {chartId, highlighted=0, chartTrigger=false} = action.payload;
// TODO: activeTrace is not implemented. switch to trace.. then highlight(?)
const {data, tablesources, activeTrace:activeDataTrace=0} = getChartData(chartId);
const {activeTrace=activeDataTrace} = action.payload; // activeTrace can be selected or highlighted trace of the data trace
const ttype = get(data, [activeTrace, 'type'], 'scatter');
const {data, tablesources, activeTrace:activeDataTrace=0, selected} = getChartData(chartId);
const {traceNum=activeDataTrace, traceName} = action.payload; // highlighted trace can be selected or highlighted trace of the data trace
const ttype = get(data, [traceNum, 'type'], 'scatter');

if (!isEmpty(tablesources) && ttype === 'scatter') {
if (!isEmpty(tablesources) && ttype.includes('scatter')) {
// activeTrace is different from activeDataTrace if a selected point highlighted, for example
const {tbl_id} = tablesources[activeDataTrace] || {};
if (!tbl_id) return;
// avoid updating chart twice
// update only as a response to table highlight change
if (!chartTrigger) {
const hlTrace = newTraceFrom(data[activeTrace], [highlighted], HIGHLIGHTED_PROPS);
const hlTrace = newTraceFrom(data[traceNum], [highlighted], HIGHLIGHTED_PROPS);
dispatchChartUpdate({chartId, changes: {highlighted: hlTrace}});
}
let traceData = data[activeTrace];

if (activeTrace !== activeDataTrace) {
// workaround for highlighting selected point - we do not store selected trace data, but plotly does
const chartDivAll = document.querySelectorAll(`#${chartId}`);
if (chartId && chartDivAll && chartDivAll.length > 0) {
const chartDiv = chartDivAll[chartDivAll.length - 1];
traceData = get(chartDiv, `data.${activeTrace}`) || chartId;
let traceData = data[traceNum];

if (traceNum !== activeDataTrace) {
if (traceName === SELECTED_PROPS.name) {
// highlighting selected point
traceData = selected;
} else if (traceName === HIGHLIGHTED_PROPS.name) {
// no need to highlight highlighted
return;
}
}
const highlightedRowIdx = getRowIdx(traceData, highlighted);
TablesCntlr.dispatchTableHighlight(tbl_id, highlightedRowIdx);
if (traceData) {
const highlightedRowIdx = getRowIdx(traceData, highlighted);
TablesCntlr.dispatchTableHighlight(tbl_id, highlightedRowIdx);
}
}
};
}
Expand Down Expand Up @@ -613,6 +619,11 @@ export function reducer(state={ui:{}, data:{}}, action={}) {
// TablesCntlr.TABLE_REMOVE, TablesCntlr.TABLE_SELECT];


function changeToScatterGL(chartData) {
get(chartData, 'data', []).forEach((d) => d.type === 'scatter' && (d.type = 'scattergl')); // use scattergl instead of scatter
['selected', 'highlighted'].map((k) => get(chartData, k, {})).forEach((d) => d.type === 'scatter' && (d.type = 'scattergl'));
}

/**
* @param state - ui part of chart state
* @param action - action
Expand All @@ -629,6 +640,7 @@ function reduceData(state={}, action={}) {
if (chartType==='plot.ly') {
rest['_original'] = cloneDeep(action.payload);
applyDefaults(rest);
useScatterGL && changeToScatterGL(rest);
}
state = updateSet(state, chartId,
omitBy({
Expand All @@ -644,6 +656,8 @@ function reduceData(state={}, action={}) {
const {chartId, changes} = action.payload;
var chartData = getChartData(chartId) || {};
chartData = updateObject(chartData, changes);
useScatterGL && changeToScatterGL(chartData);

return updateSet(state, chartId, chartData);
}
case (CHART_REMOVE) :
Expand Down
2 changes: 1 addition & 1 deletion src/firefly/js/charts/PlotlyConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export default Plotly;



const PLOTLY_SCRIPT= 'plotly-1.27.1.min.js';
const PLOTLY_SCRIPT= 'plotly-1.28.2.min.js';
const LOAD_ERR_MSG= 'Load Failed: could not load Plotly';

function initPlotLyRetriever(loadNow) {
Expand Down
17 changes: 11 additions & 6 deletions src/firefly/js/charts/ui/PlotlyChartArea.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,16 @@ export class PlotlyChartArea extends PureComponent {
render() {
const {widthPx, heightPx} = this.props;
const {data=[], highlighted, selected, layout={}, activeTrace=0} = this.state;
const doingResize= (layout && (layout.width!==widthPx || layout.height!==heightPx));
let doingResize = false;
if (widthPx !== this.widthPx || heightPx !== this.heightPx) {
this.widthPx = widthPx;
this.heightPx = heightPx;
doingResize = true;
}
const showlegend = data.length > 1;
let pdata = data;
// TODO: change highlight or selected without forcing new plot
if (!data[activeTrace] || get(data[activeTrace], 'type') === 'scatter') {
// highlight makes sence only for scatter at the moment
let pdata = [].concat(data);
if (!data[activeTrace] || get(data[activeTrace], 'type', '').includes('scatter')) {
// highlight makes sense only for scatter at the moment
pdata = selected ? pdata.concat([selected]) : pdata;
pdata = highlighted ? pdata.concat([highlighted]) : pdata;
}
Expand All @@ -86,7 +90,8 @@ function onClick(chartId) {
// we should have active trace, its related selected, and its highlight traces on top
const curveNumber = get(evData.points, `${0}.curveNumber`);
const highlighted = get(evData.points, `${0}.pointNumber`);
dispatchChartHighlighted({chartId, activeTrace: curveNumber, highlighted, chartTrigger: true});
const curveName = get(evData.points, `${0}.data.name`);
dispatchChartHighlighted({chartId, traceNum: curveNumber, traceName: curveName, highlighted, chartTrigger: true});
};
}

Expand Down
5 changes: 3 additions & 2 deletions src/firefly/js/charts/ui/PlotlyToolbar.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,9 @@ function SelectBtn({style={}, chartId, dragmode}) {
function ResetZoomBtn({style={}, chartId}) {
const {_original} = getChartData(chartId) || {};
const doClick = () => {
const changes = ['xaxis','yaxis','zaxis'].reduce((pv, axis) => {
pv[`layout.${axis}.autorange`] = get(_original, `layout.${axis}.autorange`);
// TODO: this only handles chart with 2 axes
const changes = ['xaxis','yaxis'].reduce((pv, axis) => {
pv[`layout.${axis}.autorange`] = get(_original, `layout.${axis}.autorange`, true);
pv[`layout.${axis}.range`] = get(_original, `layout.${axis}.range`);
return pv;
}, {});
Expand Down
Loading