fix(upscaling): fix screenshots when upscaling enabled

[doodlum]

Summary by CodeRabbit

• Bug Fixes
  • Automatically disables TAA after data loads when upscaling is active, preventing visual artifacts and blur.
  • Resolves issues affecting screenshot clarity during upscaling.

[coderabbitai]

Walkthrough

Adds a new lifecycle hook Upscaling::DataLoaded() in Upscaling, declared in the header and implemented to set RE::GetINISetting("bUseTAA:Display")->data.b = false during data load, affecting TAA state post-load. No other logic or files are changed.

Changes

Cohort / File(s)	Summary
Upscaling lifecycle hook and TAA toggle src/Features/Upscaling.h, src/Features/Upscaling.cpp	Declare and implement virtual override DataLoaded(); implementation disables TAA by setting bUseTAA:Display to false after data load.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Game as Game/Data Loader
  participant Upscaling as Upscaling Feature
  participant INI as RE::INI Settings

  Game->>Upscaling: DataLoaded()
  note right of Upscaling: Post-data-load hook (new)
  Upscaling->>INI: GetINISetting("bUseTAA:Display")
  INI-->>Upscaling: INI setting handle
  Upscaling->>INI: set data.b = false (disable TAA)
  note over Upscaling,INI: Apply screenshot fix path

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix: fix bad camera data #1459 — Also modifies Upscaling to change TAA toggling behavior; similar intent around preventing TAA re-enablement.
• fix(upscaling): more upscaling fixes #1498 — Introduces a new Upscaling method for TAA configuration (ConfigureTAA), paralleling this PR’s new DataLoaded hook.
• fix(upscaling): fix fg missing menu with TAA #1482 — Adjusts Upscaling and post-processing to disable temporal AA via shader flags, closely related to TAA-off behavior.

Suggested reviewers

• Pentalimbed
• jiayev

Poem

A bunny flips a tiny switch,
TAA now takes a hitch—
In data’s wake, the toggle falls,
Crisp pixels line the engine’s halls.
Hop-hop! A cleaner view I bring,
With one small boop—spring-sprung, ping! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "fix(upscaling): fix screenshots when upscaling enabled" accurately and concisely captures the primary change described in the diff — adding Upscaling::DataLoaded to disable the TAA display setting to address screenshot issues when upscaling is enabled. It is specific to the upscaling feature and directly related to the code changes in the PR. A reviewer scanning the history will understand the main intent without extra detail.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch screenshots-fix

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

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: ed2d75a
Using base ref: ed2d75a
Base commit date: 2025-09-24T15:40:34+01:00 (Wednesday, September 24, 2025 03:40 PM)
No actionable suggestions for changed features.

[coderabbitai]

Actionable comments posted: 1

📜 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 ed2d75a and 48150ff.

📒 Files selected for processing (2)

• src/Features/Upscaling.cpp (1 hunks)
• src/Features/Upscaling.h (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/Features/Upscaling.h
• 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/Features/Upscaling.h
• src/Features/Upscaling.cpp

⏰ 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). (1)

• GitHub Check: Build plugin and addons

🔇 Additional comments (1)

src/Features/Upscaling.h (1)

96-96: New lifecycle hook looks fine — confirm Feature::DataLoaded exists and is invoked

Repository search found no src/Features/Upscaling.h, no Feature base declaring virtual void DataLoaded(), and no callers of DataLoaded(); ensure the base declares virtual void DataLoaded() and the lifecycle actually calls it so this override runs.

[coderabbitai]

⚠️ Potential issue

❓ Verification inconclusive

Null deref risk and unconditional TAA disable; gate by method and add nullptr guard

RE::GetINISetting(...) can return nullptr; dereferencing causes a crash. Also, this forces TAA off even when upscaling is kNONE or kTAA, diverging from the PR intent (“when upscaling enabled”) and may affect VR.

Suggested fix:

 void Upscaling::DataLoaded()
 {
-	// Fix screenshots fix from Engine Fixes
-	RE::GetINISetting("bUseTAA:Display")->data.b = false;
+	// Fix screenshots when upscaling is active: disable menu TAA toggle to prevent artifacts
+	if (globals::game::isVR)
+		return;  // Avoid touching Display INI in VR paths
+
+	auto* setting = RE::GetINISetting("bUseTAA:Display");
+	if (!setting) {
+		logger::warn("[Upscaling] INI setting 'bUseTAA:Display' not found; screenshot fix not applied");
+		return;
+	}
+
+	const auto method = GetUpscaleMethod();
+	if (method != UpscaleMethod::kNONE && method != UpscaleMethod::kTAA) {
+		setting->data.b = false;
+	}
 }

• If you intend the change to persist to disk, also call iniPrefSettingCollection->WriteSetting(setting) (consistent with SaveSettings), otherwise leave it in-memory only.

Please verify lifecycle order so that settings are loaded before DataLoaded() (to ensure GetUpscaleMethod() reflects the user config).

---

Guard GetINISetting and conditionally disable TAA

RE::GetINISetting(...) can return nullptr (crash); the current code also unconditionally disables TAA even for kNONE/kTAA and in VR — add a nullptr guard, skip touching the INI in VR, and only disable when upscaling is active.

 void Upscaling::DataLoaded()
 {
-	// Fix screenshots fix from Engine Fixes
-	RE::GetINISetting("bUseTAA:Display")->data.b = false;
+	// Fix screenshots when upscaling is active: disable menu TAA toggle to prevent artifacts
+	if (globals::game::isVR)
+		return;  // Avoid touching Display INI in VR paths
+
+	auto* setting = RE::GetINISetting("bUseTAA:Display");
+	if (!setting) {
+		logger::warn("[Upscaling] INI setting 'bUseTAA:Display' not found; screenshot fix not applied");
+		return;
+	}
+
+	const auto method = GetUpscaleMethod();
+	if (method != UpscaleMethod::kNONE && method != UpscaleMethod::kTAA) {
+		setting->data.b = false;
+	}
 }

If persistence is required, call iniPrefSettingCollection->WriteSetting(setting). Verify lifecycle order so settings are loaded before DataLoaded() (so GetUpscaleMethod() reflects user config).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	void Upscaling::DataLoaded()
	{
	// Fix screenshots fix from Engine Fixes
	RE::GetINISetting("bUseTAA:Display")->data.b = false;
	}
	void Upscaling::DataLoaded()
	{
	// Fix screenshots when upscaling is active: disable menu TAA toggle to prevent artifacts
	if (globals::game::isVR)
	return; // Avoid touching Display INI in VR paths
	auto* setting = RE::GetINISetting("bUseTAA:Display");
	if (!setting) {
	logger::warn("[Upscaling] INI setting 'bUseTAA:Display' not found; screenshot fix not applied");
	return;
	}
	const auto method = GetUpscaleMethod();
	if (method != UpscaleMethod::kNONE && method != UpscaleMethod::kTAA) {
	setting->data.b = false;
	}
	}

🤖 Prompt for AI Agents

In src/Features/Upscaling.cpp around lines 355 to 360, guard the
RE::GetINISetting call (it may return nullptr) and only disable TAA when
upscaling is actually active and not in VR: check GetINISetting(...) for nullptr
before using it, check the runtime VR flag and
GetUpscaleMethod()/upscaling-enabled condition and return early if VR or
upscaling is inactive, then set the INI setting only when safe; if you need the
change to persist, call iniPrefSettingCollection->WriteSetting(setting) after
modifying it and ensure DataLoaded() runs after settings are loaded so
GetUpscaleMethod() reflects user config.

[github-actions]

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