chore(wetness effects): wetness improvements and add fresnel#1468
Conversation
WalkthroughThis PR overhauls wetness/raindrop shading: rewrites rain/ripple timing and normals, replaces a custom reflectance with Schlick Fresnel, adjusts deferred normal reconstruction for dynamic cubemaps, and integrates unified shore/puddle/rain wetness into the lighting PS path. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant PS as Lighting.hlsl (PS)
participant WE as WetnessEffects.hlsli
participant G as GBuffer/Masks
rect rgb(245,248,255)
note over PS: Per-pixel wetness setup
PS->>PS: Compute shoreFactor, puddle, wetnessOcclusion, flatnessAmount
end
alt shouldCalculateRaindrops
PS->>WE: GetRainDrops(ripplePos, flatnessAmount, worldNormal, time)
WE-->>PS: float4(rippleNormal, rainWetnessFactor)
else skipRaindrops
PS-->>PS: Default rippleNormal=(0,0,1), rain factor=0
end
rect rgb(240,255,245)
note over PS: Compose final wetness and normals
PS->>PS: rainWetness = f(raindropInfo)
PS->>PS: wetness = max(shoreWetness, rainWetness)
PS->>PS: wetnessNormal = ReorientNormal(mix((0,0,1), rippleNormal, flatness))
end
PS->>WE: GetWetnessAmbientSpecular(wetness, roughness)
PS->>G: Output masks (wetnessNormal.z remapped)
sequenceDiagram
autonumber
participant CS as DeferredCompositeCS.hlsl
participant Tex as MasksTexture
participant R as ReflectionFlow
CS->>Tex: sample = MasksTexture[dispatchID.xy].z
CS->>CS: wetnessMask = sample * 2.0 - 1.0
CS->>CS: normalWS.z = wetnessMask
CS->>CS: xyLength = sqrt(max(0.0, 1 - wetnessMask^2))
CS->>CS: normalWS.xy = normalize(normalWS.xy) * xyLength
CS->>R: Use reconstructed normalWS for reflection calc
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Poem
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.
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. Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
|
✅ A pre-release build is available for this PR: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli (2)
31-35: Guard against antiparallel normals in shortest-arc quaternion reorientationWhen dot(s, t) ≈ -1, the denominator sqrt(2*(dot+1)) hits 0, producing INF/NaN. Add an epsilon fallback.
- float4 q = float4(cross(s, t), dot(s, t) + 1) / sqrt(2 * (dot(s, t) + 1)); + float d = dot(s, t); + float denom = sqrt(max(2.0 * (d + 1.0), 1e-6)); + float4 q = float4(cross(s, t), d + 1.0) / denom;
273-290: Make flow ripple timing time-dependent (use SharedData::wetnessEffectsSettings.Time)frac(flowTimeScale) has no time component — the flow offset will be static. SharedData::wetnessEffectsSettings.Time is used elsewhere (Water.hlsl / Lighting.hlsl), so multiply by Time.
File: features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli:273-290
- float smoothTime = smoothstep(0.0, 1.0, frac(flowTimeScale)); + float time = SharedData::wetnessEffectsSettings.Time; + float smoothTime = smoothstep(0.0, 1.0, frac(time * flowTimeScale));
🧹 Nitpick comments (9)
features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli (5)
52-57: Nit: use exact 1/2^32 constantUse 1.0/4294967296.0f for unbiased uint-to-[0,1) mapping and reserve 4294967295 for “max-uint” checks.
- float uintToFloat = rcp(4294967295.0); + float uintToFloat = rcp(4294967296.0); // 1/2^32
100-137: Ripple neighborhood hot path: avoid sqrt in inner loop and tighten band testComputing sqrt(distSqr) each tap is costly. Compare squared distances against squared radii and derive bandLerp without sqrt using rsqrt approximation.
- float bandLerp = (sqrt(distSqr) - rippleInnerRadius) * rippleBreadthRcp; + float dist = distSqr * rsqrt(max(distSqr, 1e-6)); // approx distance + float bandLerp = (dist - rippleInnerRadius) * rippleBreadthRcp;
123-134: Normal construction algebra is fragilegrad/bitangent derived from each other can degenerate; cross(grad, bitangent) can lose precision. Use an explicit frame: T = normalize(float3(vec2Centre, 0)), B = float3(-T.y, T.x, 0), N = normalize(float3(0,0,1) - deriv * T).
- float3 grad = float3(normalize(vec2Centre), -deriv); - float3 bitangent = float3(-grad.y, grad.x, 0.0); - float3 normal = normalize(cross(grad, bitangent)); + float2 t2 = normalize(vec2Centre); + float3 T = float3(t2, 0.0); + float3 normal = normalize(float3(0,0,1) + deriv * float3(-T.x, -T.y, 0));
163-169: Expose F0 as a parameter or setting; clip NoV domainHard-coding F0=0.02 limits materials. Also, numerical stability improves with NoV clamping to [1e-4, 1].
- float3 F0 = 0.02; - float3 F = F0 + (1.0 - F0) * pow(1.0 - NoV, 5.0); + float3 F0 = SharedData::wetnessEffectsSettings.F0.xxx; // default to 0.02 in settings + float NoV5 = pow(saturate(1.0 - max(NoV, 1e-4)), 5.0); + float3 F = F0 + (1.0 - F0) * NoV5;
206-223: Make debug threshold configurableHard-coded 0.01 prevents tuning. Consider a SharedData setting or a function parameter.
package/Shaders/Lighting.hlsl (4)
2221-2229: Occlusion computation squared; verify intended response curvepow(..,2) darkens aggressively. Consider exposing exponent in settings if you observe over-occlusion.
2231-2236: Raindrop gating is too binary and angle-biased
- worldNormal.z > 0.0 excludes steep/vertical but still rain-exposed surfaces.
- Hard cutoff wetnessOcclusion > 0.5 drops entire effect per pixel.
Use continuous weights.- bool shouldCalculateRaindrops = (worldNormal.z > 0.0) && - (SharedData::wetnessEffectsSettings.Raining > 0.0) && - (SharedData::wetnessEffectsSettings.EnableRaindropFx) && - (wetnessOcclusion > 0.5); + float raindropWeight = saturate(worldNormal.z) * saturate(wetnessOcclusion); + bool shouldCalculateRaindrops = SharedData::wetnessEffectsSettings.EnableRaindropFx && + SharedData::wetnessEffectsSettings.Raining > 0.0 && + (raindropWeight > 1e-3);
2264-2271: Puddle derivation can exceed [0,1] and lacks shaping control
- Clamp puddle after modulation.
- Consider a controllable remap curve for artistic control.
- puddle *= lerp(wetness, puddleWetness, saturate(puddle - 0.25)); + puddle *= lerp(wetness, puddleWetness, saturate(puddle - 0.25)); + puddle = saturate(puddle);
2285-2290: Ripple normal mix: simplify and avoid redundant normalizelerp(flatnessAmount, 1.0, 0.5) is constant 0.5*(1+flatnessAmount). Also normalize after ReorientNormal only once.
- float3 rippleNormal = normalize(lerp(float3(0, 0, 1), raindropInfo.xyz, lerp(flatnessAmount, 1.0, 0.5))); - wetnessNormal = WetnessEffects::ReorientNormal(rippleNormal, wetnessNormal); + float k = 0.5 * (1.0 + flatnessAmount); + float3 rippleNormal = lerp(float3(0,0,1), raindropInfo.xyz, k); + wetnessNormal = normalize(WetnessEffects::ReorientNormal(rippleNormal, wetnessNormal));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli(4 hunks)package/Shaders/DeferredCompositeCS.hlsl(1 hunks)package/Shaders/Lighting.hlsl(3 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:
package/Shaders/DeferredCompositeCS.hlslpackage/Shaders/Lighting.hlslfeatures/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli
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/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli
🧠 Learnings (3)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#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.
📚 Learning: 2025-08-03T18:37:19.690Z
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.
Applied to files:
package/Shaders/DeferredCompositeCS.hlslpackage/Shaders/Lighting.hlsl
📚 Learning: 2025-07-18T15:21:03.641Z
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-18T15:21:03.641Z
Learning: In the skyrim-community-shaders rendering pipeline, materials with alpha < 1 or alpha blending enabled are rendered in non-deferred mode rather than deferred mode. This means issues with dynamic cubemaps on transparent materials are actually non-deferred rendering issues.
Applied to files:
package/Shaders/DeferredCompositeCS.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: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Build plugin and addons
🔇 Additional comments (6)
features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli (2)
73-99: Fix integer casting and RNG consistency in WetnessEffects.hlsliFile: features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli (approx. lines 73–99). Replace reinterpret-style asuint on an int2 with an explicit uint2 conversion and use the pcg RNG family consistently for the radius sample.
- uint3 hash = Random::pcg3d(uint3(asuint(gridCurr), timestep)); + uint3 hash = Random::pcg3d(uint3(uint2(gridCurr), timestep)); - float3 floatHash = float3(hash) * uintToFloat; + float3 floatHash = float3(hash) * uintToFloat; - float dropRadius = lerp(SharedData::wetnessEffectsSettings.SplashesMinRadius, - SharedData::wetnessEffectsSettings.SplashesMaxRadius, - float(Random::iqint3(hash.yz)) * uintToFloat); + float dropRadius = lerp(SharedData::wetnessEffectsSettings.SplashesMinRadius, + SharedData::wetnessEffectsSettings.SplashesMaxRadius, + float(Random::pcg2d(hash.yz).x) * uintToFloat);
6-7: Verify t70 binding does not collidefeatures/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli is not present in this branch and no occurrences of "register(t70)" were found; if TexPrecipOcclusion will use register(t70) confirm it does not conflict with other texture registers across package/Shaders and feature shaders.
package/Shaders/Lighting.hlsl (4)
2211-2220: Initialize wetness parameters: OKClear initialization and separation of albedo/specular/normal channels looks good.
2238-2246: Space choice for ripplePositionMixing model-space for SKINNED and world-space otherwise changes temporal stability across animated meshes vs statics. Confirm it’s intentional; if not, prefer a consistent projected world anchor (e.g., object/world space hashed grid).
2291-2292: Roughness mapping change: confirm energy balanceDropping the 0.9 factor boosts specular. Verify it doesn’t over-brighten wet surfaces under multiple lights.
3365-3373: Masks.z now encodes reoriented wetnessNormal.z — confirm all readers
- Verified: package/Shaders/Lighting.hlsl encodes psout.Masks.z = wetnessNormal.z * 0.5 + 0.5; package/Shaders/DeferredCompositeCS.hlsl decodes MasksTexture[dispatchID.xy].z * 2.0 - 1.0 and uses it (matches).
- Audit other G-buffer writers that set psout.Masks.z for semantic mismatches: package/Shaders/Effect.hlsl (sets to luminance), package/Shaders/DistantTree.hlsl (z = 1), package/Shaders/RunGrass.hlsl (z = 0), plus any other psout.Masks assignments.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/Features/WetnessEffects.h (2)
34-34: Confirm shader-side clamping for Shore Wetness now that default is 1.0.If the shader assumes [0,1], ensure we clamp/saturate the value when packing PerFrame to guard against bad config JSON. Consider a defensive clamp in GetCommonBufferData.
Apply this diff in GetCommonBufferData (cpp) to be safe:
- data.settings.MaxShoreWetness = settings.EnableWetnessEffects ? settings.MaxShoreWetness : 0.0f; + data.settings.MaxShoreWetness = settings.EnableWetnessEffects + ? std::clamp(settings.MaxShoreWetness, 0.0f, 1.0f) + : 0.0f;
50-51: Minor UX nit: defaults are at slider max.RaindropInterval=2.0 and Chance=1.0 place the sliders fully right on first open. Consider slightly lower defaults (e.g., 1.5f, 0.8f) for headroom while tweaking; presets can still push to 1.0.
src/Features/WetnessEffects.cpp (4)
95-98: Update copy to reflect “continuous” drops at 100% chance.“100% chance” implies every interval spawns a drop per cell. Suggest “Continuous raindrops” instead of “Moderate,” to avoid confusion with lower-frequency wording elsewhere.
- "Raindrop: 100% chance, grid 3.0 units, interval 0.3s.", + "Raindrop: 100% chance (continuous), grid 3.0 units, interval 0.3s.",
100-105: Align effects description with 100% chance.“Moderate raindrop frequency (100% chance)” is contradictory. Recommend “Continuous raindrops (100% chance).”
- "Moderate raindrop frequency (100% chance)", + "Continuous raindrops (100% chance)",
772-783: Harden PerFrame packing against invalid inputs (NaNs/Inf).Besides zero, negative or NaN inputs would propagate to shaders. Consider sanitizing with finite checks and clamps when packing.
Example:
+ if (!std::isfinite(settings.RaindropGridSize)) settings.RaindropGridSize = 4.0f; + if (!std::isfinite(settings.RaindropInterval)) settings.RaindropInterval = 0.5f;
341-365: Safer Checkbox handling for uint-backed flags.Casting uint* to bool* invokes UB. Use a temporary bool and write back. Repeat for other flags.
- if (ImGui::Checkbox("Enable Wetness", (bool*)&settings.EnableWetnessEffects)) { + bool wet = settings.EnableWetnessEffects != 0u; + if (ImGui::Checkbox("Enable Wetness", &wet)) { + settings.EnableWetnessEffects = wet ? 1u : 0u; Ripples::UpdateSettings(); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Features/WetnessEffects.cpp(2 hunks)src/Features/WetnessEffects.h(2 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/WetnessEffects.hsrc/Features/WetnessEffects.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/WetnessEffects.hsrc/Features/WetnessEffects.cpp
🧠 Learnings (2)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#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.
📚 Learning: 2025-06-24T07:17:36.604Z
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
Applied to files:
src/Features/WetnessEffects.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). (3)
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Build plugin and addons
🔇 Additional comments (1)
src/Features/WetnessEffects.cpp (1)
159-160: Include raindrop grid & interval in preset detectionApplyClimatePreset sets raindropGridSize and raindropInterval but DoesCurrentSettingsMatchPreset currently compares only chance and the three multipliers — add grid and interval comparisons with a small tolerance.
File: src/Features/WetnessEffects.cpp — DoesCurrentSettingsMatchPreset
bool WetnessEffects::DoesCurrentSettingsMatchPreset(ClimatePreset preset) const { if (preset == ClimatePreset::Custom) { return false; } const auto& climate = GetClimateSettings(preset); Settings defaultSettings{}; float expectedMaxRainWetness = defaultSettings.MaxRainWetness * climate.wetnessMultiplier; float expectedMaxPuddleWetness = defaultSettings.MaxPuddleWetness * climate.puddleMultiplier; float expectedWeatherTransitionSpeed = defaultSettings.WeatherTransitionSpeed * climate.transitionSpeed; float expectedRaindropChance = climate.raindropChance; + float expectedGridSize = climate.raindropGridSize; + float expectedInterval = climate.raindropInterval; const float tolerance = 0.001f; - return (std::abs(settings.MaxRainWetness - expectedMaxRainWetness) < tolerance && - std::abs(settings.MaxPuddleWetness - expectedMaxPuddleWetness) < tolerance && - std::abs(settings.WeatherTransitionSpeed - expectedWeatherTransitionSpeed) < tolerance && - std::abs(settings.RaindropChance - expectedRaindropChance) < tolerance); + return (std::abs(settings.MaxRainWetness - expectedMaxRainWetness) < tolerance && + std::abs(settings.MaxPuddleWetness - expectedMaxPuddleWetness) < tolerance && + std::abs(settings.WeatherTransitionSpeed - expectedWeatherTransitionSpeed) < tolerance && + std::abs(settings.RaindropChance - expectedRaindropChance) < tolerance && + std::abs(settings.RaindropGridSize - expectedGridSize) < tolerance && + std::abs(settings.RaindropInterval - expectedInterval) < tolerance); }Verification: sandbox search failed ('src' not found) — manual verification required.
doodlum
changed the title
1 participant
Summary by CodeRabbit
New Features
Improvements
Chores