Skip to content

Fix ghost shadows from clipped geometry in shadow cast pass#13388

Open
Masty88 wants to merge 20 commits intoCesiumGS:mainfrom
Masty88:fix/double-cast
Open

Fix ghost shadows from clipped geometry in shadow cast pass#13388
Masty88 wants to merge 20 commits intoCesiumGS:mainfrom
Masty88:fix/double-cast

Conversation

@Masty88
Copy link
Copy Markdown
Contributor

@Masty88 Masty88 commented Apr 9, 2026

Description

Geometry clipped via ClippingPolygonCollection or ClippingPlaneCollection
continued to cast shadows as if it were still present, making shadow/sun studies
unusable in AEC and BIM workflows where clipping replaces an existing photomesh
with a proposed design.


Root cause — ClippingPolygonCollection

For opaque models, the shadow cast shader skipped calling the original fragment
shader as an optimization (no color output needed in the depth pass). This meant
discard statements injected by the clipping stage never executed during the
shadow pass.

The fix detects whether the fragment shader contains a discard statement via
ShadowMapShader.containsDiscardForShadowCast(), which checks clipping-related
defines (HAS_CLIPPING_PLANES, ENABLE_CLIPPING_POLYGONS) as a fast path, then
falls back to scanning the shader source with comments and string literals stripped
to avoid false positives. If found (hasDiscard = true), the cast shader calls
czm_shadow_cast_main() — the renamed original fragment main — so that clipping
discards execute correctly during the shadow pass. For plain opaque shaders without
discard, the previous fast path is preserved. The flag is included in the shader
cache key to avoid collisions between the two variants.

BEFORE:
before

AFTER:
after


Root cause — ClippingPlaneCollection

The clipping planes matrix (model_clippingPlanesMatrix) was computed once per
frame inside updateReferenceMatrices using the camera view matrix and cached on
the model. Since the shadow pass runs before updateReferenceMatrices is
called each frame, the cached matrix always reflected the camera view of the
previous frame. During the shadow cast pass czm_view represents the light
direction, not the camera, so the planes were evaluated in the wrong coordinate
space — the GPU received the wrong matrix and could not tell which pixels should
be clipped.

The fix removes the per-frame computation from Model.js and moves it into the
uniform function inside ModelClippingPlanesPipelineStage, where
context.uniformState.view3D always reflects the active pass (camera or light)
at the exact moment each draw call is issued. This turns a once-per-frame
calculation into a per-draw-call one, which is an acceptable trade-off given that
clipping is only active on a subset of models. Two module-level scratch matrices
are used to avoid aliasing (Matrix4.multiply does not support src === dst).

BEFORE:
before

AFTER:
after


Issue number and link

Fixes #6261

Testing plan

  • New unit tests in ShadowMapShaderSpec.js covering the hasDiscard branching
    for both opaque and translucent cases.
  • New Sandcastle example Shadows Clipping Polygon demonstrating both fixes:
    • OSM buildings clipped by a ClippingPolygonCollection — toggle Clipping
      off to see ghost shadows reappear.
    • CesiumAir model with rear half clipped by a ClippingPlaneCollection
      mirrors the screenshot from the original bug report.
  • All existing shadow map tests pass: npm run test -- --grep ShadowMap

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have updated the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code

AI acknowledgment

  • I used AI to generate content in this PR
  • I have reviewed the code generated

Tools: Claude
Model: Claude Sonnet 4.6

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2026

Thank you for the pull request, @Masty88!

✅ We can confirm we have a CLA on file for you.

@Masty88
Copy link
Copy Markdown
Contributor Author

Masty88 commented Apr 9, 2026

Hi @ggetz, draft fix for #6261 — let me know if the approach makes sense or if I'm going in the wrong direction!

@javagl
Copy link
Copy Markdown
Contributor

javagl commented Apr 9, 2026

(Don't worry about the test that is currently failing. That's a known issue)

@Masty88
Copy link
Copy Markdown
Contributor Author

Masty88 commented Apr 9, 2026

Performance note

To quantify the trade-off introduced by the ClippingPlaneCollection fix
(matrix computed per draw call instead of once per model per frame), I instrumented
both branches with performance.mark around the matrix computation and captured
Chrome traces with 8 and 64 clipped models active.

On main, the markers wrap the existing computation in updateReferenceMatrices
in Model.js (called once per model per frame):

// Model.js — updateReferenceMatrices()
if (model.isClippingEnabled()) {
  performance.mark("clippingPlanesMatrix-start");
  // ... multiply + inverseTranspose ...
  performance.mark("clippingPlanesMatrix-end");
}

On the fix branch, the markers wrap the uniform function in
ModelClippingPlanesPipelineStage.js (called once per draw call):

// ModelClippingPlanesPipelineStage.js — model_clippingPlanesMatrix uniform
model_clippingPlanesMatrix: function () {
  performance.mark("clippingPlanesMatrix-start");
  // ... multiply + inverseTranspose ...
  performance.mark("clippingPlanesMatrix-end");
  return result;
},
Models Branch Calls / frame Cost / frame
8 main 9 0.10 ms
8 fix 71 0.52 ms
64 main 58 0.40 ms
64 fix 396 2.33 ms

The fix introduces a ~5–6× increase in matrix computations per frame (one per
draw call instead of one per model). In absolute terms the overhead remains small:
< 0.5 ms for scenes with up to 8 clipped models, ~2.3 ms at 64 models on an
NVIDIA RTX 3070.

Alternatives considered:

  • Opt-in flag (clippingPlanesAffectShadows) — let the user choose the
    trade-off. Rejected: ghost shadows are a correctness bug, not a performance
    trade-off. The correct behaviour should be the default.

@Masty88 Masty88 marked this pull request as ready for review April 21, 2026 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shadows ignore clipping planes

3 participants