fix(upscaling): more upscaling fixes

[doodlum]

Summary by CodeRabbit

• New Features
  • Earlier initialization of graphics hooks to improve startup reliability.
• Bug Fixes
  • Reduced shimmering and aliasing in distant foliage for clearer visuals.
  • More consistent TAA behavior and UI jitter, improving overall image stability.
  • Improved stability across VR and non-VR modes during upscaling.
• Refactor
  • Separated TAA configuration from upscaling management for clearer behavior and sequencing.
  • Simplified texture copying paths to reduce edge-case issues and improve reliability.

[coderabbitai]

Walkthrough

Refactors Upscaling to split TAA configuration from resource management, updates jitter sequencing, and switches certain texture copies to full-resource copies. Introduces early DXGI/D3D11 hook installation during plugin load. Adds mip bias sampling in a shader. Updates headers to declare new APIs and removes an obsolete one.

Changes

Cohort / File(s)	Summary of changes
Upscaling refactor and copy behavior src/Features/Upscaling.cpp, src/Features/Upscaling.h	Added ConfigureTAA() and moved TAA flag handling out of ConfigureUpscaling(...). Adjusted jitter update flow to call ConfigureTAA() before original logic and ConfigureUpscaling(...) after. Switched specific D3D12 texture transfers to CopyResource. Removed ApplyNISSharpening() declaration.
Early DX hook installation src/Hooks.cpp, src/Hooks.h, src/XSEPlugin.cpp	Created Hooks::InstallEarlyHooks() and moved D3D11CreateDeviceAndSwapChain/CreateDXGIFactory IAT patches into it. Called InstallEarlyHooks() during plugin load when no errors are present. Added logging for early hooks.
Shader sampling bias package/Shaders/DistantTree.hlsl	Replaced TexDiffuse.Sample(...) with TexDiffuse.SampleBias(..., SharedData::MipBias) in alpha/baseColor sampling sites; logic and control flow unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Plugin as XSEPlugin::Load
  participant Hooks as Hooks
  participant Features as Feature::Load...

  Plugin->>Hooks: InstallEarlyHooks()
  note right of Hooks: Patch IAT for D3D11CreateDeviceAndSwapChain<br/>and CreateDXGIFactory(1)
  Plugin->>Features: Iterate and Load() each feature

Loading

sequenceDiagram
  autonumber
  participant Game as Main_UpdateJitter thunk
  participant Ups as Upscaling
  participant Orig as Original_UpdateJitter

  Game->>Ups: ConfigureTAA()
  Game->>Orig: Call original jitter update
  Game->>Ups: ConfigureUpscaling(state)
  note right of Ups: Resources, UI jitter/DR, offsets

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Possibly related PRs

• fix: fix bad camera data #1459 — Adjusts TAA control flow around upscaling; overlaps with this PR’s TAA refactor and sequencing.
• fix(upscaling): fix fg missing menu with TAA #1482 — Modifies when taaEnabled is toggled in Upscaling; related to new ConfigureTAA() path.
• fix: fix for terrain variation with upscaling feature breaking mips #1364 — Applies SharedData::MipBias in shader sampling; matches this PR’s SampleBias change in DistantTree.hlsl.

Suggested reviewers

• jiayev
• alandtse

Poem

I hop through frames with jittered grace,
TAA in place, I tweak the pace.
Early hooks nibble DX roots,
Full-copy carrots in my boots.
A mip-biased leaf sways near—
Render fields are crisp this year! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check	❓ Inconclusive	The PR title "fix(upscaling): more upscaling fixes" is related to the changeset but is vague and non-descriptive. It does not identify the main modifications such as adding Upscaling::ConfigureTAA, switching to full CopyResource copies for buffers, and introducing Hooks::InstallEarlyHooks with an earlier install call. Under the project's title guidelines this makes the check inconclusive because reviewers cannot quickly determine the PR’s primary intent.	Please update the title to a concise, specific summary of the primary change (for example: "fix(upscaling): add ConfigureTAA, use full CopyResource for buffers, and install early DX hooks"). If multiple fixes are equally important, choose the single most significant change for the title and document the rest in the PR description. A clearer title will help reviewers and maintain a more useful project history.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch upscaling-fixes-5

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Using provided base ref: 44ba961
Using base ref: 44ba961
Base commit date: 2025-09-19T14:02:09+01:00 (Friday, September 19, 2025 02:02 PM)
No actionable suggestions for changed features.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

src/Features/Upscaling.cpp (1)

1045-1045: Potential performance optimization opportunity.

The change from subresource copy to full resource copy is simpler but may have performance implications. Consider whether this was intentional for correctness or if subresource copy could still be used with proper region specification.

Was the change from subresource copy to full resource copy made for correctness reasons? If it was only for simplicity, you might want to profile both approaches to ensure there's no performance regression, especially for lower-end GPUs.

src/Hooks.cpp (2)

975-984: Make InstallEarlyHooks idempotent, hook DXGI first, and log failures + actual symbol.

• Idempotency avoids double-patching if called twice.
• Hook DXGI before D3D11 so the D3D11 thunk can safely use ptrCreateDXGIFactory in its fallback path.
• Log the chosen DXGI symbol and patch failures.

Apply this diff within InstallEarlyHooks:

 void InstallEarlyHooks()
 {
+    static bool installed = false;
+    if (installed) {
+        logger::debug("InstallEarlyHooks already installed");
+        return;
+    }
+    installed = true;
+
+    // Hook DXGI first to support fallbacks in the D3D11 thunk
+    const char* dxgiSym = REL::Module::IsVR() ? "CreateDXGIFactory1" : "CreateDXGIFactory";
+    logger::info("Hooking {}", dxgiSym);
+    if (auto orig = SKSE::PatchIAT(hk_CreateDXGIFactory, "dxgi.dll", dxgiSym); orig) {
+        *reinterpret_cast<uintptr_t*>(&ptrCreateDXGIFactory) = orig;
+    } else {
+        logger::warn("Failed to hook {}", dxgiSym);
+    }
+
-    if (!globals::features::upscaling.loaded) {
-        logger::info("Hooking D3D11CreateDeviceAndSwapChain");
-        *(uintptr_t*)&ptrD3D11CreateDeviceAndSwapChain = SKSE::PatchIAT(hk_D3D11CreateDeviceAndSwapChain, "d3d11.dll", "D3D11CreateDeviceAndSwapChain");
-    }
-
-    logger::info("Hooking CreateDXGIFactory");
-    *(uintptr_t*)&ptrCreateDXGIFactory = SKSE::PatchIAT(hk_CreateDXGIFactory, "dxgi.dll", !REL::Module::IsVR() ? "CreateDXGIFactory" : "CreateDXGIFactory1");
+    if (!globals::features::upscaling.loaded) {
+        logger::info("Hooking D3D11CreateDeviceAndSwapChain");
+        if (auto orig = SKSE::PatchIAT(hk_D3D11CreateDeviceAndSwapChain, "d3d11.dll", "D3D11CreateDeviceAndSwapChain"); orig) {
+            *reinterpret_cast<uintptr_t*>(&ptrD3D11CreateDeviceAndSwapChain) = orig;
+        } else {
+            logger::warn("Failed to hook D3D11CreateDeviceAndSwapChain");
+        }
+    }
 }

---

982-984: Optional: honor caller’s RIID in hk_CreateDXGIFactory for broader compatibility.

Current hook always requests IDXGIFactory4, ignoring the caller’s riid. It typically works (vtable extends), but forwarding the riid is safer. If you need IDXGIFactory4 internally, QueryInterface it separately.

Outside the selected lines, consider:

-HRESULT WINAPI hk_CreateDXGIFactory(REFIID, void** ppFactory)
-{
-    return ptrCreateDXGIFactory(__uuidof(IDXGIFactory4), ppFactory);
-}
+HRESULT WINAPI hk_CreateDXGIFactory(REFIID riid, void** ppFactory)
+{
+    return ptrCreateDXGIFactory(riid, ppFactory);
+}

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 44ba961 and a871380.

📒 Files selected for processing (6)

• package/Shaders/DistantTree.hlsl (2 hunks)
• src/Features/Upscaling.cpp (5 hunks)
• src/Features/Upscaling.h (1 hunks)
• src/Hooks.cpp (1 hunks)
• src/Hooks.h (1 hunks)
• src/XSEPlugin.cpp (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not include TODO/FIXME placeholders; provide complete, working solutions

Files:

• src/XSEPlugin.cpp
• src/Hooks.h
• package/Shaders/DistantTree.hlsl
• src/Features/Upscaling.h
• src/Hooks.cpp
• src/Features/Upscaling.cpp

src/**/*.{cpp,cxx,cc,h,hpp,hxx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code

Files:

• src/XSEPlugin.cpp
• src/Hooks.h
• src/Features/Upscaling.h
• src/Hooks.cpp
• src/Features/Upscaling.cpp

🧠 Learnings (3)

📓 Common learnings

Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.

📚 Learning: 2025-08-05T17:40:44.828Z

Learnt from: ThePagi
PR: doodlum/skyrim-community-shaders#1369
File: package/Shaders/Lighting.hlsl:0-0
Timestamp: 2025-08-05T17:40:44.828Z
Learning: In the skyrim-community-shaders repository, ultra trees (object LOD trees) are detected using a compound shader define condition `defined(DO_ALPHA_TEST) && defined(LOD_BLENDING) && defined(RIM_LIGHTING) && defined(SOFT_LIGHTING)` because "they have no define" according to the comment. The `ExtraFlags::IsTree` flag is used for different tree handling (AO removal in skylighting) and may not apply to ultra trees specifically. Before replacing the compound condition with `IsTree`, verification is needed to ensure the flag covers ultra trees.

Applied to files:

• package/Shaders/DistantTree.hlsl

📚 Learning: 2025-08-03T18:37:19.690Z

Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.

Applied to files:

• package/Shaders/DistantTree.hlsl

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
• GitHub Check: Build plugin and addons
• GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)

🔇 Additional comments (10)

src/Hooks.h (1)

23-23: LGTM! Clean addition of early hooks declaration.

The addition of InstallEarlyHooks() follows the existing pattern and provides a clear separation between early and regular hook installation phases.

src/Features/Upscaling.h (1)

131-131: LGTM! Appropriate separation of TAA configuration.

The addition of ConfigureTAA() provides better separation of concerns by isolating TAA-specific configuration from resource management.

package/Shaders/DistantTree.hlsl (2)

212-212: Good consistency with mip bias application.

Applying mip bias to both the alpha sampling (line 203) and base color sampling (line 212) ensures consistent texture sampling behavior across both render paths.

---

203-203: Verify SharedData::MipBias is declared and initialized before shader use.
MipBias is sampled in many shaders (e.g. package/Shaders/DistantTree.hlsl:203; package/Shaders/Water.hlsl:511; package/Shaders/Utility.hlsl:550; package/Shaders/Lighting.hlsl:1340), but my searches did not find a SharedData/cbuffer definition or initialization in the repo — ensure a cbuffer/uniform containing MipBias is declared and the host sets it (or provide a safe default, e.g. 0.0) before these shaders are bound.

src/Features/Upscaling.cpp (4)

649-662: LGTM! Clean separation of TAA configuration logic.

The new ConfigureTAA() method properly handles TAA-specific flags without resource management, providing better separation of concerns from the main upscaling configuration.

---

1428-1428: Consistency improvement with full resource copy.

Good consistency with the motion vector copy change. Using full resource copies throughout reduces complexity and potential region calculation errors.

---

1612-1614: LGTM! Proper sequencing of TAA and upscaling configuration.

The thunk correctly calls ConfigureTAA() before the original jitter update logic, then ConfigureUpscaling() after. This ensures proper initialization order.

---

664-674: Verify fDRClampOffset is set correctly for VR mode. src/Features/Upscaling.cpp was not found and no C++ assignment of fDRClampOffset was located; only package/Shaders/CopyShadowDataCS.hlsl references it (comments at lines 42, 50–51, 64, 72–73). Confirm whether the VR code path should also set fDRClampOffset to 0.0f or if the default value is intended.

src/XSEPlugin.cpp (1)

199-199: LGTM! Strategic placement of early hooks.

Installing early hooks before feature loading ensures that critical D3D11/DXGI interception is in place before any feature attempts to use these APIs during their Load() methods.

src/Hooks.cpp (1)

975-984: Confirm hook gating and duplicate IAT patches

Couldn't verify—ripgrep returned "No files were searched" (likely repo not present or --type filter). Validate these two points:

• Confirm InstallEarlyHooks is invoked before Feature::Load; if so, upscaling.loaded will be false and the D3D11 hook becomes unconditional—ensure this is intentional and won't conflict with Upscaling's init/hooks.
• Ensure there are no other IAT patches for D3D11/DXGI to avoid double-patching.

Run locally if needed:
rg -nP -C2 '\bInstallEarlyHooks\s*('
rg -nP -C2 '\bFeature::Load\s*('
rg -nP -C2 'PatchIAT([^,]+,\s*"d3d11.dll",\s*"D3D11CreateDeviceAndSwapChain")'
rg -nP -C2 'PatchIAT([^,]+,\s*"dxgi.dll",\s*"CreateDXGIFactory(1)?")'

[github-actions]

✅ A pre-release build is available for this PR:
Download