Skip to content

fix(hooks): promote snow render targets to fp16 for banding#2410

Merged
SkrubbySkrubInAShrub merged 2 commits into
devfrom
claude/banding-fix-snow-rt-precision
May 31, 2026
Merged

fix(hooks): promote snow render targets to fp16 for banding#2410
SkrubbySkrubInAShrub merged 2 commits into
devfrom
claude/banding-fix-snow-rt-precision

Conversation

@doodlum
Copy link
Copy Markdown
Collaborator

@doodlum doodlum commented May 24, 2026

Extracted from the effect11sh2 branch as the snowswap-hooks half of the "banding fix" pair.

Summary

kSNOW and kSNOW_SWAP are created at R8G8B8A8_UNORM by vanilla. The snow shader accumulates wetness/sparkle values that exceed the 8-bit range and quantize into visible banding on tessellated snow. Hook their CreateRenderTarget calls (offsets 0x406/0x41C in BSShaderRenderTargets::Create) and override the format to R16G16B16A16_FLOAT to give the snow pass enough headroom.

Mirrors the existing CreateRenderTarget_Main / CreateRenderTarget_Normals / CreateRenderTarget_NormalsSwap / CreateRenderTarget_MotionVectors pattern, sourced from the same RelocationID.

Scope note: sky noise half held back

The companion commit on effect11sh2 ("better banding fix" in Sky.hlsl) replaces a per-pixel TPDF noise term — which effect11sh2 had also added — with a multiplicative 1 + noiseGrad * 10 scheme. Neither the TPDF term nor that scheme exists on dev, where the DITHER path has been restructured around ENABLE_LL (linear lighting) since the original commit. Forward-porting the multiplicative noise into the ENABLE_LL-conditional path is a graphics tuning decision that needs runtime evaluation, so it's been left out of this PR rather than guessed at.

Test plan

  • Build a Skyrim DLL and confirm tessellated snow no longer shows banding under varying sunlight.
  • Confirm no visual regression on non-snow geometry (these RTs only feed the snow pass).
  • Spot-check VRAM cost — going from R8 to R16 on these two RTs is small but non-zero.

Generated by Claude Code

Summary by CodeRabbit

  • New Features
    • Improved snow render target handling with enhanced visual quality through updated render format specifications.

kSNOW and kSNOW_SWAP are created at R8G8B8A8_UNORM by vanilla; the
snow shader accumulates wetness/sparkle values that exceed the 8-bit
range and quantize into visible banding on tessellated snow. Hook
their CreateRenderTarget calls (offsets 0x406/0x41C in
BSShaderRenderTargets::Create) and override the format to
R16G16B16A16_FLOAT to give the snow pass enough headroom.

Sources the two new hooks on the same RelocationID as the existing
Main / Normals / NormalsSwap / MotionVectors render-target hooks.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 24, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e03e8ca3-44af-4145-bb55-6b191ae49467

📥 Commits

Reviewing files that changed from the base of the PR and between e259a92 and 82a9313.

📒 Files selected for processing (1)
  • src/Hooks.cpp

📝 Walkthrough

Walkthrough

This PR adds two new renderer hooks (CreateRenderTarget_Snow and CreateRenderTarget_SnowSwap) that override the format of snow-related render targets to R16G16B16A16_FLOAT floating-point format before delegating to the original render target creation logic. Both hooks are installed in the Hooks::Install() routine.

Changes

Snow Render Target Hooks

Layer / File(s) Summary
Snow render target hooks
src/Hooks.cpp
CreateRenderTarget_Snow and CreateRenderTarget_SnowSwap hook structs set render target format to fp16 format and delegate to original creation logic. Hooks are installed in Hooks::Install() alongside existing render target hooks.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested reviewers

  • alandtse
  • sicsix
  • SkrubbySkrubInAShrub

Poem

🐰 Fresh snow hooks descend with float precision,
Render targets glow in fp16 vision,
Two new thunks installed with care,
Crystal clear now fills the air! ❄️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% 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 title 'fix(hooks): promote snow render targets to fp16 for banding' clearly and concisely describes the main change—converting snow render target formats from 8-bit to 16-bit floating-point to address banding issues.
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 claude/banding-fix-snow-rt-precision

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.12][ERROR]: Error: exception Unix_error: No such file or directory stat src/Hooks.cpp
Raised by primitive operation at UTmp.replace_named_pipe_by_regular_file_if_needed in file "libs/commons/UTmp.ml", line 145, characters 8-27
Called from Scan_CLI.replace_target_roots_by_regular_files_where_needed.(fun) in file "src/osemgrep/cli_scan/Scan_CLI.ml", lines 1086-1087, characters 19-65
Called from List_.fast_map in file "libs/commons/List_.ml", line 81, characters 17-20
Called from Scan_CLI.repla


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.

@github-actions
Copy link
Copy Markdown

No actionable suggestions for changed features.

@SkrubbySkrubInAShrub
Copy link
Copy Markdown
Collaborator

SkrubbySkrubInAShrub commented May 25, 2026

I lied.

@SkrubbySkrubInAShrub SkrubbySkrubInAShrub marked this pull request as ready for review May 31, 2026 12:33
@github-actions
Copy link
Copy Markdown

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

@SkrubbySkrubInAShrub SkrubbySkrubInAShrub merged commit ca1c494 into dev May 31, 2026
15 of 19 checks passed
@SkrubbySkrubInAShrub SkrubbySkrubInAShrub deleted the claude/banding-fix-snow-rt-precision branch May 31, 2026 13:14
ParticleTroned added a commit to ParticleTroned/skyrim-community-shaders that referenced this pull request Jun 4, 2026
Rationale: vanilla creates the snow and snow-swap render targets as R8G8B8A8_UNORM, while the snow shader can accumulate wetness and sparkle values that quantize visibly on tessellated snow.

Implementation: hook the kSNOW and kSNOW_SWAP CreateRenderTarget call sites and force their render-target format to R16G16B16A16_FLOAT before forwarding to the original creation path. The companion sky noise tuning from the source branch is intentionally left out.

Acknowledgement: Manual port of community-shaders#2410 by doodlum.

Co-authored-by: doodlum <15017472+doodlum@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.

3 participants