feat: add Extended Translucency#678
Conversation
|
This looks really interesting. It would be best to include this as an option in the JSON configs for True PBR like we did with the glints PR #413 that way different materials can have different translucency types, rather than every alpha effect getting this. Otherwise random translucent objects could look incorrect which would cause issues. True PBR actually has fabric models too, so could maybe implement that without even needed new config options. You would however need to convert assets to PBR to be able to test the model. @Jonahex is in charge of all that |
|
👍 I'll be working on the new nif flags, thanks for the reference! |
ArcEarth
force-pushed
the
extended_transcluency
branch
2 times, most recently
from
c84f52b to
8423cc5
Compare
|
@doodlum The settings are now detected from ExtraData node in the mesh's NIF. Also, I kept the option to apply this effect to everything, so that existing assets can be improved from this feature immediately. One future improvement is to make some TESForm based selection like using KID or alkies. Besides, added another option 'Stregth' to the setting, which is essentially the blending weight (alpha) of this effect. |
|
Looks really good so far. Just a few minor performance related things and I think then it's probably good to go. |
|
|
||
| ID3D11DeviceContext* context = reinterpret_cast<ID3D11DeviceContext*>(renderer->GetRuntimeData().context); | ||
| ID3D11Buffer* buffers[1]; | ||
| if (auto* data = pass->geometry->GetExtraData(NiExtraDataName)) { |
There was a problem hiding this comment.
Not sure how good for perf it to look for ExtraData in each PerGeometry
There was a problem hiding this comment.
I think it should be fine compared to the cost of updating constant buffer:
The ExtraData is empty on most geometry, and for others with a small amount of extra data, it's a linear/binary lookup on an array of char*, the compare is pointer based instead of strcmp.
I tried to reuse some shader flags, like VertexAlpha, but found there are lot of random assets using them 😂
|
@doodlum @Jonahex @FlayaN |
It is probably fine for now. Soon we will probably scrap all additional constant buffers. State switching and updates need to be minimised but it only matters when it affects a significant number of calls. So two more things: |
ArcEarth
force-pushed
the
extended_transcluency
branch
from
810a105 to
7d51a1d
Compare
|
Updated for the refactored light flag :) |
|
@doodlum Anything else to be addressed before we can merge? :) Beside, wondering if we have some detail about the mod release process, if I or the team should upload the feature mod? Also, the raw Lighting.hlsl edit can be release as a replacer for the current stable version of CS (without settings or nif settings), wondering if we should release it now or release it with the next version? |
WalkthroughThis update introduces a new "Extended Translucency" feature to the rendering pipeline. It adds shader utilities, configuration, and runtime logic for handling advanced translucency effects, including anisotropic alpha materials. The implementation includes new shader code, configuration files, global integration, feature registration, buffer updates, UI controls, serialization support, and integration with the rendering pipeline via hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant ExtendedTranslucency
participant State
participant Shader
User->>UI: Adjust translucency settings (ImGui)
UI->>ExtendedTranslucency: Update MaterialParams
ExtendedTranslucency->>State: Update global feature descriptor
State->>Shader: Pass updated settings via buffer
Shader->>Shader: Use new alpha/material model logic in rendering
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (11)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
package/Shaders/Common/SharedData.hlsli (1)
186-192: Ensure HLSL–C++ constant buffer alignment
The newExtendedTranslucencySettingsstruct and its insertion intoFeatureDatamust exactly match the layout and size of the corresponding C++ struct inFeatureBuffer.cpp.
Please verify with a C++static_asserton the buffer size and field offsets or add explicit padding if necessary.Also applies to: 208-208
🧹 Nitpick comments (8)
src/State.h (1)
154-154: Fix typo in comment
The comment reads “ExtendedTransluceny” instead of “ExtendedTranslucency.” Please correct the spelling to avoid confusion.- uint ExtraFeatureDescriptor; // TerrainHelper : 6, ExtendedTransluceny : 3 + uint ExtraFeatureDescriptor; // TerrainHelper : 6, ExtendedTranslucency : 3package/Shaders/Common/Permutation.hlsli (1)
82-82: Fix spelling typo in comment
The inline comment reads "ExtendedTransluceny"—it should be "ExtendedTranslucency" to match the feature name and avoid confusion.- uint ExtraFeatureDescriptor; // TerrainHelper : 6, ExtendedTransluceny : 2 + uint ExtraFeatureDescriptor; // TerrainHelper : 6, ExtendedTranslucency : 2features/Extended Translucency/Shaders/ExtendedTranslucency/ExtendedTranslucency.hlsli (3)
23-27: Consider extracting magic constant
The0.001offset prevents division by zero but is hard-coded. You might define a named constant (e.g.,kMinDotEpsilon) for clarity and easy tuning.
28-36: Review 2D fabric alpha formula
TheGetViewDependentAlphaFabric2Dfunction is mathematically sound. If performance becomes critical, consider precomputinga0outside high-frequency loops.
38-43: Guard against zero or negative limits
CurrentlySoftClampdivides bylimitwithout checking for zero. Adding a check prevents potential NaNs or infinities whenlimit == 0.float SoftClamp(float alpha, float limit) { if (limit <= 0) { return 0; } alpha = min(alpha, limit / (1 + exp(-4 * (alpha - limit*0.5) / limit))); return saturate(alpha); }src/Features/ExtendedTranslucency.cpp (3)
7-8: Unused compile-time switch – remove or make it meaningful
EXTENDED_TRANSLUCENCY_PER_MATERIAL_BUFFERis defined but never referenced inside the TU (and I can’t find any reference in the other files from this PR). Dead macros make the intent unclear and risk bit-rot.
94-114: UI slider wired to a non-persisted value
Even after fixing the serialisation list,AlphaStrengthdefaults to 0 every run because the struct default is0.f. Consider initialising it to a sensible value (e.g.1.f) so users immediately see an effect.
66-69: Hook installation: missing failure handling
Ifwrite_vfuncfails (e.g. table layout differs on AE/VR),Install()will silently succeed and the feature will act unpredictably. Emitting an error or returning a boolean would aid debugging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
package/Shaders/Lighting.hlslis excluded by!**/*.hlsl
📒 Files selected for processing (12)
features/Extended Translucency/Shaders/ExtendedTranslucency/ExtendedTranslucency.hlsli(1 hunks)features/Extended Translucency/Shaders/Features/ExtendedTranslucency.ini(1 hunks)package/Shaders/Common/Permutation.hlsli(1 hunks)package/Shaders/Common/SharedData.hlsli(2 hunks)src/Feature.cpp(2 hunks)src/FeatureBuffer.cpp(2 hunks)src/Features/ExtendedTranslucency.cpp(1 hunks)src/Features/ExtendedTranslucency.h(1 hunks)src/Features/TerrainHelper.cpp(1 hunks)src/Globals.cpp(3 hunks)src/Globals.h(2 hunks)src/State.h(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Globals.cpp (1)
src/Features/ExtendedTranslucency.cpp (2)
GetSingleton(17-21)GetSingleton(17-17)
🔇 Additional comments (15)
features/Extended Translucency/Shaders/Features/ExtendedTranslucency.ini (1)
1-2: INI version descriptor looks good
The newExtendedTranslucency.inifile provides a clear version anchor for the feature.src/Features/TerrainHelper.cpp (1)
153-153: Helpful reminder for bit‐allocation maintenance
The newly added comment makes it clear where to update the bit allocation if you change the texture‐slot count. This is a useful guide for future maintenance.src/FeatureBuffer.cpp (2)
6-6: Include ExtendedTranslucency feature header
The new include forExtendedTranslucency.his correctly placed alongside the other feature headers.
49-50:Details
✅ Verification successful
Integrate ExtendedTranslucency settings into buffer data
You’ve appendedextendedTranslucency->settingsto the feature‐buffer payload. Ensure that the order here matches the layout defined in the HLSLFeatureDatastructure to avoid mismatches between CPU and GPU data.Run this to confirm the sequence in your shared-data header:
🏁 Script executed:
#!/bin/bash # Find the FeatureData struct and show its fields, including ExtendedTranslucencySettings file=$(fd SharedData.hlsli) rg -n 'struct FeatureData' -n 'ExtendedTranslucencySettings' "$file"Length of output: 239
🏁 Script executed:
#!/usr/bin/env bash # Locate the SharedData HLSL file and dump the FeatureData struct file=$(fd SharedData.hlsli) if [[ -z "$file" ]]; then echo "Error: SharedData.hlsli not found" exit 1 fi echo "=== struct FeatureData in $file ===" sed -n '/struct FeatureData/,/};/p' "$file"Length of output: 347
🏁 Script executed:
#!/usr/bin/env bash # Dump the FeatureData struct with ample context to inspect its fields file=$(fd SharedData.hlsli) if [[ -z "$file" ]]; then echo "Error: SharedData.hlsli not found" >&2 exit 1 fi echo "=== Showing struct FeatureData with 50 lines after header ===" rg -n 'struct FeatureData' -A50 "$file"Length of output: 327
🏁 Script executed:
#!/usr/bin/env bash # Locate SharedData.hlsli and inspect struct definitions and FeatureData occurrences file=$(fd SharedData.hlsli) if [[ -z "$file" ]]; then echo "Error: SharedData.hlsli not found" >&2 exit 1 fi echo "=== Occurrences of 'FeatureData' in $file ===" rg -n 'FeatureData' "$file" || echo "(none found)" echo echo "=== Top-level struct definitions in $file ===" rg -n '^struct' -A20 "$file" || echo "(none found)"Length of output: 670
🏁 Script executed:
#!/usr/bin/env bash # Extract cbuffer FeatureData from SharedData.hlsli to inspect field order file=$(fd SharedData.hlsli) if [[ -z "$file" ]]; then echo "Error: SharedData.hlsli not found" >&2 exit 1 fi echo "=== cbuffer FeatureData fields in $file ===" awk '/cbuffer FeatureData/,/};/' "$file"Length of output: 989
🏁 Script executed:
#!/usr/bin/env bash # Extract the FeatureData payload initialization from FeatureBuffer.cpp file=$(fd FeatureBuffer.cpp) if [[ -z "$file" ]]; then file="src/FeatureBuffer.cpp" fi echo "=== FeatureData initializer in $file ===" awk '/FeatureData[[:space:]]*data/,/};/' "$file"Length of output: 273
🏁 Script executed:
#!/usr/bin/env bash # Show the context around lines 49-50 in FeatureBuffer.cpp file=$(fd FeatureBuffer.cpp | head -n1) if [[ -z "$file" ]]; then echo "Error: FeatureBuffer.cpp not found" >&2 exit 1 fi echo "=== Lines 30-70 of $file ===" nl -ba "$file" | sed -n '30,70p' echo echo "=== Extracted settings payload block ===" awk '/globals::features.*->settings/,/\);/' "$file"Length of output: 1166
FeatureData settings order matches HLSL struct
I’ve verified theFeatureDatacbuffer inSharedData.hlsliand confirmed that the sequence inFeatureBuffer.cpp—ending withglobals::features::extendedTranslucency->settings—exactly matches the HLSL layout. No changes are needed here.src/Feature.cpp (2)
7-7: Register ExtendedTranslucency in feature list
The new include is correctly added so the feature is visible to theFeaturesystem.
157-158: Ensure feature‐list ordering consistency
You’ve addedextendedTranslucencyto the static feature vector. Please verify that its position here aligns with its position in the feature‐buffer and the HLSL layout to maintain consistent indexing across systems.src/Globals.h (2)
27-27: Register the new feature globally
Forward-declaringExtendedTranslucencyin the globals header aligns with other features and exposes the global pointer.
80-80: ExposeextendedTranslucencypointer
Adding theextern ExtendedTranslucency* extendedTranslucency;entry integrates the feature into the global feature list.src/Globals.cpp (3)
17-17: Include the feature header
#include "Features/ExtendedTranslucency.h"ensures the singleton and class definitions are available for initialization.
79-79: Define the global pointer
InitializingExtendedTranslucency* extendedTranslucency = nullptr;alongside other features maintains consistency in global declarations.
163-163: Initialize the feature singleton
Assigningfeatures::extendedTranslucency = ExtendedTranslucency::GetSingleton();inOnInit()hooks the feature into the runtime pipeline.features/Extended Translucency/Shaders/ExtendedTranslucency/ExtendedTranslucency.hlsli (2)
1-9: Verify material model enum definitions
TheMaterialModelconstants (Disabled=0…AnisotropicFabric=3) align with two-bit encoding. This mapping is clear and concise.
18-21: Approve naive alpha modulation
GetViewDependentAlphaNaiveimplements the intended simple blend. Ensureviewandnormalare normalized upstream to avoid unexpected results.src/Features/ExtendedTranslucency.h (2)
34-36: Descriptor mask shape – document or widen
ExtraFeatureDescriptorMaskis0b111(values 0–7) but the enum only defines 0–3. Clarify the extra states (4–7) in a comment or tighten the mask to0b11.
20-20: VR support flag – double-check runtime parity
Returningtrueunconditionally implies the feature is known to run on both SE & VR. Please verify on the VR build; otherwise guard with a compile-time platform check.
|
✅ A pre-release build is available for this PR: |
|
@doodlum Fixed minor issues found by AI, and added support for UI refactoring. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/Features/ExtendedTranslucency.h (3)
13-13: Remove unnecessary semicolon after method definition.The semicolon after the closing brace is unnecessary for method definitions and inconsistent with other methods in the class.
- virtual bool HasShaderDefine(RE::BSShader::Type shaderType) override { return RE::BSShader::Type::Lighting == shaderType; }; + virtual bool HasShaderDefine(RE::BSShader::Type shaderType) override { return RE::BSShader::Type::Lighting == shaderType; }
19-19: Remove unnecessary semicolon after method definition.Consistent with line 13, this semicolon should be removed for style consistency.
- virtual bool SupportsVR() override { return true; }; + virtual bool SupportsVR() override { return true; }
44-44: Consider explicit enum conversion for clarity.While the implicit conversion from enum to uint32_t works correctly, making it explicit would improve code clarity and match the pattern suggested in previous reviews.
- uint32_t AlphaMode = MaterialModel::AnisotropicFabric; + uint32_t AlphaMode = static_cast<uint32_t>(MaterialModel::AnisotropicFabric);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package/Shaders/Lighting.hlslis excluded by!**/*.hlsl
📒 Files selected for processing (4)
features/Extended Translucency/Shaders/ExtendedTranslucency/ExtendedTranslucency.hlsli(1 hunks)src/Features/ExtendedTranslucency.cpp(1 hunks)src/Features/ExtendedTranslucency.h(1 hunks)src/State.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/State.h
- src/Features/ExtendedTranslucency.cpp
- features/Extended Translucency/Shaders/ExtendedTranslucency/ExtendedTranslucency.hlsli
🔇 Additional comments (3)
src/Features/ExtendedTranslucency.h (3)
30-37: Well-designed MaterialModel enum with clear documentation.The enum values are well-defined with meaningful names and helpful comments. The use of specific values (0, 1, 2, 3, 7) and the distinction between "Disabled" and "ForceDisabled" shows thoughtful design for the bit manipulation scheme.
42-48: Proper GPU buffer alignment and parameter organization.The
alignas(16)directive is appropriate for GPU constant buffer usage, and the parameter names and default values are sensible for translucency effects.
6-25: Comprehensive feature interface implementation.The class properly implements all required Feature interface methods with appropriate overrides. The shader define targeting only the Lighting shader type is correct for this feature's scope.
|
✅ A pre-release build is available for this PR: |
|
@doodlum Here is a AIO package for testing. |
|
Please also sync with the latest. |
Adding View Dependent Transparency methods to the game. This effect will make transclucent surface more opaque when view direction is parallel to it. The effect models regular fabric's transcluceny from the seams between threads. The thickness of the threads will occlude the seams based on view angle. Avoid frequent constant buffer update - We now use 5 pre defined constant buffers which don't need update - The only buffer update happens in setting menu - Minor fixes addressed in review - #if !defined(DEFERRED) cause the effect to disapper in one of my testing equipment, thus not included. Use Permutation::ExtraFeatureDescriptor for material specification Add memory fence after descriptor change Nif extra data can now disable this effect style: 🎨 apply clang-format changes Fix issues from AI review style: 🎨 apply clang-format changes
ArcEarth
force-pushed
the
extended_transcluency
branch
from
b368cac to
1754485
Compare
|
✅ A pre-release build is available for this PR: |
doodlum
left a comment
doodlum
left a comment
There was a problem hiding this comment.
This seems to work as expected. I do not really understand why there are settings but it's whatever I guess, sort of like SSS settings.
|
nice, also consider adding feature desc |
|
😂 Glad to see it's merged! Thank you a lot! 🎊 |
|
If you can get the link for the unpublished nexus page, we can update the link inside CS to cover it. We also can put in a short description if you provide some details. |
|
@alandtse That's amazing! Btw, I have already added the NexusURL/description in the feature's header file, following format in CS UI rework PR 😉 |
|
Ahh I remember now. We also simplified the format so all you need now is modid but this should be fine too. |
Adding View Dependent Transparency methods to the game. This effect will make transclucent surface more opaque when view direction is parallel to it. The effect models regular fabric's transcluceny from the seams between threads. The thickness of the threads will occlude the seams based on view angle. Avoid frequent constant buffer update - We now use 5 pre defined constant buffers which don't need update - The only buffer update happens in setting menu - Minor fixes addressed in review - #if !defined(DEFERRED) cause the effect to disapper in one of my testing equipment, thus not included. Use Permutation::ExtraFeatureDescriptor for material specification Add memory fence after descriptor change Nif extra data can now disable this effect style: 🎨 apply clang-format changes Fix issues from AI review style: 🎨 apply clang-format changes
6 participants
Hi, everyone,
I am super excited to contribute to this great project!
Please correct me if I got anything wrong :)
I am a little constraint on time so may reply to things late.
Adding a new feature to control translucent material's opacity.
The core changes is all in the HLSL side, where cpp changes just provide 3 setting variables.
Vanilla:
Anisotropic Fabric model:
Settings UI:
Future work:
Summary by CodeRabbit
New Features
Documentation
Chores