Skip to content

chore: add shader compile settings for development#2234

Merged
alandtse merged 5 commits into
devfrom
shader-compile
Apr 30, 2026
Merged

chore: add shader compile settings for development#2234
alandtse merged 5 commits into
devfrom
shader-compile

Conversation

@doodlum
Copy link
Copy Markdown
Collaborator

@doodlum doodlum commented Apr 29, 2026

Summary by CodeRabbit

  • New Features
    • Two new shader compiler toggles in Developer settings: "Half Precision (Partial Precision)" and "Avoid Flow Control".
    • Tooltips explain each toggle’s effects.
    • Changing either toggle immediately clears the shader cache to force recompilation.
    • "Half Precision (Partial Precision)" persists between sessions; "Avoid Flow Control" is transient and not saved.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

📝 Walkthrough

Walkthrough

Adds two developer shader-compiler toggles to the Advanced Settings UI (Partial Precision and Avoid Flow Control), exposes them as atomic flags in State (Partial Precision persisted; Avoid Flow Control transient), clears shader cache when toggled, and applies corresponding D3DCompile flags (plus SKIP_VALIDATION when disk cache is used) during shader compilation.

Changes

Cohort / File(s) Summary
Developer Settings UI
src/Menu/AdvancedSettingsRenderer.cpp
Added two checkboxes (Half Precision / Avoid Flow Control) that use atomic load/store (relaxed), show tooltips via Util::HoverTooltipWrapper(), update globals::state, and immediately call globals::shaderCache->Clear() on change.
State declarations & persistence
src/State.h, src/State.cpp
Added std::atomic_bool enablePartialPrecision{false} and std::atomic_bool enableAvoidFlowControl{false}. SaveToJson persists Partial Precision under Advanced["Partial Precision"]; LoadFromJson restores it if present. Avoid Flow Control remains transient (not saved).
Shader compilation behavior
src/Utils/D3D.cpp
Compiler reads the atomic flags and conditionally sets D3DCOMPILE_PARTIAL_PRECISION and D3DCOMPILE_AVOID_FLOW_CONTROL. When disk cache is active (globals::shaderCache->IsDiskCache()), D3DCOMPILE_SKIP_VALIDATION is also added to skip fxc validation.

Sequence Diagram

sequenceDiagram
    participant User
    participant UI as Advanced Settings UI
    participant State as State (atomics)
    participant Cache as Shader Cache
    participant Compiler as D3D Compiler

    User->>UI: Toggle Half Precision / Avoid Flow Control
    UI->>State: atomic store (enablePartialPrecision / enableAvoidFlowControl)
    UI->>Cache: globals::shaderCache->Clear()
    Note over Cache: Shader cache invalidated
    User->>Compiler: Request shader build
    Compiler->>State: atomic load (partialPrecision, avoidFlowControl)
    Compiler->>Cache: IsDiskCache()
    alt disk cache active
        Compiler->>Compiler: add D3DCOMPILE_SKIP_VALIDATION
    end
    Compiler->>Compiler: add D3DCOMPILE_PARTIAL_PRECISION / D3DCOMPILE_AVOID_FLOW_CONTROL as needed
    Compiler->>Cache: Store compiled shader
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • alandtse
  • SkrubbySkrubInAShrub

Poem

🐰 I flipped two tiny toggles under moonlit code,
Precision trimmed, flow control on the road,
Cache swept clean so compilers may play,
Flags whisper to shaders and they learn to sway,
A rabbit hops home, builds purring all the way.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'chore: add shader compile settings for development' accurately describes the main change: adding new shader compilation toggles (partial precision and avoid flow control) to the developer UI settings.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch shader-compile

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
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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.
@doodlum doodlum changed the title chore: shader compile settings for development chore: add shader compile settings for development Apr 29, 2026
@github-actions
Copy link
Copy Markdown

No actionable suggestions for changed features.

Changed enablePartialPrecision from true to false.
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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/State.h`:
- Around line 144-155: Change the two plain bool fields enablePartialPrecision
and enableAvoidFlowControl in State (declared in State.h) to std::atomic_bool to
avoid data races, and update every access site (UI thread stores and shader
compilation worker thread reads) to use .store(...) for writes and .load() for
reads; search for references to enablePartialPrecision and
enableAvoidFlowControl and replace direct assignments/reads with the atomic
.store/.load patterns so all thread interactions are synchronized.
🪄 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: a4e3a611-ff58-467a-8d75-f25ff04d9e91

📥 Commits

Reviewing files that changed from the base of the PR and between 651426e and 18a2f8d.

📒 Files selected for processing (4)
  • src/Menu/AdvancedSettingsRenderer.cpp
  • src/State.cpp
  • src/State.h
  • src/Utils/D3D.cpp

Comment thread src/State.h Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 30, 2026

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

Compilation runs on a worker pool while the toggles are written from
the UI thread; convert enablePartialPrecision and enableAvoidFlowControl
to std::atomic_bool so the cross-thread access is well-defined.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Utils/D3D.cpp`:
- Around line 201-205: The
CreatePixelShader/CreateVertexShader/CreateHullShader/CreateDomainShader calls
currently return directly when D3DCOMPILE_SKIP_VALIDATION is set, leaving
HRESULTs unchecked; wrap each of these four ID3D11Device::Create*Shader calls
with DX::ThrowIfFailed (like the existing compute shader paths) so failures
throw on error instead of returning potentially invalid pointers, preserving the
existing behavior for the ComputeShader paths that already use DX::ThrowIfFailed
and keeping D3DCOMPILE_SKIP_VALIDATION logic intact.
🪄 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: b639bc2b-3e1f-4d24-ae43-cc4b61bbe0cb

📥 Commits

Reviewing files that changed from the base of the PR and between 066829f and 4719701.

📒 Files selected for processing (4)
  • src/Menu/AdvancedSettingsRenderer.cpp
  • src/State.cpp
  • src/State.h
  • src/Utils/D3D.cpp
✅ Files skipped from review due to trivial changes (1)
  • src/State.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/Menu/AdvancedSettingsRenderer.cpp

Comment thread src/Utils/D3D.cpp
@alandtse alandtse merged commit 0e692ec into dev Apr 30, 2026
16 checks passed
ParticleTroned pushed a commit to ParticleTroned/skyrim-community-shaders that referenced this pull request May 2, 2026
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