feat: advanced skin#2428
Conversation
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This allows using the build script for scripting and testing.
) * ci: treat skipped hlsl validation as success * chore: trigger cpp validation * ci: allow cpp success without changes * revert: "chore: trigger cpp validation" This reverts commit dafb4b9.
…s#1099) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* feat: add raindrop ripples on water * style: 🎨 apply clang-format changes * feat: ripple effect now supports water parallax * style: 🎨 apply clang-format changes * fix: fix compilation errors * fix: fix wrong type Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * chore: sync cache on defaults Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * style: 🎨 apply pre-commit.ci formatting Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details. * fix: vanilla ripple display * refactor: add flowmap functions * feat: add flow maps to ripples * style: 🎨 apply pre-commit.ci formatting Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details. * fix: fix ripple and splash flows * fix: remove dupe GetFeatureModLink * style: fix lint * refactor: address further ai comments * fix: avoid entropy collapse in ripple hash * style: restore function names and lines --------- Co-authored-by: TheRiverwoodModder <TheRiverwoodModder@users.noreply.github.com> Co-authored-by: Alan Tse <alandtse@users.noreply.github.com> Co-authored-by: Alan Tse <alandtse@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Full Icon framework & support. * UI Text helper function for better looking large text Function for crispier large text like titles. Helps subtly but does make a difference, especially when high-res icons are visible near text. * Update Menu.cpp * Update src/UIIconLoader.cpp Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update src/UIIconLoader.cpp Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Icon folder changes, fix for compile issue. * Reorganise * Update README.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fixes * rabbit suggested fixes * fix * style: 🎨 apply pre-commit.ci formatting Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details. * build fix * Fixes * Update Menu.cpp * style: 🎨 apply pre-commit.ci formatting Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details. * Licence tweaks * Comment clean & Mipmap fixes * Texture memory leak cleanup * fixes * style: 🎨 apply pre-commit.ci formatting Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details. * style: 🎨 apply pre-commit.ci formatting Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
refactor(skin): extract LoadSkinDetailTexture method and update resource handling
Replace the per-pixel world-Z water line comparison with a per-bone wetness history system that remembers which body parts were submerged. - Track per-bone wetness on CPU using NiSkinInstance bone world positions - Interpolate bone wetness in VS using skeletal animation weights (TEXCOORD13) - Gradual evaporation via configurable EvaporationRate / WetFadeTime - Fallback to original Z-comparison for non-skinned CS_SKIN meshes - New shared SkinData.hlsli for VS/PS cbuffer at register b7 Fixes water line following model movement during pose changes.
Two bugs in bone-anchored wetness: 1. Early-out checked skinPerGeometry.y == 0 without considering bone data, killing wetness even when bones still had residual moisture. 2. waterWet = skinPerGeometry.y * boneWetnessInterp caused bone wetness to be multiplied by the global y fade (2.0 -> 0 over WetFadeTime). When y reached 0, all bone data was zeroed out prematurely. This also broke clothing/armor (SKINNED but not SKIN shaders). Fix: bone path now uses fixed intensity (2.0) and relies solely on CPU-side per-bone evaporation for temporal fade. The global y fade only applies to the fallback Z-comparison path.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an Advanced Skin feature: shader utilities (SSS, BRDF, normals, procedural wetness), lighting-path integration, a C++ Skin feature (settings, hooks, per-geometry wetness caching, extra texture derivation), a DynamicWetness public API header, and engine wiring/registering for runtime binding. ChangesSkin Rendering Feature
sequenceDiagram
participant GameEngine
participant SkinFeature
participant DynamicWetnessDLL
participant PixelShader
GameEngine->>SkinFeature: PostPostLoad / Install hooks
SkinFeature->>DynamicWetnessDLL: Init() / Query wetness (GetFinalWetness)
PixelShader->>SkinFeature: Request wetness / skin params (GetWetness)
SkinFeature->>PixelShader: Bind SRVs + Per-geometry CB (SkinPerGeometry)
PixelShader->>GameEngine: Render with modified material/wetness
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.14][ERROR]: Error: exception Unix_error: No such file or directory stat src/Features/Skin.cpp 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 |
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
features/Skin/Shaders/Features/Skin.ini (1)
1-2: Suggested PR metadata polishConsider updating the PR title to a stricter Conventional Commit form, e.g.
feat(skin): add advanced skin shading, and add a description footer likeImplements#2428(or `Addresses `#2428) for auto-linking semantics.As per coding guidelines, "When reviewing PRs, please provide suggestions for Conventional Commit Titles ... and Issue References ...".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/Skin/Shaders/Features/Skin.ini` around lines 1 - 2, Update the PR metadata to follow Conventional Commits by changing the PR title to a stricter format such as "feat(skin): add advanced skin shading" and append an issue reference footer like "Implements `#2428`" or "Addresses `#2428`" in the PR description; this applies to the submission touching Features/Skin/Shaders/Features/Skin.ini (see the [Info] Version entry) so the commit/PR title and description include the conventional prefix and the issue footer for auto-linking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@features/Skin/Shaders/Skin/Skin.hlsli`:
- Around line 190-193: The quaternion computation for reorienting normals can
divide by zero when dot(s, t) ≈ -1; replace the single-line q = float4(...) /
sqrt(...) with a guarded computation: compute float d = dot(s,t); if (2*(d+1) is
below an epsilon) build a 180° quaternion by choosing an arbitrary axis
orthogonal to s (e.g., axis = normalize(cross(s, (abs(s.x) < 0.9) ?
float3(1,0,0) : float3(0,1,0))); q = float4(axis, 0)); otherwise compute q as
before using denom = sqrt(2*(d+1)) and the existing formula; keep the subsequent
rotation formula that uses q.w and q.xyz unchanged. This change should be
applied where q (and variables s, t, u) are computed/used in Skin.hlsli.
- Around line 165-166: The diffuse lobe calculation subtracts only specular.x
and specular.y from all color channels, which ignores the blue channel and can
go negative; change the line using the per-channel specular weights and clamp
the result: set lobeWeights.diffuse = material.BaseColor * saturate(1.0 -
lobeWeights.specular) (using HLSL saturate or max to ensure non-negative
components) so the subtraction is done component-wise against
lobeWeights.specular and cannot produce negative values.
In `@package/Shaders/Lighting.hlsl`:
- Around line 1900-1903: The fragment shader is calling
TexSkinExtraSampler.GetDimensions and TexSkinWetnessSampler.GetDimensions
per-pixel which is expensive; remove those runtime dimension queries and instead
source hasSkinExtra and hasSkinWetness from constant data or binding-time state
(e.g., a material/feature flag in the constant buffer) or by binding known
fallback textures so availability is known at draw time; update the shader to
stop using
TexSkinExtraSampler.GetDimensions/TexSkinWetnessSampler.GetDimensions, read
hasSkinExtra/hasSkinWetness from the provided constant (or sampler binding
presence), and ensure any code that sets these flags at draw/dispatch time is
adjusted to populate the constant buffer or bind the fallback textures
accordingly.
In `@src/Features/Skin.cpp`:
- Around line 318-339: The water-height lookup in the
get_nearest_water_object_height lambda (used to set waterHeight inside
GetWetness(), which is called from BSLightingShader_SetupGeometry()) iterates
waterManager->waterObjects and multiBounds per draw; move this work out of the
geometry hook by computing and caching the nearest water height once per actor
per frame (or during a prepass/update step) and reading the cached value in
GetWetness(); implement a per-frame cache keyed by the actor (or shader owner)
plus a frame counter to invalidate each frame, populate it from the
prepass/update function (or actor update hook) and replace the inline walk
inside get_nearest_water_object_height with a simple cached lookup, and apply
the same change to the similar code around the referenced 367-370 region.
- Around line 610-614: The thunk BSLightingShader_SetupGeometry::thunk currently
always calls into skin.BSLightingShader_SetupGeometry(Pass) and can dereference
a null PerGeometryCB if SetupResources hasn't run; guard this by checking the
skin feature is initialized and PerGeometryCB is non-null (e.g., a boolean like
skin.initialized or directly skin.PerGeometryCB) before calling
skin.BSLightingShader_SetupGeometry(Pass), and if the check fails simply call
and return func(This, Pass, RenderFlags) without touching PerGeometryCB;
reference the thunk BSLightingShader_SetupGeometry::thunk, the member
skin.BSLightingShader_SetupGeometry, the SetupResources-produced PerGeometryCB,
and the original func to implement the guard.
- Around line 390-403: The fade math uses 1.0f / settings.WetFadeTime which can
divide by zero; before any use (e.g., around the blocks updating
cached.x/cached.y/wetness.x/wetness.w) clamp or sanitize settings.WetFadeTime to
a safe minimum (e.g., replace with max(settings.WetFadeTime, 0.0001f) or compute
a local float wetFadeInv = settings.WetFadeTime > 0 ? 1.0f /
settings.WetFadeTime : 1.0f / 0.0001f) and use that local value in the
subtraction expressions so the code that updates cached and wetness never
performs 1.0f / 0.0f.
In `@src/Features/Skin.h`:
- Line 30: SupportsVR() currently returns true unconditionally while
BSLightingShader vfunc hooks are installed with fixed slots; change this so
SupportsVR() performs runtime variant detection and only returns true when VR
layout is detected, and update the hook installation to use REL::RelocateMember
(or equivalent runtime relocation) to resolve the BSLightingShader vfunc slots
per-variant before patching; specifically modify SupportsVR(), the
BSLightingShader hook setup code, and any installer that assumes fixed slots so
they use the runtime-detection pattern and REL::RelocateMember to select correct
vfunc indices for SE/AE/VR.
---
Nitpick comments:
In `@features/Skin/Shaders/Features/Skin.ini`:
- Around line 1-2: Update the PR metadata to follow Conventional Commits by
changing the PR title to a stricter format such as "feat(skin): add advanced
skin shading" and append an issue reference footer like "Implements `#2428`" or
"Addresses `#2428`" in the PR description; this applies to the submission touching
Features/Skin/Shaders/Features/Skin.ini (see the [Info] Version entry) so the
commit/PR title and description include the conventional prefix and the issue
footer for auto-linking.
🪄 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 Plus
Run ID: fa779de6-f973-4591-ada1-470ee0bfa216
⛔ Files ignored due to path filters (1)
features/Skin/Shaders/Skin/skin_detail_n.ddsis excluded by!**/*.dds
📒 Files selected for processing (19)
features/Skin/Shaders/Features/Skin.inifeatures/Skin/Shaders/Skin/Skin.hlsliinclude/DynamicWetness_PublicAPI.hpackage/Shaders/Common/LightingCommon.hlslipackage/Shaders/Common/LightingEval.hlslipackage/Shaders/Common/SharedData.hlslipackage/Shaders/Lighting.hlslsrc/Feature.cppsrc/FeatureBuffer.cppsrc/Features/Skin.cppsrc/Features/Skin.hsrc/Features/TerrainHelper.cppsrc/Features/TerrainHelper.hsrc/Globals.cppsrc/Globals.hsrc/Hooks.cppsrc/State.cppsrc/TruePBR.cppsrc/TruePBR.h
|
✅ A pre-release build is available for this PR: |
|
looks good, please address AI |
This pull request introduces a new skin shading feature, adding both configuration and shader logic for advanced skin rendering. The main changes are the creation of a skin shader HLSL include file implementing subsurface scattering, dual specular lobes, fuzz, wetness, and detail normal blending, as well as a new configuration file for version tracking.
Skin shading feature implementation:
Skin/Skin.hlslicontaining a comprehensive namespace with functions for skin-specific lighting, subsurface scattering, dual specular GGX, fuzz, wetness, detail normal blending, and procedural noise for effects like sweat. This file leverages multiple shared shader utilities and implements techniques from recent rendering research.Configuration and versioning:
[Info]section with a version number inSkin/Shaders/Features/Skin.inito track the feature's version.Summary by CodeRabbit