fix(upscaling): guard OpenComposite conflict#79
Conversation
When the OpenComposite VR shim runs its own DLSS/FSR/DLAA upscaling, our upscaler stacking on top causes double upscaling, double jitter, and conflicting dynamic-resolution sizing in VR. Detect OpenComposite's upscaling from opencomposite.ini and stand down: force Method to None, lock the picker, and skip our D3D11 device hook, render hooks, resource setup, and Streamline/FidelityFX init while OpenComposite owns it. VR-only and gated on globals::game::isVR; SteamVR users are unaffected. Ported from ParticleTroned#1, re-authored against our ImGui::Combo method picker and ordered ahead of FoveatedRender setup so the foveated path stands down too. Dropped the source PR's unused refresh-timestamp field. Co-Authored-By: ColdBomb1 <ColdBomb1@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughDetects OpenComposite upscaling by parsing nearby INI files, caches the result, and forces the local upscaling pipeline (UI, persistence, SDKs, hooks, resources) to NONE when OpenComposite is active. ChangesOpenComposite Upscaling Blocker
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.22.0)OpenGrep fatal error (exit code 2): [00.14][ERROR]: Error: exception Unix_error: No such file or directory stat src/Features/VR/OpenVRDetection.h 🔧 Infer (1.2.0)src/Features/Upscaling.cppUsage Error: Failed to execute compilation command: Error message: *** Infer needs a working compilation command to run. src/Features/VR/OpenVRDetection.cppUsage Error: Failed to execute compilation command: Error message: *** Infer needs a working compilation command to run. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a VR-only guard that detects when the OpenComposite shim is doing its own DLSS/FSR/DLAA upscaling (by probing opencomposite.ini / opencomposite_ext.ini) and forces the Upscaling feature to stand down so the two don't stack. The guard short-circuits lifecycle entry points (Load, PostPostLoad, SetupResources, LoadUpscalingSDKs, DataLoaded), forces upscaleMethod/upscaleMethodNoDLSS to None across Save/Load/Restore, and locks the Method picker in the UI with a warning. Results are cached and refreshed only on lifecycle entry points so per-frame callers (GetUpscaleMethod, DrawSettings) stay cheap.
Changes:
- New anonymous-namespace helpers in
Upscaling.cppto enumerate OpenComposite config candidates and parse boolean upscaling keys section-agnostically. Upscaling::ApplyOpenCompositeUpscalingBlocker/GetOpenCompositeUpscalingBlockerwith a cachedOpenCompositeUpscalingBlockerstruct; wired into Load/PostPostLoad/SetupResources/LoadUpscalingSDKs/DataLoaded/Save/Load/Restore andGetUpscaleMethod.- UI: disables the Method combo and prints a
WrappedWarningplus tooltip when the guard fires.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Features/Upscaling.h | Declares the OpenCompositeUpscalingBlocker struct, cache fields, and getter/applier members. |
| src/Features/Upscaling.cpp | Implements config probing, the guard apply/get methods, lifecycle short-circuits, and UI lock + warning. |
|
No actionable suggestions for changed features. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Features/Upscaling.cpp`:
- Around line 311-326: GetOpenCompositeConfigCandidates currently only adds
opencomposite.ini from the GetLoadedOpenVRDirectory but omits
opencomposite_ext.ini; update the block that checks loadedOpenVRDirectory in
GetOpenCompositeConfigCandidates so that after AddUniquePath(candidates,
loadedOpenVRDirectory / L"opencomposite.ini") you also call
AddUniquePath(candidates, loadedOpenVRDirectory / L"opencomposite_ext.ini") (use
the same pattern as the currentDirectory block and keep using
GetLoadedOpenVRDirectory and AddUniquePath to ensure both files are discovered
next to openvr_api.dll).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d97ab092-0d82-4f85-9b56-643138724dcb
📒 Files selected for processing (2)
src/Features/Upscaling.cppsrc/Features/Upscaling.h
Extract the opencomposite.ini probing/parsing out of Upscaling.cpp into VRDetection::DetectOpenCompositeUpscaling, where it sits alongside the existing OpenComposite runtime detection. Upscaling keeps only the force-to-None policy and the lifecycle/UI gating, dropping ~230 lines and its parallel blocker struct in favor of the shared VRDetection type. Consolidate the loaded openvr_api.dll path lookup into one shared helper (GatherDLLInfo previously had its own copy), and search opencomposite_ext.ini next to the DLL too, not just in the CWD. Co-Authored-By: ColdBomb1 <ColdBomb1@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
✅ A pre-release build is available for this PR: |
Address self-review of the OpenComposite guard: condense the verbose multi-line comments to one-liners (keeping only non-obvious "why"), drop the redundant re-Apply in DrawSettings (the method combo is already disabled when blocked, so the value can't change), and short-circuit the per-frame GetUpscaleMethod blocker check on globals::game::isVR so non-VR pays zero per-frame cost. No behavior change: the blocker was already inactive on non-VR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g enables it ReadOpenCompositeUpscalingSettings probes multiple candidates (DLL dir + CWD, opencomposite.ini + opencomposite_ext.ini). UpdateOpenCompositeSettingValue overwrote unconditionally, so a later file's explicit false clobbered an earlier file's true and the guard could miss an active upscaler. Only an explicit true now updates the setting (and records the enabling file's path), matching the block-if-any-enabled intent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Ports the OpenComposite upscaling conflict guard from
ParticleTroned/skyrim-community-shaders#1
(by @ColdBomb1) into Open Shaders.
When the OpenComposite VR shim (OpenXR-over-OpenVR) runs its own DLSS/FSR/DLAA
upscaling, our upscaler stacking on top causes double upscaling, double jitter,
and conflicting dynamic-resolution sizing — blur/ghosting/broken AA in the
headset. This detects OpenComposite's upscaling from
opencomposite.iniandmakes our Upscaling feature stand down so OpenComposite owns the upscale.
What it does
opencomposite.ini/opencomposite_ext.ini(loadedopenvr_api.dlldirectory + CWD) for
dlssEnabled,fsrEnabled,dlaaEnabled,fsrNativeAA,fsr3PostAAEnabled(section-agnostic).upscaleMethod/upscaleMethodNoDLSSto None, locks the Method picker with a warning, andshort-circuits
Load(IAT hook),PostPostLoad(render hooks),SetupResources, andLoadUpscalingSDKs(Streamline/FFX init).GetUpscaleMethod()returns None while blocked;Save/Load/Restorere-assert None so the override persists.
the cached bool.
Differences from the source PR (re-authored, not cherry-picked)
ImGui::Combo+BootSnapshot, not the source'sSliderInt/UpscaleUiChoice. The lock + warning are re-authored against ours.PostPostLoadearly-return is placed beforefoveatedRender.PostPostLoad(), and theGetUpscaleMethodcheck before thePerfMode branch, so our DLSS-foveated divergence stands down too.
GetLoadedOpenVRDirectory()rather than reusingVRDetection::GatherDLLInfo(which also parses version info / enumerates thefile — more work than a directory lookup needs). Possible future consolidation.
naming rules; warning uses
Util::Text::WrappedWarning.Testing
BuildRelease.bat ALL(universal SE/AE/VR), exit 0, no warningson
Upscaling.cpp.on
globals::game::isVRand only fires whenopencomposite.iniactually hasan upscaler enabled, so SteamVR and flat-screen users are unaffected. In-game
VR validation recommended before merge.
Attribution
Original work by @ColdBomb1 in ParticleTroned#1.
🤖 Generated with Claude Code
Summary by CodeRabbit