Rendering: a shadow-caster stream for materials, and a bright pass that survives NaN by Nicogo1705 · Pull Request #3387 · stride3d/stride

Nicogo1705 · 2026-09-06T15:20:36Z

PR Details

Two rendering fixes, both found while rendering a ray-marched surface through the material path, both general.

Shadows: a stream tells a material it is drawn for a shadow map

ShadowMapCasterPassInfo declares stage stream float IsShadowMapCasterPass; ShadowMapCasterPassMarker, mixed into the three caster effects, writes it in the pixel stage before the material chain runs. A material whose surface is not its mesh - a volume, a field walked in the pixel shader - can then keep its own depth for the shadow map where it would otherwise draw nothing.

The value is the view's, not a constant. The caster effect is also what a compositor's depth-only prepass runs for the camera, and a constant 1 there had such a material write its shadow depth into the camera's own depth buffer, where the surface it drew afterwards failed the depth test wherever the two disagreed. ShadowCasterRenderFeature writes a per-view flag for every view that has the group, skipping a view that draws nothing through a layout, which has no buffer prepared for it.

Post-processing: guard the bright pass against infinities and NaNs

The bright filter's output is downsampled and blurred through a pyramid of half-float targets. A value above 65504 becomes an infinity there, the first blur weight of zero turns that infinity into a NaN, and the NaN spreads across the kernel and surfaces as a black block the size of the mip it happened on. Luma already floored negatives; nothing capped the other end. The bright filter and the range compressor now clamp to 32000 - half a half-float's ceiling, leaving headroom for the sums that follow - through a shared HalfPrecisionUtils.ClampToSafeRange, written as min(max(color, 0), max), which also replaces a NaN.

The optimization I wrote about the clamping I think should be better in theory. Better to test its correctness and performance, of course. min + max should be branchless while also not falling into the previous infinite/NaN problem.
Nice idea the shadow caster pass identification!
I would have preferred however if this were called something like DepthOnlyPass or something alike. It's weird (even if logically correct) that the depth-prepass invokes a shadow-caster pass. But maybe renaming those are a bigger change not worth the effort.

Ethereal77 · 2026-09-07T21:30:55Z

+            // block the size of the mip it happened on. Luma floors negatives; nothing capped the
+            // other end. Written as a comparison rather than isfinite() because a NaN fails every
+            // comparison, so this catches it, an infinity and a merely absurd value alike.
+            color = color > 0.0 ? min(color, MaxBrightPassValue) : float3(0, 0, 0);


Possibly not important (I haven't benchmarked it), but maybe better as color = min(max(color, 0.0), MaxBrightPassValue) (less warp divergence, more SIMT/SIMD friendly than a branch).

Done in both shaders. I compared the fxc output (ps_5_0, /O3) of the two forms on a minimal shader: neither branches, the ternary compiles to a compare plus a mask, so min(max(...)) saves one of three instructions. Both replace a NaN by 0 (lt is false for a NaN, min/max return the non-NaN operand).

// color = color > 0.0 ? min(color, 32000.0) : 0 lt r1.xyz, l(0, 0, 0, 0), r0.xyzx min r0.xyz, r0.xyzx, l(32000, 32000, 32000, 0) and o0.xyz, r0.xyzx, r1.xyzx // color = min(max(color, 0.0), 32000.0) max r0.xyz, r0.xyzx, l(0, 0, 0, 0) min o0.xyz, r0.xyzx, l(32000, 32000, 32000, 0)

Verified on the VoxelGI demo with bloom on: same image, no NaN blocks.

Ethereal77 · 2026-09-07T21:33:14Z

+            // A NaN fails every comparison, so this keeps sane values, caps an infinity and floors
+            // a negative one - any of which would otherwise become the NaN that FXAA reads as this
+            // pixel's luma and then spreads along whatever edge it finds.
+            color = color > 0.0 ? min(color, 32000.0) : float3(0, 0, 0);


Same as above.

Also, better introduce here also the static const float MaxBrightPassValue = 32000.0 instead of hardcoding its value.

Even better would be if the const was defined just once and shared.

Done. Both shaders now call HalfPrecisionUtils.ClampToSafeRange(color), a small shared utility shader in Rendering/Images on the LuminanceUtils pattern, which owns the 32000 constant.

Ethereal77 · 2026-09-07T21:40:45Z

+            /// 1 in a shadow map view, 0 in every other view - the camera's own depth-only
+            /// prepass included, which runs the very same caster effect. Written for every
+            /// view by ShadowCasterRenderFeature, so it is never stale.
+            stage float4 ShadowMapViewFlag;


Why float4 if only .x is ever read?

No reason, leftover from a first version. It is a stage float now, and ShadowCasterRenderFeature sets a plain float.

xen2 · 2026-09-08T05:29:32Z

@Ethereal77
Sorry for the grouped PR, @Nicogo1705 asked me before submitting and I approved his proposed grouping.
But in retrospective, I agree it might have been better to split them further, for all the independent parts.
I think there is a large pass to do on the comments for all his PRs (too long/detailed, cf #3388 (comment)), then maybe @Nicogo1705 can reopen them with a bit more fine-grained PRs?

…it is the view's answer ShadowMapCasterPassInfo declares IsShadowMapCasterPass, which ShadowMapCasterPassMarker - mixed into the three caster effects - writes in the pixel stage before the material chain runs, so a material whose surface is not its mesh can keep its depth for the shadow map where it would otherwise draw nothing. The value is the view's, not a constant: the caster effect is also what a compositor's depth-only prepass runs for the camera, and a constant 1 there had such a material write its shadow depth into the camera's own depth buffer. ShadowCasterRenderFeature writes the flag for every view that has the group, skipping a view that draws nothing through a layout, which has no buffer prepared for it. (from 5039fec, c5da03b, 111a3f0 and 3812f35, without their Stride.Voxels parts) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The bright filter's output is downsampled and blurred through a pyramid of half-float targets. A value above 65504 becomes an infinity there, and the first blur weight of zero turns that infinity into a NaN, which then spreads across the kernel and surfaces as a black block the size of the mip it happened on. Luma already floored negatives; nothing capped the other end. Clamp to 32000 - half a half-float's ceiling, leaving headroom for the sums that follow - and write it as a comparison rather than isfinite(), since a NaN fails every comparison and so is caught by the same test as an infinity or a merely absurd value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 4def349)

… shorter comments Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Nicogo1705 · 2026-09-08T15:17:37Z

Thanks for the review!

Branchless clamp applied in both shaders, see above.
On the name: I agree it reads oddly that the depth prepass runs a "shadow caster" effect. But the stream answers "am I in a shadow map view" and is 0 in the camera's depth prepass, so IsShadowMapCasterPass is the accurate name for what it carries.
Fair point on splitting, I will go one fix per PR from now on.

Nicogo1705 · 2026-09-08T15:17:38Z

Comments trimmed on this PR and on #3388 / #3389: 1 to 2 lines, no more references to the previous behaviour. If you would rather have these two fixes as separate PRs, I can reopen them that way, just say so.

Ethereal77 · 2026-09-08T18:50:07Z

Hey, no need to apologize. I'm not opposed to grouping if the changes are small. I understand it's more convenient sometimes to dump several changes into a single PR than open several. I just like that PRs sometimes serve as a kind of auto-documentation on how the software evolved and why some changes were needed, and combining several changes into one makes each change less discoverable and sometimes less understandable (context switching overhead 😄). Just my opinion, of course

Ethereal77 · 2026-09-08T18:55:34Z

On the name: I agree it reads oddly that the depth prepass runs a "shadow caster" effect. But the stream answers "am I in a shadow map view" and is 0 in the camera's depth prepass, so IsShadowMapCasterPass is the accurate name for what it carries.

Oh, now I see what you mean. I have misread the PR and thought you have also rewritten the depth-prepass to use this and this just served to compute the depth-only output (hence my comment). But I see now that the depth-prepass was already using the shadow caster effect. Sorry for the confusion

xen2 · 2026-09-09T03:00:48Z

+        /// </summary>
+        static float3 ClampToSafeRange(float3 color)
+        {
+            return min(max(color, 0.0), MaxSafeValue);


Unfortunately, it works only on D3D.
Vulkan FMax/FMin result is undefined on NaN.
Comparison (as in your previous commit) should work everywhere:

static float3 ClampToSafeRange(float3 color) { // A NaN fails every comparison, so it lands on MaxSafeValue like an infinity does. return color < MaxSafeValue ? max(color, 0.0) : MaxSafeValue; }

There is no way to make this branchless in Vulkan?

xen2 · 2026-09-09T03:05:56Z

                var shadowMapRenderView = view as ShadowMapRenderView;
+
+                // ShadowMapCasterPassInfo flag, written for every view (not only shadow ones) so the group is never left uninitialized.
+                foreach (var viewLayout in viewFeature.Layouts)


viewFeature.Layouts is looped twice (and they have different loop guards, yours is checking resourceGroup but not the other one; which is right?)

xen2 · 2026-09-09T03:16:08Z

+            stage float ShadowMapViewFlag;
+        }
+
+        stage stream float IsShadowMapCasterPass;


Question: why does user/material need to write 0?
Can't we do like in other shaders:
override void ResetStream() { base.ResetStream(); streams.IsShadowMapCasterPass= 0.0f; }

Never mind, ResetStream won't be called in this case.
But then another question: since this is computed in pixel shader, why using a stream at all? can't we use ShadowMapViewFlag directly?

Ethereal77 reviewed Sep 7, 2026

View reviewed changes

Nicogo1705 and others added 3 commits September 8, 2026 16:43

Review: branchless clamp shared in HalfPrecisionUtils, flag as float,…

f27bcef

… shorter comments Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Nicogo1705 force-pushed the rendering-fixes branch from f3108f8 to 2b4d960 Compare September 8, 2026 15:17

Nicogo1705 force-pushed the rendering-fixes branch from 2b4d960 to f27bcef Compare September 8, 2026 15:26

Nicogo1705 mentioned this pull request Sep 8, 2026

Games, Graphics, Profiling: pick the discrete GPU of a hybrid laptop, type the GPU preference, keep the profiler page at 1 or more #3388

Open

4 tasks

xen2 reviewed Sep 9, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

Nicogo1705 commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Details

Shadows: a stream tells a material it is drawn for a shadow map

Post-processing: guard the bright pass against infinities and NaNs

Related

Uh oh!

Ethereal77 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xen2 commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nicogo1705 commented Sep 8, 2026

Uh oh!

Nicogo1705 commented Sep 8, 2026

Uh oh!

Ethereal77 commented Sep 8, 2026

Uh oh!

Ethereal77 commented Sep 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Nicogo1705 commented Sep 6, 2026 •

edited

Loading

xen2 commented Sep 8, 2026 •

edited

Loading