Skip to content

revert: "perf(terrain shadows): half res and 4x faster updates"#2172

Merged
alandtse merged 1 commit into
devfrom
revert-2163-terrain-shadows-opt
Apr 22, 2026
Merged

revert: "perf(terrain shadows): half res and 4x faster updates"#2172
alandtse merged 1 commit into
devfrom
revert-2163-terrain-shadows-opt

Conversation

@doodlum
Copy link
Copy Markdown
Collaborator

@doodlum doodlum commented Apr 21, 2026

Reverts #2163

Summary by CodeRabbit

Refactor

  • Streamlined terrain shadow coordinate calculations to reduce redundant dimension queries.
  • Improved heightmap loading efficiency by eliminating unnecessary intermediate processing steps.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 21, 2026

📝 Walkthrough

Walkthrough

The PR consolidates terrain shadow rendering to use unified heightmap dimensions for both sampling and output operations. It removes separate shadow texture dimension handling, eliminates an intermediate heightmap resize step during loading, and updates dispatch coordinate calculations to reference the heightmap dimensions consistently.

Changes

Cohort / File(s) Summary
Shader Dimension Unification
features/Terrain Shadows/Shaders/TerrainShadows/ShadowUpdate.cs.hlsl
Removed separate dimension queries for heightmap and shadow render target, replacing them with a single dimension derived from heightmap. Consolidated per-thread pixel coordinate calculation to use one coordinate space for both reading and writing operations, updating all corresponding indexing and function calls.
Heightmap Loading & Dispatch Logic
src/Features/TerrainShadows.cpp
Eliminated intermediate DDS load and downscaling step in heightmap loading; DDS now loads directly. Changed shadow dispatch dimensions from shadow texture to heightmap dimensions. Updated pixel size constant buffer data to derive from heightmap texture rather than shadow texture.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • davo0411
  • alandtse

Poem

🐰 Shadows once split in two dimensions wide,
Now unified—one heightmap as our guide!
No resizing dance, just direct and clean,
The sharpest terrain rendering ever seen! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately describes the main change: reverting a previous performance optimization for terrain shadows. It directly reflects the PR's objective.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch revert-2163-terrain-shadows-opt

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@alandtse alandtse changed the title fix(terrain shadows): revert "perf(terrain shadows): half res and 4x faster updates" revert: "perf(terrain shadows): half res and 4x faster updates" Apr 21, 2026
@alandtse alandtse merged commit b77e9e1 into dev Apr 22, 2026
10 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request May 11, 2026
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.

2 participants