Implement Windows and Linux GPU Acceleration

[Jordonbc]

No description provided.

[github-actions]

Review Findings

Minor: Log message before logger initialization — Backend/src/lib.rs:154-157

The log::info! call that reports GPU acceleration status runs before logging::init() (line 160). The comment on line 159 explicitly states logging is initialized after these adjustments. Since no logger is configured yet, the log::info! call is silently dropped — the message never appears in application logs. This defeats the purpose of the log entry.

Fix: Either move logging::init() above the log message, or defer the log to after logging::init() (by calling it from the setup closure or by storing the value and logging later).

Confidence: High

---

Minor: Test gap for undefined/null inputs — Frontend/src/scripts/ui/layout.test.ts:84-93

applyGpuAccelerationPreference has the signature enabled: boolean | undefined | null, but the test only covers true and false. When undefined or null is passed, enabled === false evaluates to false, so data-gpu-acceleration is set to "on" (GPU enabled). This is the likely-correct fallback, but it is untested — so a future change could silently alter the fallback behaviour.

Fix: Add test cases for undefined and null inputs.

Confidence: Medium

---

No blocking or important issues found in this PR.

opencode session  |  github run

[github-actions]

Based on a thorough review of the full diff across all 10 changed files, here are my findings:

---

Minor: Misleading _ prefix on _main_window_browser_args — Backend/src/lib.rs:150-151

The variable _main_window_browser_args uses Rust's _ prefix convention which signals "intentionally unused." On Windows this variable IS used (cloned inside the setup closure at line ~228 and passed to additional_browser_args). The _ prefix is only correct on non-Windows where it is genuinely unused. Consider renaming to main_window_browser_args to avoid misleading readers.

Confidence: Medium

---

No blocking or important issues were found. The previous review comment claiming log::info! runs before logging::init() is incorrect — the final code clearly calls logging::init() on the line immediately before log::info!, so the log message will be properly recorded.

opencode session  |  github run

[github-actions]

Based on my review of the full diff across all 10 changed files, here is my finding:

---

Minor: SmartScreen flag incorrectly bundled with GPU-disable arguments — Backend/src/workarounds.rs:112

The main_window_browser_args function on Windows appends --disable-features=msSmartScreenProtection when GPU acceleration is disabled. Disabling SmartScreen has nothing to do with GPU acceleration — it is a WebView2 security feature for phishing/malware protection. While OpenVCS renders only local content (making the practical security impact negligible), bundling an unrelated feature flag under "disable GPU" is misleading and couples two independent concerns. If the intent is to reduce WebView2 overhead on low-end hardware, that should be a separate setting or at least documented as intentional.

Fix: Remove msSmartScreenProtection from the string, or if intentional, add a comment explaining why.

Confidence: Medium

---

No blocking or important issues found. The environment variable ordering on Linux is correct (apply_linux_nvidia_workaround controls WEBKIT_DISABLE_DMABUF_RENDERER, apply_gpu_acceleration_preference controls WEBKIT_DISABLE_COMPOSITING_MODE/WEBKIT_DISABLE_WEBGL — separate env vars, no conflict). The logging::init() call precedes log::info! (line 158 before 159), so the earlier review comment about the log being dropped was incorrect for the final code. The main_window_browser_args variable is properly cfg-gated to Windows and correctly captured in the setup closure. The frontend gpuChanged detection via modal.dataset.currentCfg works correctly because loadSettingsIntoForm populates that dataset attribute (line 919) before any save attempt. The undefined/null fallback test cases are present in the test file.

opencode session  |  github run

[github-actions]

I have completed my review of all 10 changed files in this PR. Here is my finding:

---

Minor: Unrelated WebView2 feature flags bundled with GPU-disable args — Backend/src/workarounds.rs:112

The GPU-disable argument string includes --disable-features=msWebOOUI,msPdfOOUI, which disables Office Online and PDF rendering features in WebView2. These features are unrelated to GPU acceleration. Bundling them under "disable GPU" couples two independent concerns and is misleading — a future maintainer removing the GPU-disable path might unknowingly re-enable these features, or someone trying to independently control them has no way to do so. While the practical impact is negligible (OpenVCS renders only local content), the coupling should be documented or the flags should be separated.

Suggested fix: Either remove msWebOOUI,msPdfOOUI from the disable string, or add a comment explaining why they are included alongside GPU flags.

Confidence: Medium

---

No blocking or important issues found. The test coverage for undefined/null inputs in applyGpuAccelerationPreference is present in layout.test.ts, the logging initialization precedes the log::info! call (lib.rs:158-165), the main_window_browser_args variable is properly cfg-gated and correctly captured in the setup closure on Windows, and the modal.dataset.currentCfg is populated via loadSettingsIntoForm (settings.ts:919) before any save attempt reads it.

opencode session  |  github run

[github-actions]

Based on my thorough review of all 10 changed files in this PR, no blocking or important issues were found.

The previous review comments in the PR thread referencing msSmartScreenProtection, msWebOOUI, msPdfOOUI flags, a _main_window_browser_args naming issue, and incorrect logging initialization order all refer to earlier revisions that are no longer present in the current code. The final diff shows:

• main_window_browser_args returns only --disable-gpu --disable-gpu-compositing (Backend/src/workarounds.rs:112)
• The variable is named main_window_browser_args without a _ prefix (Backend/src/lib.rs:151)
• logging::init() is called immediately before log::info! (Backend/src/lib.rs:158-159)
• The undefined/null test cases are present in the test file (Frontend/src/scripts/ui/layout.test.ts:84-93)

opencode session  |  github run

[github-actions]

Based on my review of the full diff across all 10 changed files, here are my findings:

---

Important: gpu_accel serde default mismatch (Backend/src/settings.rs:289)

The field uses #[serde(default)] which deserializes a missing field to false, while the struct-level Default impl returns true. Existing users who have a [performance] section in their config (e.g., from previously setting animations = false) but lack gpu_accel will silently default to GPU disabled on upgrade — contradicting the intended default. This field definition pre-dates the PR but becomes impactful now that the field is actively used.

Fix: #[serde(default = "default_true")] on gpu_accel (matching animations).

Confidence: High

---

Minor: Stale modal.dataset.currentCfg after save (Frontend/src/scripts/features/settings.ts:708)

After settings are saved, modal.dataset.currentCfg is not updated. On subsequent saves within the same modal session, gpuChanged is computed against the old config, potentially showing "GPU changes apply after restart" when GPU didn't actually change. Notification only, no behavioural impact.

Fix: Add modal.dataset.currentCfg = JSON.stringify(next); after save completes.

Confidence: Medium

---

No blocking issues were found. The compiler would catch any type errors at build time; the main_window_browser_args capture and cfg-gating, the frontend CSS attribute matching (data-gpuAcceleration → data-gpu-acceleration), the undefined/null fallback in applyGpuAccelerationPreference, and the notification logic are all correct. Earlier review comments about SmartScreen/msWebOOUI flags and _main_window_browser_args naming refer to prior revisions no longer present in the diff.

opencode session  |  github run

[github-actions]

No blocking or important issues found in this PR. The changes are well-structured and consistent:

• #[serde(default = "default_true")] on gpu_accel (Backend/src/settings.rs:288) correctly fixes the serde deserialization default to match the struct-level Default impl, preventing silent GPU-disabled on upgrade for users with existing [performance] config sections.
• Config is loaded once before AppState is created (Backend/src/lib.rs:150,167), avoiding the prior double-load.
• Window creation is moved from tauri.conf.json to programmatic setup() (Backend/src/lib.rs:214-244), enabling conditional additional_browser_args on Windows. The #[cfg] gating is consistent across variable definition and capture.
• Env var manipulation (Backend/src/workarounds.rs:54-72) follows the same unsafe-at-startup pattern as the existing apply_linux_nvidia_workaround.
• eprintln! is used for diagnostic output before logging::init(), avoiding the log crate before the logger is configured.
• Frontend save logic (Frontend/src/scripts/features/settings.ts:700-733) correctly detects GPU changes against modal.dataset.currentCfg and surfaces the restart-needed notification. currentCfg is updated after save (line 713, pre-existing).
• Tests cover true, false, undefined, and null inputs for the new function (Frontend/src/scripts/ui/layout.test.ts:84-93).

opencode session  |  github run