feat: Add option to disable WebGL rendering #2134

mofojed · 2024-07-08T20:00:26Z

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

Screenshot of when it's enabled/editable:

Screenshot of when it's disabled/editable (@dsmmcken I think we may need to change Switch styling up here, or the Settings menu background):

Screenshot of when it's disabled and not editable on the server:

- 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 - 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 ```

codecov · 2024-07-08T20:06:09Z

Codecov Report

Attention: Patch coverage is 22.50000% with 31 lines in your changes missing coverage. Please review.

Project coverage is 46.62%. Comparing base (989b8ec) to head (c477869).
Report is 6 commits behind head on main.

Files	Patch %	Lines
...ode-studio/src/settings/AdvancedSectionContent.tsx	0.00%	11 Missing ⚠️
packages/chart/src/Chart.tsx	0.00%	10 Missing ⚠️
packages/chart/src/ChartUtils.ts	44.44%	5 Missing ⚠️
packages/chart/src/FigureChartModel.ts	25.00%	3 Missing ⚠️
packages/chart/src/ChartModel.ts	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2134      +/-   ##
==========================================
+ Coverage   46.59%   46.62%   +0.02%     
==========================================
  Files         682      685       +3     
  Lines       38441    38489      +48     
  Branches     9578     9590      +12     
==========================================
+ Hits        17912    17945      +33     
- Misses      20519    20534      +15     
  Partials       10       10

Flag	Coverage Δ
unit	`46.62% <22.50%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dsmmcken

theme-spectrum-overrides.css

span[class*='spectrum-ToggleSwitch-switch'] {
  /* increase contrast of switch in off position */
  background: var(--dh-color-gray-500);
}

dsmmcken · 2024-07-09T16:50:15Z

Is there a follow on ticket for deephaven-express to check this setting?

mofojed · 2024-07-09T17:48:41Z

@dsmmcken followup ticket: deephaven/deephaven-plugins#612

- The Off position blended with the background too much. Add an override so it does not

packages/code-studio/src/settings/AdvancedSectionContent.tsx

bmingles · 2024-07-10T22:20:16Z

packages/components/src/theme/theme-spectrum/theme-spectrum-overrides.css

+
+span[class*='spectrum-ToggleSwitch-switch'] {
+  /* increase contrast of switch in off position */
+  background: var(--dh-color-gray-500);


Should we introduce a --dh-toggle-switch-bg: var(--dh-color-gray-500) to make this overridable?

I don't see why not? @dsmmcken

yes, and actually should double check it in light mode, as I didn't.

Should be added to theme-dark-components.css and theme-light-components.css with appropriate values, that use that variable here.

packages/redux/src/store.ts

bmingles · 2024-07-10T22:21:59Z

packages/chart/src/ChartUtils.ts

   */
  getPlotlyChartType(
    plotStyle: DhType.plot.SeriesPlotStyle,
-    isBusinessTime: boolean
+    isBusinessTime: boolean,
+    allowWebGL: boolean


Should this be mentioned as a BREAKING CHANGE?

I'll just default it to true to avoid making it a breaking change. I wanted to make sure I caught all the places where it was called from though.

bmingles · 2024-07-10T22:22:13Z

packages/chart/src/ChartUtils.ts

@@ -1019,7 +1025,8 @@ class ChartUtils {

  getChartType(
    plotStyle: DhType.plot.SeriesPlotStyle,
-    isBusinessTime: boolean
+    isBusinessTime: boolean,
+    allowWebGL: boolean


Should this be mentioned as a BREAKING CHANGE?

bmingles

Left a couple of suggestions / questions

- Use typed useSelector - Add css variable for toggle switch background

packages/components/src/theme/theme-dark/theme-dark-semantic.css

bmingles

1 issue with location of the css variable, but otherwise looks good.

bmingles

LGTM

packages/components/src/theme/theme-light/theme-light-components.css

…ts.css

bmingles

LGTM

mofojed requested a review from dsmmcken July 8, 2024 20:00

mofojed self-assigned this Jul 8, 2024

mofojed marked this pull request as ready for review July 8, 2024 20:00

dsmmcken requested changes Jul 9, 2024

View reviewed changes

mofojed mentioned this pull request Jul 9, 2024

Deephaven express should respect the disable webGL flag deephaven/deephaven-plugins#612

Closed

Add override to add contrast to Switch component when off

9059965

- The Off position blended with the background too much. Add an override so it does not

mofojed requested a review from dsmmcken July 9, 2024 17:51

dsmmcken previously approved these changes Jul 9, 2024

View reviewed changes

mofojed requested a review from bmingles July 10, 2024 21:28

bmingles reviewed Jul 10, 2024

View reviewed changes

packages/code-studio/src/settings/AdvancedSectionContent.tsx Outdated Show resolved Hide resolved

bmingles reviewed Jul 10, 2024

View reviewed changes

packages/redux/src/store.ts Show resolved Hide resolved

bmingles reviewed Jul 10, 2024

View reviewed changes

bmingles requested changes Jul 10, 2024

View reviewed changes

Address review comments

ba01275

- Use typed useSelector - Add css variable for toggle switch background

mofojed dismissed dsmmcken’s stale review via ba01275 July 11, 2024 13:06

bmingles reviewed Jul 11, 2024

View reviewed changes

packages/components/src/theme/theme-dark/theme-dark-semantic.css Outdated Show resolved Hide resolved

bmingles requested changes Jul 11, 2024

View reviewed changes

Move to theme-*-components.css instead

da98278

bmingles previously approved these changes Jul 11, 2024

View reviewed changes

dsmmcken reviewed Jul 11, 2024

View reviewed changes

packages/components/src/theme/theme-light/theme-light-components.css Outdated Show resolved Hide resolved

Update packages/components/src/theme/theme-light/theme-light-componen…

c477869

…ts.css

dsmmcken dismissed bmingles’s stale review via c477869 July 11, 2024 20:55

dsmmcken approved these changes Jul 11, 2024

View reviewed changes

dsmmcken requested a review from bmingles July 11, 2024 20:56

bmingles approved these changes Jul 11, 2024

View reviewed changes

mofojed enabled auto-merge (squash) July 11, 2024 21:00

mofojed merged commit 011eb33 into deephaven:main Jul 11, 2024
11 checks passed

mofojed deleted the disable-webgl branch July 11, 2024 21:16

github-actions bot locked and limited conversation to collaborators Jul 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add option to disable WebGL rendering #2134

feat: Add option to disable WebGL rendering #2134

mofojed commented Jul 8, 2024

codecov bot commented Jul 8, 2024 •

edited

Loading

dsmmcken left a comment

dsmmcken commented Jul 9, 2024

mofojed commented Jul 9, 2024

bmingles Jul 10, 2024

mofojed Jul 11, 2024

dsmmcken Jul 11, 2024

bmingles Jul 10, 2024

mofojed Jul 11, 2024

bmingles Jul 10, 2024

bmingles left a comment

bmingles left a comment

bmingles left a comment

bmingles left a comment

feat: Add option to disable WebGL rendering #2134

feat: Add option to disable WebGL rendering #2134

Conversation

mofojed commented Jul 8, 2024

codecov bot commented Jul 8, 2024 • edited Loading

Codecov Report

dsmmcken left a comment

Choose a reason for hiding this comment

dsmmcken commented Jul 9, 2024

mofojed commented Jul 9, 2024

bmingles Jul 10, 2024

Choose a reason for hiding this comment

mofojed Jul 11, 2024

Choose a reason for hiding this comment

dsmmcken Jul 11, 2024

Choose a reason for hiding this comment

bmingles Jul 10, 2024

Choose a reason for hiding this comment

mofojed Jul 11, 2024

Choose a reason for hiding this comment

bmingles Jul 10, 2024

Choose a reason for hiding this comment

bmingles left a comment

Choose a reason for hiding this comment

bmingles left a comment

Choose a reason for hiding this comment

bmingles left a comment

Choose a reason for hiding this comment

bmingles left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 8, 2024 •

edited

Loading