Skip to content

chore(upscaling): update SDKs#1684

Merged
davo0411 merged 2 commits into
devfrom
upscaling-updates-4
Jan 11, 2026
Merged

chore(upscaling): update SDKs#1684
davo0411 merged 2 commits into
devfrom
upscaling-updates-4

Conversation

@doodlum
Copy link
Copy Markdown
Collaborator

@doodlum doodlum commented Jan 6, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Removed DLSS preset selection from upscaling settings, including the VR-specific preset UI option.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 6, 2026

📝 Walkthrough

Walkthrough

This pull request removes DLSS preset selection functionality from the upscaling feature. Changes include removing the DLSSPreset member from the Settings struct, eliminating the associated UI controls, and stripping out the preset-based branching logic that previously configured multiple DLSS variant presets based on user selection. The submodule reference for Streamline-DX12 is also updated.

Changes

Cohort / File(s) Summary
Settings Structure Removal
src/Features/Upscaling.h, src/Features/Upscaling.cpp
Removed DLSSPreset member (defaulted to 2) from Upscaling::Settings struct and updated serialization macro to exclude the field.
DLSS Preset Configuration Logic
src/Features/Upscaling/Streamline.cpp
Eliminated preset-based branching that selected and applied DLSS presets (dlaa, quality, balanced, performance, ultraPerformance). Mode mapping and standard option initialization remain functional.
Dependency Update
extern/Streamline-DX12
Submodule pointer updated from b998246a3d499c08765c5681b229c9e6b4513348 to 348ae90b95f37e7f7b7a81296589fdfc6a5b15b9.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • doodlum
  • jiayev

Poem

🐰 A preset so fine, now takes its leave,
No DLSS branching to deceive,
Settings streamlined, clean and bright,
The upscaler now sees the light! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
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.
Title check ❓ Inconclusive The title 'chore(upscaling): update SDKs' is vague and does not accurately describe the main changes, which involve removing the DLSSPreset feature from the upscaling settings. Consider revising the title to be more specific about the actual changes, such as 'chore(upscaling): remove DLSSPreset from settings' or similar, to better reflect the functional changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings

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

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

Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 6, 2026

Using provided base ref: ba6e43e
Using base ref: ba6e43e
Base commit date: 2026-01-06T21:26:34+10:00 (Tuesday, January 06, 2026 09:26 PM)
No actionable suggestions for changed features.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/Features/Upscaling.cpp (1)

15-25: Consider a more descriptive PR title.

The current title "chore(upscaling): update SDKs" emphasizes the SDK update but doesn't describe the main functional change—removal of DLSS preset selection. Consider a title that better captures the scope, such as:

  • refactor(upscaling): remove DLSS preset selection and update SDKs
  • chore(upscaling): streamline DLSS configuration and update SDKs

This helps reviewers and future maintainers better understand the changes at a glance.

Based on coding guidelines requesting suggestions for conventional commit titles that accurately describe code changes.

📜 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 ba6e43e and 7afd496.

⛔ Files ignored due to path filters (7)
  • features/Upscaling/Shaders/Upscaling/FidelityFX/amd_fidelityfx_framegeneration_dx12.dll is excluded by !**/*.dll
  • features/Upscaling/Shaders/Upscaling/FidelityFX/amd_fidelityfx_loader_dx12.dll is excluded by !**/*.dll
  • features/Upscaling/Shaders/Upscaling/Streamline/nvngx_dlss.dll is excluded by !**/*.dll
  • features/Upscaling/Shaders/Upscaling/Streamline/sl.common.dll is excluded by !**/*.dll
  • features/Upscaling/Shaders/Upscaling/Streamline/sl.dlss.dll is excluded by !**/*.dll
  • features/Upscaling/Shaders/Upscaling/Streamline/sl.interposer.dll is excluded by !**/*.dll
  • features/Upscaling/Shaders/Upscaling/Streamline/sl.nis.dll is excluded by !**/*.dll
📒 Files selected for processing (6)
  • extern/Streamline-DX12
  • features/Upscaling/Shaders/Upscaling/FidelityFX/amd_fidelityfx_framegeneration_dx12.pdb
  • features/Upscaling/Shaders/Upscaling/FidelityFX/amd_fidelityfx_loader_dx12.pdb
  • src/Features/Upscaling.cpp
  • src/Features/Upscaling.h
  • src/Features/Upscaling/Streamline.cpp
💤 Files with no reviewable changes (2)
  • src/Features/Upscaling.h
  • src/Features/Upscaling/Streamline.cpp
🧰 Additional context used
📓 Path-based instructions (3)
**/*

⚙️ CodeRabbit configuration file

**/*: When reviewing PRs, please provide suggestions for:

  1. Conventional Commit Titles (if not following https://www.conventionalcommits.org/ or
    if the existing title does not describe the code changes):
    Format: type(scope): description
    Length: 50 characters limit for title, 72 for body
    Style: lowercase description, no ending period
    Examples:

    • feat(vr): add cross-eye sampling
    • fix(water): resolve flowmap bug
    • docs: update shader documentation
  2. Issue References (if PR fixes bugs or implements features):
    Suggest adding appropriate GitHub keywords:

    • "Fixes #123" or "Closes #123" for bug fixes
    • "Implements #123" or "Addresses #123" for features
    • "Related to #123" for partial implementations

Otherwise, use your standard review approach focusing on code quality.

Files:

  • extern/Streamline-DX12
  • src/Features/Upscaling.cpp
**/*.{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.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.cpp
🧠 Learnings (1)
📓 Common learnings
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 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.
Learnt from: ThePagi
Repo: doodlum/skyrim-community-shaders PR: 1369
File: src/Features/SnowCover.cpp:515-515
Timestamp: 2025-10-02T14:20:33.454Z
Learning: In the Community Shaders codebase (skyrim-community-shaders repository), hardcoded shader resource slot numbers are used directly in code rather than being defined as named constants. This is the established convention and should not be flagged as an issue.
⏰ 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). (4)
  • 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)
  • GitHub Check: Run Shader Unit Tests
🔇 Additional comments (5)
src/Features/Upscaling.cpp (3)

186-218: LGTM: Preset UI correctly uses qualityMode.

The upscaling preset UI has been properly updated to use settings.qualityMode for both FSR and DLSS methods. The bounds checking with std::clamp at line 194 is good defensive programming.


353-374: Proper settings validation with bounds checking.

The settings loading includes appropriate validation and clamping for the upscale method enums. The bounds checking at lines 359-361 and 363-365 will prevent out-of-range values from causing issues.


15-25: File not found in repository. The review references src/Features/Upscaling.cpp, but this file does not exist in the codebase. Additionally, searches for the referenced code patterns—including NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT, DLSSPreset, upscaleMethod, and related symbols—return no results. The review cannot be validated against the actual repository content.

Likely an incorrect or invalid review comment.

extern/Streamline-DX12 (2)

1-1: PR title follows conventional commits format correctly.

The title "chore(upscaling): update SDKs" appropriately uses the conventional commit format with type and scope.


1-1: This review comment is based on incorrect assumptions about the codebase.

The Upscaling settings structure does not contain a DLSSPreset member that needs to be removed. The current implementation correctly uses an UpscaleMethod enum (with values kNONE, kTAA, kFSR, kDLSS) for method selection and a qualityMode uint for quality level settings. The serialization logic includes only the necessary fields, and the UI properly displays preset options through local string arrays without any orphaned preset-specific branching logic.

The submodule pointer update to Streamline-DX12 is valid, but the verification concerns raised in this comment do not apply to the actual code structure in the repository.

Likely an incorrect or invalid review comment.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 6, 2026

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

@davo0411 davo0411 merged commit 55806e6 into dev Jan 11, 2026
18 checks passed
@davo0411 davo0411 deleted the upscaling-updates-4 branch January 11, 2026 01:39
davo0411 added a commit to davo0411/skyrim-community-shaders that referenced this pull request Jan 11, 2026
* build: bump eastl to 3.27.01 (community-shaders#1673)

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

* fix(grass collision): ignore hkpListShape (community-shaders#1661)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* chore(upscaling): update SDKs (community-shaders#1684)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* chore(terrain shadows): modernize (community-shaders#1678)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* feat: add user wiki link to readme (community-shaders#1689)

* feat(LLF): add debug overlay in llf visualiser (community-shaders#1666)

* feat: linear lighting (community-shaders#1359)

Co-authored-by: jiayev <jiayev@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* feat: weather and imagespace editor (community-shaders#1630)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fix(ui): load previously selected theme on startup (community-shaders#1664)

---------

Co-authored-by: Alan Tse <alandtse@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: jiayev <l936249247@hotmail.com>
Co-authored-by: jiayev <jiayev@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants