Skip to content

fix: fix blown out water specular #1853

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.

Sign up for GitHub

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

Merged
doodlum merged 5 commits into from
Feb 11, 2026
Merged

fix: fix blown out water specular #1853

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
97d4e1c
fix: fix blown out water specular
doodlum Feb 9, 2026
3ab734b
Merge branch 'dev' into water-brightnes-fix
doodlum Feb 9, 2026
f074c0c
chore: tonemap instead
doodlum Feb 9, 2026
9b2cc09
Merge branch 'water-brightnes-fix' of https://github.com/doodlum/skyr…
doodlum Feb 9, 2026
82b6619
fix: fix nonsense
doodlum Feb 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package/Shaders/Water.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1096,7 +1096,7 @@ float3 GetSunColor(float3 normal, float3 viewDirection)
float reflectionMul = exp2(VarAmounts.x * log2(saturate(dot(reflectionDirection, SunDir.xyz))));

float llDirLightMult = (SharedData::linearLightingSettings.enableLinearLighting && !SharedData::linearLightingSettings.isDirLightLinear) ? SharedData::linearLightingSettings.dirLightMult : 1.0f;
return reflectionMul * Color::DirectionalLight(SunColor.xyz / max(llDirLightMult, 1e-5), SharedData::linearLightingSettings.isDirLightLinear) * SunDir.w * DeepColor.w * llDirLightMult;
return reflectionMul * Color::DirectionalLight((SunColor.xyz * SunDir.w) / max(llDirLightMult, 1e-5), SharedData::linearLightingSettings.isDirLightLinear) * (1.0 - exp(-DeepColor.w))* llDirLightMult;
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Tonemap may unintentionally dim specular for moderate DeepColor.w values.

1.0 - exp(-x) asymptotes to 1.0, which fixes the blown-out case (x > 2000), but it also compresses moderate values: at DeepColor.w = 1.0 the result is ~0.632 (a ~37% reduction vs. the previous direct multiply), and at DeepColor.w = 0.5 it's ~0.393 (a ~21% reduction). If vanilla records commonly use values in the 0–2 range for normal water, this would noticeably reduce their specular brightness compared to before.

Was this dimming for "normal" records evaluated, or is the intent purely to tame the extreme outliers? A scaled variant like 1.0 - exp(-DeepColor.w / k) with an appropriate k could shift the knee point to preserve existing behavior for typical values while still clamping the extremes.

Also, minor formatting nit — missing space before the last *:

-	return reflectionMul * Color::DirectionalLight((SunColor.xyz * SunDir.w) / max(llDirLightMult, 1e-5), SharedData::linearLightingSettings.isDirLightLinear) * (1.0 - exp(-DeepColor.w))* llDirLightMult;
+	return reflectionMul * Color::DirectionalLight((SunColor.xyz * SunDir.w) / max(llDirLightMult, 1e-5), SharedData::linearLightingSettings.isDirLightLinear) * (1.0 - exp(-DeepColor.w)) * llDirLightMult;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return reflectionMul * Color::DirectionalLight((SunColor.xyz * SunDir.w) / max(llDirLightMult, 1e-5), SharedData::linearLightingSettings.isDirLightLinear) * (1.0 - exp(-DeepColor.w))* llDirLightMult;
return reflectionMul * Color::DirectionalLight((SunColor.xyz * SunDir.w) / max(llDirLightMult, 1e-5), SharedData::linearLightingSettings.isDirLightLinear) * (1.0 - exp(-DeepColor.w)) * llDirLightMult;
🤖 Prompt for AI Agents 
In `@package/Shaders/Water.hlsl` at line 1099, The tonemap expression using (1.0 -
exp(-DeepColor.w)) compresses moderate DeepColor.w values and dims specular;
replace it with a scaled variant such as (1.0 - exp(-DeepColor.w /
SpecularTonemapK)) where SpecularTonemapK is a tunable constant (e.g., 8–16) so
normal water (0–2 range) preserves previous brightness while still clamping
extremes, and add the missing space before the final *; update the expression
involving reflectionMul, Color::DirectionalLight, DeepColor.w and llDirLightMult
to use the new scaled function/constant and ensure formatting includes a space
before the last multiplication.

# endif
}
# endif
Expand Down