forked from community-shaders/skyrim-community-shaders
-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(slf): extract Tier-2 testable helpers #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
alandtse
wants to merge
7
commits into
dev
Choose a base branch
from
test/cpp-tier2-extracted-logic
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
44500e7
refactor(isl): extract radius/attenuation math to a testable header
alandtse f68a5b3
build: add /bigobj to the plugin for MSVC 14.50
alandtse d28c4a5
refactor(slf): extract shadow-pointer/percentile/sanitize math for te…
alandtse 64d4537
build(slf): drop /bigobj from plugin target
alandtse 5499cf2
style: 🎨 apply pre-commit.ci formatting
pre-commit-ci[bot] 8ec7a49
fix(llf,isl): harden extracted helpers against degenerate inputs
alandtse 688c960
test(scm): cover count-above-window; doc SanitizeFloat precondition
alandtse File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| #pragma once | ||
|
|
||
| #include <algorithm> | ||
| #include <cmath> | ||
|
|
||
| // Pure ISL radius/attenuation math, extracted so it can be unit-tested without | ||
| // the game/RE runtime. InverseSquareLighting's static methods delegate here. | ||
| namespace ISLMath | ||
| { | ||
| inline constexpr float DefaultCutoff = 0.05f; | ||
| inline constexpr float DefaultShadowCasterCutoff = 0.022f; | ||
|
|
||
| inline constexpr float Scale = 0.8f; | ||
| inline constexpr float MetresToUnits = 70.f; | ||
| inline constexpr float MetresToUnitsSq = MetresToUnits * MetresToUnits; | ||
| inline constexpr float ScaledUnitsSq = Scale * MetresToUnitsSq; | ||
| inline constexpr float FadeZoneBase = 4.5f * Scale * MetresToUnits; | ||
|
|
||
| inline float SmoothStep(float edge0, float edge1, float x) | ||
| { | ||
| const float t = std::clamp((x - edge0) / (edge1 - edge0), 0.0f, 1.0f); | ||
| return t * t * (3.0f - 2.0f * t); | ||
| } | ||
|
|
||
| // Radius at which the inverse-square falloff reaches `cutoff`. A cutoffOverride | ||
| // of exactly 1.0 means "use the default"; anything else overrides it. Any | ||
| // degenerate result clamps to 1.0 so a light never gets a bad radius: a | ||
| // negative sqrt argument yields NaN, an exact-zero argument yields 0 (callers | ||
| // divide by radius, so 0 would drive a 0/0 SmoothStep), and a zero | ||
| // cutoffOverride yields +inf -- a finite-and-positive check guards all three. | ||
| inline float CalculateRadius(float intensity, bool shadowCaster, float cutoffOverride, float size) | ||
| { | ||
| float cutoff = shadowCaster ? DefaultShadowCasterCutoff : DefaultCutoff; | ||
| cutoff = cutoffOverride == 1.f ? cutoff : cutoffOverride; | ||
| const float radius = std::sqrt(ScaledUnitsSq * ((2 * intensity - cutoff * size * size) / (2 * cutoff))); | ||
| return std::isfinite(radius) && radius > 0.0f ? radius : 1.0f; | ||
| } | ||
|
|
||
| inline float GetAttenuation(float distance, float radius, float size) | ||
| { | ||
| const float attenuation = ScaledUnitsSq / (distance * distance + ScaledUnitsSq * size * size / 2); | ||
| const float fadeZone = std::clamp(FadeZoneBase / radius, 0.0f, 1.0f); | ||
| const float fade = SmoothStep(0, radius * fadeZone, radius - distance); | ||
| return attenuation * fade; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| #pragma once | ||
|
|
||
| #include <algorithm> | ||
| #include <cmath> | ||
|
|
||
| // Pure settings-sanitization helper extracted from LightLimitFix so it can be | ||
| // unit-tested without the game/RE runtime. | ||
| namespace LightLimitFixSanitize | ||
| { | ||
| // Clamp a user/config float to [lo, hi]. std::clamp passes NaN through | ||
| // unchanged (every NaN comparison is false), which would let a non-finite | ||
| // value reach the GPU and cause divisions / infinite loops / corruption, so | ||
| // reject non-finite inputs explicitly and fall back to the lower bound for | ||
| // degraded-but-stable behavior. | ||
| // | ||
| // Precondition: lo and hi are finite with lo <= hi. Only the value is | ||
| // untrusted; the bounds are compile-time constants at every call site (the | ||
| // feature's fixed setting ranges), so they are not re-validated here -- a | ||
| // NaN lo would defeat the fallback and hi < lo is std::clamp UB. | ||
| inline float SanitizeFloat(float v, float lo, float hi) | ||
| { | ||
| return std::isfinite(v) ? std::clamp(v, lo, hi) : lo; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| #pragma once | ||
|
|
||
| #include <algorithm> | ||
| #include <cstdint> | ||
|
|
||
| // Pure helpers extracted from ShadowCasterManager so they can be unit-tested | ||
| // without the game/RE runtime. | ||
| namespace ShadowCasterManager | ||
| { | ||
| // A shadow-light accumulator slot can hold heap garbage between our prepass | ||
| // and the engine's read. Treat a pointer as plausible only if it is at or | ||
| // above the low 64 KiB (the Windows x64 null-guard region is never a valid | ||
| // user mapping, so a near-null garbage value like 0x8 must be rejected -- | ||
| // dereferencing it is a guaranteed AV/CTD), inside the x64 user-mode | ||
| // canonical range, and 8-byte aligned (BSShadowLight is pointer-aligned). | ||
| // ForEachShadowLight stops iterating at the first implausible entry rather | ||
| // than dereferencing garbage. | ||
| inline bool IsPlausibleShadowLightPtr(std::uintptr_t raw) noexcept | ||
| { | ||
| return raw >= 0x10000ull && raw < 0x0000800000000000ull && (raw & 0x7) == 0; | ||
| } | ||
|
alandtse marked this conversation as resolved.
|
||
|
|
||
| // 90th-percentile of the most-recent min(count, Window) frame-time samples | ||
| // in `ring`. Percentile is order-independent, so the first `n` entries are | ||
| // sampled directly (ring head/wraparound doesn't matter). Returns the 60fps | ||
| // fallback (16.67 ms) before any samples exist. A non-positive count (no | ||
| // samples, or a corrupt/negative value) takes the fallback -- a negative n | ||
| // would otherwise drive std::copy / std::nth_element out of bounds. | ||
| template <int Window> | ||
| inline float FrameTimePercentile90(const float (&ring)[Window], int count) | ||
| { | ||
| if (count <= 0) | ||
| return 16.67f; | ||
| const int n = std::min(count, Window); | ||
| float tmp[Window]; | ||
| std::copy(ring, ring + n, tmp); | ||
| const int idx = static_cast<int>(n * 0.9f); | ||
| std::nth_element(tmp, tmp + idx, tmp + n); | ||
| return tmp[idx]; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| // Unit tests for ISL radius/attenuation math (RadiusMath.h). | ||
|
|
||
| #include "Features/InverseSquareLighting/RadiusMath.h" | ||
|
|
||
| #include <catch2/catch_approx.hpp> | ||
| #include <catch2/catch_test_macros.hpp> | ||
|
|
||
| using Catch::Approx; | ||
|
|
||
| TEST_CASE("SmoothStep is a clamped Hermite ramp", "[isl]") | ||
| { | ||
| REQUIRE(ISLMath::SmoothStep(0.0f, 1.0f, 0.0f) == Approx(0.0f)); | ||
| REQUIRE(ISLMath::SmoothStep(0.0f, 1.0f, 1.0f) == Approx(1.0f)); | ||
| REQUIRE(ISLMath::SmoothStep(0.0f, 1.0f, 0.5f) == Approx(0.5f)); // t^2(3-2t) at 0.5 | ||
| // Clamps outside [edge0, edge1]. | ||
| REQUIRE(ISLMath::SmoothStep(0.0f, 1.0f, -1.0f) == Approx(0.0f)); | ||
| REQUIRE(ISLMath::SmoothStep(0.0f, 1.0f, 2.0f) == Approx(1.0f)); | ||
| } | ||
|
|
||
| TEST_CASE("CalculateRadius matches the closed form for default cutoff", "[isl]") | ||
| { | ||
| // cutoffOverride == 1 -> default cutoff (0.05 for non-shadow); size 0. | ||
| // radius = sqrt(ScaledUnitsSq * (2*intensity / (2*0.05))) = sqrt(3920 * 20) = 280. | ||
| REQUIRE(ISLMath::CalculateRadius(1.0f, false, 1.0f, 0.0f) == Approx(280.0f)); | ||
| } | ||
|
|
||
| TEST_CASE("Shadow casters get a larger radius (smaller cutoff)", "[isl]") | ||
| { | ||
| const float normal = ISLMath::CalculateRadius(1.0f, false, 1.0f, 0.0f); | ||
| const float shadow = ISLMath::CalculateRadius(1.0f, true, 1.0f, 0.0f); | ||
| REQUIRE(shadow > normal); | ||
| } | ||
|
|
||
| TEST_CASE("cutoffOverride replaces the default regardless of shadow flag", "[isl]") | ||
| { | ||
| // Override of 0.05 forces the non-shadow default value even for a shadow caster. | ||
| REQUIRE(ISLMath::CalculateRadius(1.0f, true, 0.05f, 0.0f) == Approx(280.0f)); | ||
| } | ||
|
|
||
| TEST_CASE("CalculateRadius clamps degenerate results to 1", "[isl]") | ||
| { | ||
| // NaN: intensity 0 with a large size makes the sqrt argument negative. | ||
| REQUIRE(ISLMath::CalculateRadius(0.0f, false, 1.0f, 10.0f) == Approx(1.0f)); | ||
| // Exact zero: 2*intensity == cutoff*size^2 (0.05 * 2^2 = 0.2, intensity 0.1) | ||
| // makes the sqrt argument 0. A 0 radius would drive a 0/0 SmoothStep downstream. | ||
| REQUIRE(ISLMath::CalculateRadius(0.1f, false, 1.0f, 2.0f) == Approx(1.0f)); | ||
| // +inf: a zero cutoffOverride divides by 2*cutoff == 0. isnan() alone misses | ||
| // this (sqrt(+inf) is +inf, not NaN); the finite check catches it. The cutoff | ||
| // is read through a volatile so MSVC can't constant-fold the literal /0 into a | ||
| // compile-time C4723 -- at runtime the float divide yields +inf as intended. | ||
| volatile float zeroCutoff = 0.0f; | ||
| REQUIRE(ISLMath::CalculateRadius(1.0f, false, zeroCutoff, 0.0f) == Approx(1.0f)); | ||
| } | ||
|
|
||
| TEST_CASE("GetAttenuation peaks at the source and vanishes past the radius", "[isl]") | ||
| { | ||
| // distance 0, radius 280, size 1: attenuation = 3920/(0 + 3920/2) = 2.0; | ||
| // fade is 1 at the source. | ||
| REQUIRE(ISLMath::GetAttenuation(0.0f, 280.0f, 1.0f) == Approx(2.0f)); | ||
| // Beyond the radius the SmoothStep fade clamps to zero. | ||
| REQUIRE(ISLMath::GetAttenuation(300.0f, 280.0f, 1.0f) == Approx(0.0f)); | ||
| } | ||
|
|
||
| TEST_CASE("GetAttenuation decreases monotonically with distance in range", "[isl]") | ||
| { | ||
| // 'near'/'far' are legacy Windows.h macros, so use distinct names. | ||
| const float attNear = ISLMath::GetAttenuation(10.0f, 280.0f, 1.0f); | ||
| const float attFar = ISLMath::GetAttenuation(150.0f, 280.0f, 1.0f); | ||
| REQUIRE(attNear > attFar); | ||
| REQUIRE(attFar > 0.0f); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| // Unit tests for the LightLimitFix settings sanitizer | ||
| // (src/Features/LightLimitFix/SettingsSanitize.h). | ||
|
|
||
| #include "Features/LightLimitFix/SettingsSanitize.h" | ||
|
|
||
| #include <catch2/catch_approx.hpp> | ||
| #include <catch2/catch_test_macros.hpp> | ||
|
|
||
| #include <cmath> | ||
| #include <limits> | ||
|
|
||
| using Catch::Approx; | ||
| using LightLimitFixSanitize::SanitizeFloat; | ||
|
|
||
| TEST_CASE("SanitizeFloat clamps in-range, low, and high inputs", "[llf]") | ||
| { | ||
| REQUIRE(SanitizeFloat(0.5f, 0.0f, 1.0f) == Approx(0.5f)); | ||
| REQUIRE(SanitizeFloat(-1.0f, 0.0f, 1.0f) == Approx(0.0f)); | ||
| REQUIRE(SanitizeFloat(2.0f, 0.0f, 1.0f) == Approx(1.0f)); | ||
| REQUIRE(SanitizeFloat(64.0f, 64.0f, 4096.0f) == Approx(64.0f)); // exact lower bound | ||
| REQUIRE(SanitizeFloat(4096.0f, 64.0f, 4096.0f) == Approx(4096.0f)); // exact upper bound | ||
| } | ||
|
|
||
| TEST_CASE("SanitizeFloat falls back to the lower bound on non-finite input", "[llf]") | ||
| { | ||
| const float nan = std::numeric_limits<float>::quiet_NaN(); | ||
| const float inf = std::numeric_limits<float>::infinity(); | ||
| REQUIRE(SanitizeFloat(nan, 0.5f, 8.0f) == Approx(0.5f)); | ||
| REQUIRE(SanitizeFloat(inf, 0.5f, 8.0f) == Approx(0.5f)); | ||
| REQUIRE(SanitizeFloat(-inf, 0.5f, 8.0f) == Approx(0.5f)); | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.