From 011eb33b067412ffb6362237c9f6dc7256476bcd Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Thu, 11 Jul 2024 17:16:15 -0400 Subject: [PATCH] feat: Add option to disable WebGL rendering (#2134) - Advanced Settings section in the Settings menu, with an option to disable WebGL - Read from the server config for default values - web.webgl: default setting for WebGL being enabled or not - web.webgl.editable: disallow the user from enabling WebGL - Contextual Help item to add more info - Tested by adding the following to the deephaven.prop config: ``` web.webgl=false web.webgl.editable=false client.configuration.list=java.version,deephaven.version,barrage.version,http.session.durationMs,file.separator,web.storage.layout.directory,web.storage.notebook.directory,web.webgl,web.webgl.editable ``` --------- Co-authored-by: Don --- .../src/storage/LocalWorkspaceStorage.ts | 10 +++ packages/chart/src/Chart.tsx | 30 +++++++-- packages/chart/src/ChartModel.ts | 17 +++++ packages/chart/src/ChartUtils.ts | 21 ++++-- packages/chart/src/FigureChartModel.ts | 12 +++- .../src/settings/AdvancedSectionContent.tsx | 66 +++++++++++++++++++ .../code-studio/src/settings/SettingsMenu.tsx | 29 +++++++- .../theme-dark/theme-dark-components.css | 3 + .../theme-light/theme-light-components.css | 3 + .../theme-spectrum-overrides.css | 5 ++ packages/dashboard/src/redux/selectors.ts | 15 +++++ packages/jsapi-utils/src/Settings.ts | 4 +- packages/redux/src/selectors.ts | 8 +++ packages/redux/src/store.ts | 6 +- 14 files changed, 212 insertions(+), 17 deletions(-) create mode 100644 packages/code-studio/src/settings/AdvancedSectionContent.tsx diff --git a/packages/app-utils/src/storage/LocalWorkspaceStorage.ts b/packages/app-utils/src/storage/LocalWorkspaceStorage.ts index 5180bb874..63d78a5f8 100644 --- a/packages/app-utils/src/storage/LocalWorkspaceStorage.ts +++ b/packages/app-utils/src/storage/LocalWorkspaceStorage.ts @@ -59,6 +59,8 @@ export class LocalWorkspaceStorage implements WorkspaceStorage { defaultNotebookSettings: { isMinimapEnabled: false, }, + webgl: true, + webglEditable: true, }; const serverSettings = { defaultDateTimeFormat: serverConfigValues?.get('dateTimeFormat'), @@ -109,6 +111,14 @@ export class LocalWorkspaceStorage implements WorkspaceStorage { ) as boolean, } : undefined, + webgl: LocalWorkspaceStorage.getBooleanServerConfig( + serverConfigValues, + 'web.webgl' + ), + webglEditable: LocalWorkspaceStorage.getBooleanServerConfig( + serverConfigValues, + 'web.webgl.editable' + ), }; const keys = Object.keys(serverSettings) as Array; diff --git a/packages/chart/src/Chart.tsx b/packages/chart/src/Chart.tsx index 3a355967f..88c983784 100644 --- a/packages/chart/src/Chart.tsx +++ b/packages/chart/src/Chart.tsx @@ -41,16 +41,20 @@ import useChartTheme from './useChartTheme'; const log = Log.module('Chart'); -type FormatterSettings = ColumnFormatSettings & +type ChartSettings = ColumnFormatSettings & DateTimeFormatSettings & { decimalFormatOptions?: DecimalColumnFormatterOptions; integerFormatOptions?: IntegerColumnFormatterOptions; + webgl?: boolean; }; interface ChartProps { model: ChartModel; theme: ChartTheme; - settings: FormatterSettings; + + /** User settings that are relevant to the chart, e.g. formatter settings */ + settings: ChartSettings; + isActive: boolean; Plotly: typeof Plotly; containerRef?: React.RefObject; @@ -58,6 +62,8 @@ interface ChartProps { onReconnect: () => void; onUpdate: (obj: { isLoading: boolean }) => void; onError: (error: Error) => void; + + /** Called when the settings for the ChartModel are changed */ onSettingsChanged: (settings: Partial) => void; } @@ -91,6 +97,7 @@ class Chart extends Component { showTimeZone: false, showTSeparator: true, formatter: [], + webgl: true, }, Plotly, onDisconnect: (): void => undefined, @@ -195,7 +202,7 @@ class Chart extends Component { componentDidUpdate(prevProps: ChartProps): void { const { isActive, model, settings, theme } = this.props; - this.updateFormatterSettings(settings as FormatterSettings); + this.updateFormatterSettings(settings); if (model !== prevProps.model) { this.unsubscribe(prevProps.model); @@ -239,6 +246,8 @@ class Chart extends Component { integerFormatOptions: IntegerColumnFormatterOptions; + webgl?: boolean; + rect?: DOMRect; ranges?: unknown; @@ -612,10 +621,10 @@ class Chart extends Component { initFormatter(): void { const { settings } = this.props; - this.updateFormatterSettings(settings as FormatterSettings); + this.updateFormatterSettings(settings); } - updateFormatterSettings(settings: FormatterSettings): void { + updateFormatterSettings(settings: ChartSettings): void { const columnFormats = FormatterUtils.getColumnFormats(settings); const dateTimeFormatterOptions = FormatterUtils.getDateTimeFormatterOptions(settings); @@ -633,6 +642,11 @@ class Chart extends Component { this.integerFormatOptions = integerFormatOptions; this.updateFormatter(); } + + if (this.webgl !== settings.webgl) { + this.webgl = settings.webgl; + this.updateRenderOptions(); + } } updateFormatter(): void { @@ -647,6 +661,12 @@ class Chart extends Component { model.setFormatter(formatter); } + updateRenderOptions(): void { + const { model } = this.props; + const renderOptions = { webgl: this.webgl }; + model.setRenderOptions(renderOptions); + } + updateDimensions(): void { const rect = this.getPlotRect(); const { Plotly: PlotlyProp } = this.props; diff --git a/packages/chart/src/ChartModel.ts b/packages/chart/src/ChartModel.ts index 462a1adcb..0cf03815a 100644 --- a/packages/chart/src/ChartModel.ts +++ b/packages/chart/src/ChartModel.ts @@ -7,6 +7,12 @@ import type { Layout, Data } from 'plotly.js'; import { FilterColumnMap, FilterMap } from './ChartUtils'; export type ChartEvent = CustomEvent; + +export type RenderOptions = { + /** Allow WebGL as an option. Defaults to `true`, explicitly set to `false` to disable. */ + webgl?: boolean; +}; + /** * Model for a Chart * All of these methods should return very quickly. @@ -41,8 +47,11 @@ class ChartModel { listeners: ((event: ChartEvent) => void)[]; + /** Formatter settings for the chart, such as how to format dates and numbers */ formatter?: Formatter; + renderOptions?: RenderOptions; + rect?: DOMRect; isDownsamplingDisabled: boolean; @@ -86,6 +95,14 @@ class ChartModel { this.formatter = formatter; } + /** + * Set additional options for rendering the chart + * @param renderOptions Options for rendering the chart + */ + setRenderOptions(renderOptions: RenderOptions): void { + this.renderOptions = renderOptions; + } + /** * Disable downsampling * @param isDownsamplingDisabled True if downsampling should be disabled diff --git a/packages/chart/src/ChartUtils.ts b/packages/chart/src/ChartUtils.ts index 6b102db3e..ce66ae77e 100644 --- a/packages/chart/src/ChartUtils.ts +++ b/packages/chart/src/ChartUtils.ts @@ -681,17 +681,22 @@ class ChartUtils { * Converts the Iris plot style into a plotly chart type * @param plotStyle The plotStyle to use, see dh.plot.SeriesPlotStyle * @param isBusinessTime If the plot is using business time for an axis + * @param allowWebGL If WebGL is allowedd */ getPlotlyChartType( plotStyle: DhType.plot.SeriesPlotStyle, - isBusinessTime: boolean + isBusinessTime: boolean, + allowWebGL = true ): PlotType | undefined { const { dh } = this; switch (plotStyle) { case dh.plot.SeriesPlotStyle.SCATTER: case dh.plot.SeriesPlotStyle.LINE: - // scattergl mode is more performant, but doesn't support the rangebreaks we need for businessTime calendars - return !isBusinessTime && IS_WEBGL_SUPPORTED ? 'scattergl' : 'scatter'; + // scattergl mode is more performant (usually), but doesn't support the rangebreaks we need for businessTime calendars + // In some cases, WebGL is less performant (like in virtual desktop environments), so we also allow the option of the user explicitly disabling it even if it's supported + return !isBusinessTime && IS_WEBGL_SUPPORTED && allowWebGL + ? 'scattergl' + : 'scatter'; case dh.plot.SeriesPlotStyle.BAR: case dh.plot.SeriesPlotStyle.STACKED_BAR: return 'bar'; @@ -871,7 +876,8 @@ class ChartUtils { series: DhType.plot.Series, axisTypeMap: AxisTypeMap, seriesVisibility: boolean | 'legendonly', - showLegend: boolean | null = null + showLegend: boolean | null = null, + allowWebGL = true ): Partial { const { name, @@ -888,7 +894,7 @@ class ChartUtils { const isBusinessTime = sources.some( source => source.axis?.businessCalendar ); - const type = this.getChartType(plotStyle, isBusinessTime); + const type = this.getChartType(plotStyle, isBusinessTime, allowWebGL); const mode = this.getPlotlyChartMode( plotStyle, isLinesVisible ?? undefined, @@ -1019,7 +1025,8 @@ class ChartUtils { getChartType( plotStyle: DhType.plot.SeriesPlotStyle, - isBusinessTime: boolean + isBusinessTime: boolean, + allowWebGL = true ): PlotType | undefined { const { dh } = this; switch (plotStyle) { @@ -1028,7 +1035,7 @@ class ChartUtils { // plot.ly to calculate the bins and sum values, just convert it to a bar chart return 'bar'; default: - return this.getPlotlyChartType(plotStyle, isBusinessTime); + return this.getPlotlyChartType(plotStyle, isBusinessTime, allowWebGL); } } diff --git a/packages/chart/src/FigureChartModel.ts b/packages/chart/src/FigureChartModel.ts index 50945a7aa..a96cf03de 100644 --- a/packages/chart/src/FigureChartModel.ts +++ b/packages/chart/src/FigureChartModel.ts @@ -17,7 +17,7 @@ import type { DateTimeColumnFormatter, Formatter, } from '@deephaven/jsapi-utils'; -import ChartModel, { ChartEvent } from './ChartModel'; +import ChartModel, { ChartEvent, RenderOptions } from './ChartModel'; import ChartUtils, { AxisTypeMap, ChartModelSettings, @@ -219,7 +219,8 @@ class FigureChartModel extends ChartModel { series, axisTypeMap, ChartUtils.getSeriesVisibility(series.name, this.settings), - showLegend + showLegend, + this.renderOptions?.webgl ?? true ); this.seriesDataMap.set(series.name, seriesData); @@ -535,6 +536,13 @@ class FigureChartModel extends ChartModel { this.resubscribe(); } + setRenderOptions(renderOptions: RenderOptions): void { + super.setRenderOptions(renderOptions); + + // Reset all the series to re-render them with the correct rendering options + this.initAllSeries(); + } + setDownsamplingDisabled(isDownsamplingDisabled: boolean): void { super.setDownsamplingDisabled(isDownsamplingDisabled); diff --git a/packages/code-studio/src/settings/AdvancedSectionContent.tsx b/packages/code-studio/src/settings/AdvancedSectionContent.tsx new file mode 100644 index 000000000..6750b0e62 --- /dev/null +++ b/packages/code-studio/src/settings/AdvancedSectionContent.tsx @@ -0,0 +1,66 @@ +import React, { useCallback } from 'react'; +import { useDispatch } from 'react-redux'; +import { + Content, + ContextualHelp, + Heading, + Switch, + Text, +} from '@deephaven/components'; +import { getWebGL, getWebGLEditable, updateSettings } from '@deephaven/redux'; +import { useAppSelector } from '@deephaven/dashboard'; + +function AdvancedSectionContent(): JSX.Element { + const dispatch = useDispatch(); + const webgl = useAppSelector(getWebGL); + const webglEditable = useAppSelector(getWebGLEditable); + + const handleWebglChange = useCallback( + newValue => { + dispatch(updateSettings({ webgl: newValue })); + }, + [dispatch] + ); + + const helpText = webglEditable ? ( + + WebGL in most cases has significant performance improvements, particularly + when plotting large datasets. However, in some environments (such as + remote desktop environments), WebGL may not use hardware acceleration and + have degraded performance. If you are experiencing issues with WebGL, you + can disable it here. + + ) : ( + + WebGL is disabled by your system administrator. Contact your system + administrator to enable. + + ); + + return ( + <> +
+ Advanced settings here. Be careful! +
+ +
+ + Enable WebGL + + + Enable WebGL + + {helpText} + + +
+ + ); +} + +export default AdvancedSectionContent; diff --git a/packages/code-studio/src/settings/SettingsMenu.tsx b/packages/code-studio/src/settings/SettingsMenu.tsx index a57f4af14..93bf36a8f 100644 --- a/packages/code-studio/src/settings/SettingsMenu.tsx +++ b/packages/code-studio/src/settings/SettingsMenu.tsx @@ -9,6 +9,7 @@ import { vsPaintcan, dhUserIncognito, dhUser, + vsTools, } from '@deephaven/icons'; import { Button, @@ -38,6 +39,7 @@ import { getFormattedPluginInfo, getFormattedVersionInfo, } from './SettingsUtils'; +import AdvancedSectionContent from './AdvancedSectionContent'; interface SettingsMenuProps { serverConfigValues: ServerConfigValues; @@ -68,6 +70,8 @@ export class SettingsMenu extends Component< static THEME_SECTION_KEY = 'SettingsMenu.theme'; + static ADVANCED_SECTION_KEY = 'SettingsMenu.advanced'; + static focusFirstInputInContainer(container: HTMLDivElement | null): void { const input = container?.querySelector('input, select, textarea'); if (input) { @@ -289,7 +293,11 @@ export class SettingsMenu extends Component< )} title={ <> - {' '} + Keyboard Shortcuts } @@ -297,6 +305,25 @@ export class SettingsMenu extends Component< > + + + Advanced + + } + onToggle={this.handleSectionToggle} + > + +
diff --git a/packages/components/src/theme/theme-dark/theme-dark-components.css b/packages/components/src/theme/theme-dark/theme-dark-components.css index 02c09402e..4847fbf8a 100644 --- a/packages/components/src/theme/theme-dark/theme-dark-components.css +++ b/packages/components/src/theme/theme-dark/theme-dark-components.css @@ -166,4 +166,7 @@ var(--dh-color-visual-gray) 5%, transparent ); + + /* Switch */ + --dh-toggle-switch-bg: var(--dh-color-gray-500); } diff --git a/packages/components/src/theme/theme-light/theme-light-components.css b/packages/components/src/theme/theme-light/theme-light-components.css index 30ab0ef0b..4e9ab3cca 100644 --- a/packages/components/src/theme/theme-light/theme-light-components.css +++ b/packages/components/src/theme/theme-light/theme-light-components.css @@ -162,4 +162,7 @@ var(--dh-color-visual-gray) 5%, transparent ); + + /* Switch */ + --dh-toggle-switch-bg: var(--dh-color-gray-400); } diff --git a/packages/components/src/theme/theme-spectrum/theme-spectrum-overrides.css b/packages/components/src/theme/theme-spectrum/theme-spectrum-overrides.css index f0c064de7..4a46015d1 100644 --- a/packages/components/src/theme/theme-spectrum/theme-spectrum-overrides.css +++ b/packages/components/src/theme/theme-spectrum/theme-spectrum-overrides.css @@ -111,3 +111,8 @@ label[class*='spectrum-Radio'] { --dh-color-negative-key-focus-bg ); } + +span[class*='spectrum-ToggleSwitch-switch'] { + /* increase contrast of switch in off position */ + background: var(--dh-toggle-switch-bg); +} diff --git a/packages/dashboard/src/redux/selectors.ts b/packages/dashboard/src/redux/selectors.ts index acdcdfd3f..a57c16087 100644 --- a/packages/dashboard/src/redux/selectors.ts +++ b/packages/dashboard/src/redux/selectors.ts @@ -1,9 +1,17 @@ import { + AppStore, DashboardData, PluginData, PluginDataMap, + RootDispatch, RootState, } from '@deephaven/redux'; +import { + TypedUseSelectorHook, + useDispatch, + useSelector, + useStore, +} from 'react-redux'; import { ClosedPanels, OpenedPanelMap } from '../PanelManager'; const EMPTY_MAP = new Map(); @@ -14,6 +22,13 @@ const EMPTY_ARRAY = Object.freeze([]); type Selector = (state: RootState) => R; +// https://react-redux.js.org/using-react-redux/usage-with-typescript#define-typed-hooks +// Defined in @deephaven/dashboard, as that's the most common package with React and @deephaven/redux as dependencies +// We could have another package specifically for this, but it's not really necessary. +export const useAppDispatch: () => RootDispatch = useDispatch; +export const useAppSelector: TypedUseSelectorHook = useSelector; +export const useAppStore: () => AppStore = useStore; + /** * Retrieve the data for all dashboards * @param store The redux store diff --git a/packages/jsapi-utils/src/Settings.ts b/packages/jsapi-utils/src/Settings.ts index de64bce9c..264615892 100644 --- a/packages/jsapi-utils/src/Settings.ts +++ b/packages/jsapi-utils/src/Settings.ts @@ -26,6 +26,8 @@ export interface NumberFormatSettings { export interface Settings extends ColumnFormatSettings, DateTimeFormatSettings, - NumberFormatSettings {} + NumberFormatSettings { + webgl?: boolean; +} export default Settings; diff --git a/packages/redux/src/selectors.ts b/packages/redux/src/selectors.ts index 3326d9584..f4557a660 100644 --- a/packages/redux/src/selectors.ts +++ b/packages/redux/src/selectors.ts @@ -138,6 +138,14 @@ export const getShortcutOverrides = ( store: State ): Settings['shortcutOverrides'] => getSettings(store).shortcutOverrides; +export const getWebGL = ( + store: State +): Settings['webgl'] => getSettings(store).webgl; + +export const getWebGLEditable = ( + store: State +): Settings['webglEditable'] => getSettings(store).webglEditable; + export const getDefaultNotebookSettings = ( store: State ): Settings['defaultNotebookSettings'] => diff --git a/packages/redux/src/store.ts b/packages/redux/src/store.ts index fc5d11eb0..74455c09f 100644 --- a/packages/redux/src/store.ts +++ b/packages/redux/src/store.ts @@ -57,6 +57,8 @@ export interface WorkspaceSettings { defaultNotebookSettings: { isMinimapEnabled?: boolean; }; + webgl: boolean; + webglEditable: boolean; } export interface WorkspaceData { @@ -138,6 +140,8 @@ reducerRegistry.setListener(newReducers => { store.replaceReducer(combineReducers(newReducers)); }); -export default store; +export type AppStore = typeof store; export type RootDispatch = typeof store.dispatch; + +export default store;