fix(desktop): improve Linux desktop experience #1208
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,7 +69,24 @@ export async function makeAppSetup( | |
| return window; | ||
| } | ||
|
|
||
| // Allow users with problematic GPU drivers to disable hardware acceleration | ||
| if (process.env.SUPERSET_DISABLE_GPU === "1") { | ||
| app.disableHardwareAcceleration(); | ||
| } | ||
|
|
||
| // Enable GPU optimizations on Linux | ||
| if (PLATFORM.IS_LINUX) { | ||
| app.commandLine.appendSwitch("enable-gpu-rasterization"); | ||
| app.commandLine.appendSwitch("enable-zero-copy"); | ||
| app.commandLine.appendSwitch("ignore-gpu-blocklist"); | ||
| // Canvas OOP rasterization offloads canvas rendering to GPU process | ||
| app.commandLine.appendSwitch( | ||
| "enable-features", | ||
| "CanvasOopRasterization,VaapiVideoDecodeLinuxGL", | ||
| ); | ||
| // Auto-detect Wayland vs X11 for native compositor integration | ||
| app.commandLine.appendSwitch("ozone-platform-hint", "auto"); | ||
| } | ||
|
Comment on lines
+77
to
+89
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: # Find package.json in the desktop app to check Electron version
find . -name "package.json" -type f | head -20Repository: superset-sh/superset Length of output: 586 🏁 Script executed: # Search for electron in package files
rg "electron" --type json -l | head -10Repository: superset-sh/superset Length of output: 163 🏁 Script executed: cat ./apps/desktop/package.json | rg "electron" -A 1 -B 1Repository: superset-sh/superset Length of output: 1388 🌐 Web query:
💡 Result: Electron 39.x is built on Chromium 142 (Chrome/Chromium “M142”). (endoflife.date) Exact Chromium patch versions depend on the Electron 39 minor/patch release, for example:
To verify for any specific 39.x version, check its entry on Electron Releases (it lists the bundled Chromium/Node/V8). (releases.electronjs.org) Citations:
🌐 Web query:
💡 Result: Electron 39 ships with Chromium 142.0.7444.52 (releases.electronjs.org), so you’re dealing with “modern” Linux VA-API behavior/flags. What
|
||
|
|
||
| PLATFORM.IS_WINDOWS && | ||
| app.setAppUserModelId( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ import nodePath from "node:path"; | |
| import { EXTERNAL_APPS, type ExternalApp } from "@superset/local-db"; | ||
|
|
||
| /** Map of app IDs to their macOS application names */ | ||
| const MAC_APP_NAMES: Record<ExternalApp, string | null> = { | ||
| finder: null, // Handled specially with shell.showItemInFolder | ||
| vscode: "Visual Studio Code", | ||
| "vscode-insiders": "Visual Studio Code - Insiders", | ||
|
|
@@ -29,17 +29,57 @@ const APP_NAMES: Record<ExternalApp, string | null> = { | |
| rustrover: "RustRover", | ||
| }; | ||
|
|
||
| /** Map of app IDs to their Linux CLI commands */ | ||
| const LINUX_APP_COMMANDS: Record<ExternalApp, string | null> = { | ||
| finder: null, | ||
| vscode: "code", | ||
| "vscode-insiders": "code-insiders", | ||
| cursor: "cursor", | ||
| zed: "zed", | ||
| xcode: null, // macOS only | ||
| iterm: null, // macOS only | ||
| warp: null, // macOS only | ||
| terminal: "x-terminal-emulator", | ||
| ghostty: "ghostty", | ||
| sublime: "subl", | ||
| intellij: "idea", | ||
| webstorm: "webstorm", | ||
| pycharm: "pycharm", | ||
| phpstorm: "phpstorm", | ||
| rubymine: "rubymine", | ||
| goland: "goland", | ||
| clion: "clion", | ||
| rider: "rider", | ||
| datagrip: "datagrip", | ||
| appcode: null, // macOS only | ||
| fleet: "fleet", | ||
| rustrover: "rustrover", | ||
| }; | ||
|
|
||
| /** | ||
| * Get the command and args to open a path in the specified app. | ||
| * Uses `open -a` on macOS, direct CLI commands on Linux. | ||
| */ | ||
| export function getAppCommand( | ||
| app: ExternalApp, | ||
| targetPath: string, | ||
| ): { command: string; args: string[] } | null { | ||
| if (process.platform === "darwin") { | ||
| const appName = MAC_APP_NAMES[app]; | ||
| if (!appName) return null; | ||
| return { command: "open", args: ["-a", appName, targetPath] }; | ||
| } | ||
|
|
||
| if (process.platform === "linux") { | ||
| const command = LINUX_APP_COMMANDS[app]; | ||
| if (!command) return null; | ||
| return { command, args: [targetPath] }; | ||
| } | ||
|
|
||
| // Windows: use start command | ||
| const appName = MAC_APP_NAMES[app]; | ||
| if (!appName) return null; | ||
| return { command: "cmd", args: ["/c", "start", "", appName, targetPath] }; | ||
|
Comment on lines
+79
to
+82
There was a problem hiding this comment. The Windows implementation in getAppCommand uses MAC_APP_NAMES which contains application display names like "Visual Studio Code", "IntelliJ IDEA", etc. These are not valid executable names for Windows. The |
||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,6 +117,9 @@ class PortManager extends EventEmitter { | |
| if (this.scanInterval) return; | ||
|
|
||
| this.scanInterval = setInterval(() => { | ||
| // Skip scanning when no sessions are registered (adaptive polling) | ||
| if (this.sessions.size === 0 && this.daemonSessions.size === 0) return; | ||
|
|
||
| this.scanAllSessions().catch((error) => { | ||
| console.error("[PortManager] Scan error:", error); | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,29 +50,47 @@ export interface InitialWindowBounds { | |
| /** | ||
| * Computes initial window bounds from saved state, with fallbacks. | ||
| * | ||
| * - No saved state → maximize on primary display | ||
| * - Saved position visible → restore exactly | ||
| * - Saved position not visible (monitor disconnected) → use saved size, but center | ||
| * - Saved size is much smaller than current display → scale up proportionally | ||
| */ | ||
| export function getInitialWindowBounds( | ||
| savedState: WindowState | null, | ||
| ): InitialWindowBounds { | ||
| const { workAreaSize } = screen.getPrimaryDisplay(); | ||
|
|
||
| // No saved state → maximize by default for best first-launch experience | ||
| if (!savedState) { | ||
| return { | ||
| width: workAreaSize.width, | ||
| height: workAreaSize.height, | ||
| center: true, | ||
| isMaximized: true, | ||
| }; | ||
| } | ||
|
|
||
| let { width, height } = clampToWorkArea(savedState.width, savedState.height); | ||
|
|
||
| // Scale up if saved bounds are much smaller than current work area | ||
| // This handles moving from a small screen to a large screen | ||
| const areaRatio = | ||
| (width * height) / (workAreaSize.width * workAreaSize.height); | ||
| if (areaRatio < 0.6) { | ||
| // Saved window covers less than 60% of current screen → scale up | ||
| // Use 90% of work area while maintaining aspect ratio | ||
| const savedAspect = width / height; | ||
| const targetWidth = Math.round(workAreaSize.width * 0.9); | ||
| const targetHeight = Math.round(targetWidth / savedAspect); | ||
|
|
||
| if (targetHeight <= workAreaSize.height) { | ||
| width = targetWidth; | ||
| height = targetHeight; | ||
| } else { | ||
| height = Math.round(workAreaSize.height * 0.9); | ||
| width = Math.round(height * savedAspect); | ||
| } | ||
| } | ||
|
Comment on lines
+73
to
+93
Contributor
There was a problem hiding this comment. Scale against the display that contains the saved bounds. Right now scaling is based on the primary display’s work area. If the window was saved on a smaller secondary display, the scaled bounds can exceed that display and be treated as off-screen, causing an unexpected recenter onto the primary display. Consider deriving the work area from the display matching the saved bounds before applying the scale. Also, please extract the scaling thresholds to constants and switch 🔧 Suggested update (display-aware scaling + constants + object params) const MIN_VISIBLE_OVERLAP = 50;
const MIN_WINDOW_SIZE = 400;
+const SCALE_UP_AREA_RATIO_THRESHOLD = 0.6;
+const SCALE_UP_WORKAREA_RATIO = 0.9;
function clampToWorkArea(
- width: number,
- height: number,
+ { width, height }: { width: number; height: number },
): { width: number; height: number } {
const { workAreaSize } = screen.getPrimaryDisplay();
return {
width: Math.min(Math.max(width, MIN_WINDOW_SIZE), workAreaSize.width),
height: Math.min(Math.max(height, MIN_WINDOW_SIZE), workAreaSize.height),
};
}
export function getInitialWindowBounds(
savedState: WindowState | null,
): InitialWindowBounds {
- const { workAreaSize } = screen.getPrimaryDisplay();
+ const primaryWorkArea = screen.getPrimaryDisplay().workAreaSize;
// No saved state → maximize by default for best first-launch experience
if (!savedState) {
return {
- width: workAreaSize.width,
- height: workAreaSize.height,
+ width: primaryWorkArea.width,
+ height: primaryWorkArea.height,
center: true,
isMaximized: true,
};
}
- let { width, height } = clampToWorkArea(savedState.width, savedState.height);
+ let { width, height } = clampToWorkArea({
+ width: savedState.width,
+ height: savedState.height,
+ });
+ const baseBounds: Rectangle = {
+ x: savedState.x,
+ y: savedState.y,
+ width,
+ height,
+ };
+ const { workAreaSize } = screen.getDisplayMatching(baseBounds);
// Scale up if saved bounds are much smaller than current work area
// This handles moving from a small screen to a large screen
const areaRatio =
(width * height) / (workAreaSize.width * workAreaSize.height);
- if (areaRatio < 0.6) {
+ if (areaRatio < SCALE_UP_AREA_RATIO_THRESHOLD) {
// Saved window covers less than 60% of current screen → scale up
// Use 90% of work area while maintaining aspect ratio
const savedAspect = width / height;
- const targetWidth = Math.round(workAreaSize.width * 0.9);
+ const targetWidth = Math.round(
+ workAreaSize.width * SCALE_UP_WORKAREA_RATIO,
+ );
const targetHeight = Math.round(targetWidth / savedAspect);
if (targetHeight <= workAreaSize.height) {
width = targetWidth;
height = targetHeight;
} else {
- height = Math.round(workAreaSize.height * 0.9);
+ height = Math.round(
+ workAreaSize.height * SCALE_UP_WORKAREA_RATIO,
+ );
width = Math.round(height * savedAspect);
}
}As per coding guidelines, Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic; Use object parameters for functions with 2 or more parameters instead of positional arguments. 🤖 Prompt for AI Agents |
||
|
|
||
| const savedBounds: Rectangle = { | ||
| x: savedState.x, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,7 +136,7 @@ export class Session { | |
| this.emulator = new HeadlessEmulator({ | ||
| cols: options.cols, | ||
| rows: options.rows, | ||
| scrollback: options.scrollbackLines ?? 5000, | ||
| }); | ||
|
|
||
| // Set initial CWD | ||
|
|
@@ -940,7 +940,7 @@ export class Session { | |
| if (process.platform === "win32") { | ||
| return process.env.COMSPEC || "cmd.exe"; | ||
| } | ||
| return process.env.SHELL || "/bin/sh"; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GPU acceleration escape hatch (SUPERSET_DISABLE_GPU=1) doesn't fully disable GPU features on Linux. When SUPERSET_DISABLE_GPU=1 is set, disableHardwareAcceleration() is called, but then the Linux-specific GPU switches (enable-gpu-rasterization, enable-zero-copy, etc.) are still applied unconditionally. This creates inconsistent behavior where the user attempts to disable GPU acceleration, but GPU-specific features are still enabled via command-line switches. The Linux GPU optimization block should be wrapped in a condition that checks if GPU acceleration wasn't disabled.