fix: visual artifacts small particles#1574
Conversation
WalkthroughModifies reactiveMask calculation in EncodeTexturesCS.hlsl by adding two max operations: clamping against NormalsWaterMask.w and transparencyCompositionMask. This introduces additional data dependencies before the final ReactiveMask assignment. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
features/Upscaling/Shaders/Upscaling/EncodeTexturesCS.hlsl (1)
66-67: LGTM! Consider consolidating texture reads for efficiency.The logic correctly incorporates water and transparency information into the reactive mask, which should help the upscaler apply appropriate temporal filtering for these surfaces.
However,
NormalsWaterMask[dispatchID.xy]is read twice—once on line 24 for.zand again here for.w. Consider reading the fullfloat4once and extracting both components:- float transparencyCompositionMask = NormalsWaterMask[dispatchID.xy].z; + float4 normalsWater = NormalsWaterMask[dispatchID.xy]; + float transparencyCompositionMask = normalsWater.z; + float waterMask = normalsWater.w;Then update line 66:
- reactiveMask = max(reactiveMask, NormalsWaterMask[dispatchID.xy].w); + reactiveMask = max(reactiveMask, waterMask);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
features/Upscaling/Shaders/Upscaling/EncodeTexturesCS.hlsl(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:
features/Upscaling/Shaders/Upscaling/EncodeTexturesCS.hlsl
features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh}: Place all feature shaders under features/YourFeature/Shaders/
Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)
Files:
features/Upscaling/Shaders/Upscaling/EncodeTexturesCS.hlsl
🧠 Learnings (1)
📓 Common learnings
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.
⏰ 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: Build plugin and addons
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
|
✅ A pre-release build is available for this PR: |
|
@doodlum does this make sense? |
alandtse
changed the title
|
@ItsAdeline Please ensure you allow edits by contributors or run pre-commit yourself. |
|
I have edits enabled @alandtse |
No. The transparency mask is a less aggressive form of the reactive mask, per relevant documentation. This does not fix any known issues and there is no explanation why it would. Reactive elements should be added properly either by modifying shaders or adding new heuristic detection methods, e.g. checking before/after transparency passes. Transparency elements are ones that aren't alpha effects exactly. Generally, for specular reflections. So water, SSR, etc. when you want things to be denoised and upscaled but reactive to things like a moving camera. Reactive is practically just lerping the result. On something like water it would look lower res, flicker. DLSS will try to avoid this, FSR and XeSS will look pixelated. If OP enables debug views for FSR/DLSS and shows that scene elements are added to the mask by this change I will review. |
testttt.-.Made.with.Clipchamp.1.mp4 |
|
your change definitely adds this? |
|
it should, These are the only 2 lines I added aside from the debug view. |
|
my bad |
3 participants
2025-10-16.12-18-59.mp4
This pull request modifies the EncodeTexturesCS.hlsl compute shader to incorporate additional scene
information into the reactiveMask output.
The specific lines in question are:
reactiveMask = max(reactiveMask, NormalsWaterMask[dispatchID.xy].w);
reactiveMask = max(reactiveMask, transparencyCompositionMask);
Details:
The reactiveMask is an input to temporal upscaling algorithms (e.g., DLSS, FSR) used to guide temporal
accumulation and reconstruction.
The first line integrates the w component of NormalsWaterMask, which typically represents water surface identification, into the reactiveMask. I don't think this one is a big issue but thought to add it in anyways.
The second line integrates the transparencyCompositionMask, which identifies transparent scene elements,
into the reactiveMask.
This integration provides the upscaling algorithm with additional per-pixel context regarding water and
transparency. This data can be utilized by the upscaler to apply specific temporal filtering or
reconstruction strategies to pixels identified as belonging to water surfaces or transparent objects. The
objective is to enable more granular control over the upscaling process in these specific regions of the
rendered frame.
Summary by CodeRabbit