Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
jnumainville committed Oct 15, 2024
1 parent 4e1104f commit f695af0
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 20 deletions.
60 changes: 60 additions & 0 deletions plugins/plotly-express/src/js/src/PlotlyExpressChartModel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,4 +262,64 @@ describe('PlotlyExpressChartModel', () => {
new CustomEvent(ChartModel.EVENT_DOWNSAMPLEFAILED)
);
});

it('should swap replaceable WebGL traces without blocker events if WebGL is disabled or reenabled', async () => {
const mockWidget = createMockWidget([SMALL_TABLE], 'scattergl');
const chartModel = new PlotlyExpressChartModel(
mockDh,
mockWidget,
jest.fn()
);

const mockSubscribe = jest.fn();
await chartModel.subscribe(mockSubscribe);
chartModel.renderOptions = { webgl: true };
expect(chartModel.plotlyData[0].type).toBe('scattergl');
chartModel.renderOptions = { webgl: false };
expect(chartModel.plotlyData[0].type).toBe('scatter');
chartModel.renderOptions = { webgl: true };
expect(chartModel.plotlyData[0].type).toBe('scattergl');
await new Promise(process.nextTick); // Subscribe is async

// No events should be emitted since the trace is replaceable
expect(mockSubscribe).toHaveBeenCalledTimes(0);
});

it('should emit blocker events only if unreplaceable WebGL traces are present and WebGL is disabled, then blocker clear events when reenabled', async () => {
const mockWidget = createMockWidget([SMALL_TABLE], 'scatter3d');
const chartModel = new PlotlyExpressChartModel(
mockDh,
mockWidget,
jest.fn()
);

const mockSubscribe = jest.fn();
await chartModel.subscribe(mockSubscribe);
chartModel.renderOptions = { webgl: true };
await new Promise(process.nextTick); // Subscribe is async
// no calls yet because WebGL is enabled
expect(mockSubscribe).toHaveBeenCalledTimes(0);
chartModel.renderOptions = { webgl: false };
await new Promise(process.nextTick);
// blocking event should be emitted
expect(mockSubscribe).toHaveBeenCalledTimes(1);
expect(mockSubscribe).toHaveBeenLastCalledWith(
new CustomEvent(ChartModel.EVENT_BLOCKER)
);
chartModel.renderOptions = { webgl: true };
await new Promise(process.nextTick);
// blocking clear event should be emitted
expect(mockSubscribe).toHaveBeenCalledTimes(2);
expect(mockSubscribe).toHaveBeenLastCalledWith(
new CustomEvent(ChartModel.EVENT_BLOCKER_CLEAR)
);
// if user had accepted the rendering, no EVENT_BLOCKER event should be emitted again
chartModel.fireBlockerClear();
chartModel.renderOptions = { webgl: false };
await new Promise(process.nextTick);
expect(mockSubscribe).toHaveBeenCalledTimes(3);
expect(mockSubscribe).toHaveBeenLastCalledWith(
new CustomEvent(ChartModel.EVENT_BLOCKER_CLEAR)
);
});
});
50 changes: 39 additions & 11 deletions plugins/plotly-express/src/js/src/PlotlyExpressChartModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import {
downsample,
getDataMappings,
getPathParts,
getWebGlTraceIndexes,
getReplaceableWebGlTraceIndexes,
getWidgetData,
isAutoAxis,
isLineSeries,
isLinearAxis,
removeColorsFromData,
setWebGlTraceType,
hasUnreplaceableWebGlTraces
hasUnreplaceableWebGlTraces,
} from './PlotlyExpressChartUtils';

const log = Log.module('@deephaven/js-plugin-plotly-express.ChartModel');
Expand Down Expand Up @@ -126,6 +126,11 @@ export class PlotlyExpressChartModel extends ChartModel {
*/
webGlTraceIndices: Set<number> = new Set();

/**
* The WebGl warning is only shown once per chart. When the user acknowledges the warning, it will not be shown again.
*/
hasAcknowledgedWebGlWarning = false;

override getData(): Partial<Data>[] {
const hydratedData = [...this.plotlyData];

Expand Down Expand Up @@ -218,24 +223,47 @@ export class PlotlyExpressChartModel extends ChartModel {
}

override setRenderOptions(renderOptions: RenderOptions): void {
this.handleWebGlAllowed(renderOptions.webgl, this.renderOptions?.webgl);
super.setRenderOptions(renderOptions);
this.handleWebGlAllowed(renderOptions.webgl);
}

handleWebGlAllowed(webgl: boolean | undefined): void {
/**
* Handle the WebGL option being set in the render options.
* If WebGL is enabled, traces have their original types as given.
* If WebGL is disabled, replace traces that require WebGL with non-WebGL traces if possible.
* Also, show a dismissible warning per-chart if there are WebGL traces that cannot be replaced.
* @param webgl The new WebGL value. True if WebGL is enabled.
* @param prevWebgl The previous WebGL value
*/
handleWebGlAllowed(
webgl: boolean | undefined,
prevWebgl: boolean | undefined
): void {
if (webgl != null) {
setWebGlTraceType(this.plotlyData, webgl, this.webGlTraceIndices);

if (hasUnreplaceableWebGlTraces(this.plotlyData) && !webgl) {
this.fireError([
'This chart is using WebGL, which is disabled, but this chart cannot render without it.',
// If WebGL is disabled and there are traces that require WebGL, show a warning that is dismissible on a per-chart basis
if (
hasUnreplaceableWebGlTraces(this.plotlyData) &&
!webgl &&
!this.hasAcknowledgedWebGlWarning
) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
this.fireBlocker([
'WebGL is disabled but this chart cannot render without it. Check the Advanced section in the settings to enable WebGL or click below to render with WebGL for this chart.',
]);
} else {
this.fireError([]);
} else if (webgl === true && prevWebgl === false) {
// clear the blocker but not the acknowledged flag in case WebGL is disabled again
super.fireBlockerClear();
}
}
}

override fireBlockerClear(): void {
super.fireBlockerClear();
this.hasAcknowledgedWebGlWarning = true;
}

updateLayout(data: PlotlyChartWidgetData): void {
const { figure } = data;
const { plotly } = figure;
Expand Down Expand Up @@ -275,8 +303,8 @@ export class PlotlyExpressChartModel extends ChartModel {
);
}

// Retrieve the indexes of traces that require WebGL to flip on demand
this.webGlTraceIndices = getWebGlTraceIndexes(this.plotlyData);
// Retrieve the indexes of traces that require WebGL so they can be replaced if WebGL is disabled
this.webGlTraceIndices = getReplaceableWebGlTraceIndexes(this.plotlyData);

this.handleWebGlAllowed(this.renderOptions?.webgl);

Expand Down
116 changes: 116 additions & 0 deletions plugins/plotly-express/src/js/src/PlotlyExpressChartUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import {
removeColorsFromData,
getDataMappings,
PlotlyChartWidgetData,
getReplaceableWebGlTraceIndexes,
hasUnreplaceableWebGlTraces,
setWebGlTraceType,
} from './PlotlyExpressChartUtils';

describe('getDataMappings', () => {
Expand Down Expand Up @@ -202,3 +205,116 @@ describe('areSameAxisRange', () => {
expect(areSameAxisRange(null, [0, 10])).toBe(false);
});
});

describe('getReplaceableWebGlTraceIndexes', () => {
it('should return the indexes of any trace with gl', () => {
expect(
getReplaceableWebGlTraceIndexes([
{
type: 'scattergl',
},
{
type: 'scatter',
},
{
type: 'scatter3d',
},
{
type: 'scattergl',
},
])
).toEqual([0, 3]);
});

it('should return an empty array if there are no traces with gl', () => {
expect(
getReplaceableWebGlTraceIndexes([
{
type: 'scatter',
},
{
type: 'scatter3d',
},
])
).toEqual([]);
});
});

describe('hasUnreplaceableWebGlTraces', () => {
it('should return true if there is a single unreplaceable trace', () => {
expect(
hasUnreplaceableWebGlTraces([
{
type: 'scattermapbox',
},
])
).toBe(true);
});

it('should return true if there are unreplaceable traces', () => {
expect(
hasUnreplaceableWebGlTraces([
{
type: 'scattergl',
},
{
type: 'scatter',
},
{
type: 'scatter3d',
},
{
type: 'scattergl',
},
{
type: 'scatter3d',
},
])
).toBe(true);
});

it('should return false if there are no unreplaceable traces', () => {
expect(
hasUnreplaceableWebGlTraces([
{
type: 'scatter',
},
{
type: 'scattergl',
},
])
).toBe(false);
});
});

describe('setWebGlTraceType', () => {
it('should set the trace type to gl if webgl is enabled', () => {
const data: Plotly.Data[] = [
{
type: 'scatter',
},
{
type: 'scatter',
},
];
const webGlTraceIndices = new Set([1]);
setWebGlTraceType(data, true, webGlTraceIndices);
expect(data[0].type).toBe('scattergl');
expect(data[1].type).toBe('scattergl');
});

it('should remove the gl from the trace type if webgl is disabled', () => {
const data: Plotly.Data[] = [
{
type: 'scatter',
},
{
type: 'scattergl',
},
];
const webGlTraceIndices = new Set([1]);
setWebGlTraceType(data, false, webGlTraceIndices);
expect(data[0].type).toBe('scatter');
expect(data[1].type).toBe('scatter');
});
});
26 changes: 17 additions & 9 deletions plugins/plotly-express/src/js/src/PlotlyExpressChartUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import type { dh as DhType } from '@deephaven/jsapi-types';

/**
* Traces that are at least partially powered by WebGL and have no SVG equivalent.
* These traces will not be able to render if WebGL is disabled and the setting is not editable.
* They will prompt the user to allow WebGL for this specific trace if the setting is editable.
*/
const UNREPLACEABLE_WEBGL_TRACE_TYPES = new Set([
'splom',
Expand Down Expand Up @@ -234,7 +232,13 @@ export function downsample(
);
}

export function getWebGlTraceIndexes(data: Data[]): Set<number> {
/**
* Get the indexes of the replaceable WebGL traces in the data
* A replaceable WebGL has a type that ends with 'gl' which indicates it has a SVG equivalent
* @param data The data to check
* @returns The indexes of the WebGL traces
*/
export function getReplaceableWebGlTraceIndexes(data: Data[]): Set<number> {
const webGlTraceIndexes = new Set<number>();
data.forEach((trace, index) => {
if (trace.type && trace.type.endsWith('gl')) {
Expand All @@ -244,19 +248,23 @@ export function getWebGlTraceIndexes(data: Data[]): Set<number> {
return webGlTraceIndexes;
}

/**
* Check if the data contains any traces that are at least partially powered by WebGL and have no SVG equivalent.
* @param data The data to check for WebGL traces
* @returns True if the data contains any unreplaceable WebGL traces
*/
export function hasUnreplaceableWebGlTraces(data: Data[]): boolean {
return data.some(
trace => trace.type && UNREPLACEABLE_WEBGL_TRACE_TYPES.has(trace.type)
);
}

/**
* Removes all possible WebGL from the plotly figure
* Drops 'gl' from any traces that have it, which will force them to use SVG
* Additionally, if traces are detected that use WebGL and have no SVG equivalent,
* that is also detected.
* @param data The plotly figure data to remove WebGL from
* @returns Object with a boolean indicating if unreplaceable WebGL traces are present and indexes of the replaced traces
* Set traces to use WebGL if WebGL is enabled and the trace was originally WebGL
* or swap out WebGL for SVG if WebGL is disabled and the trace was originally WebGL
* @param data The plotly figure data to update
* @param webgl True if WebGL is enabled
* @param webGlTraceIndices The indexes of the traces that are originally WebGL traces
*/
export function setWebGlTraceType(
data: Data[],
Expand Down

0 comments on commit f695af0

Please sign in to comment.