Skip to content

fix(TerrainBlending): reduce offset depth to prevent ground artifacts#1788

Closed
ParticleTroned wants to merge 2 commits into
community-shaders:devfrom
ParticleTroned:dev
Closed

fix(TerrainBlending): reduce offset depth to prevent ground artifacts#1788
ParticleTroned wants to merge 2 commits into
community-shaders:devfrom
ParticleTroned:dev

Conversation

@ParticleTroned
Copy link
Copy Markdown
Contributor

@ParticleTroned ParticleTroned commented Jan 31, 2026

Issue:
In VR with Terrain Blending, a rectangular ground shadow artifact can appears (not always). The artifact moves with the HMD, is visible in the shadowmask path, and is gone when TB is toggled off. The acrtifact scales with TB’s depth mismatch; lowering the bias removes it while keeping blending stable.

Fix: Decreasing Depth Offset Utility.hlsl to vsout.PositionCS.z += 1.25; I could see no difference in blending on snow, rock etc when using values from 1.5 to 10.

--> I have not tested this extensively in flat! but in principle it should behave the same as VR.

Examples: vsout.PositionCS.z += 10;
vs10

Examples: vsout.PositionCS.z += 5; -> artifact only visible when looking straight down
vs5d

Examples: vsout.PositionCS.z += 1.25; -> artifact not visible anymore in HMD
vs125

Summary by CodeRabbit

  • Bug Fixes
    • Reduced depth rendering offset to improve visual layering and reduce z-fighting in 3D scenes.
  • New Features
    • Editor now advertises VR support, enabling VR-aware tooling and workflows.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 31, 2026

📝 Walkthrough

Walkthrough

Shader z-offset in the OFFSET_DEPTH path was reduced from 5.0 to 1.25. The WeatherEditor class gained a SupportsVR() override (duplicated declarations returning true), adding a public API indicating VR support.

Changes

Cohort / File(s) Summary
Shader Depth Offset
package/Shaders/Utility.hlsl
Z-offset constant in the OFFSET_DEPTH conditional changed from 5.0 to 1.25 (single-line numeric change).
WeatherEditor VR support
src/Features/WeatherEditor.h
Added two SupportsVR() overrides with identical signatures returning true (one inline, one non-inline) — duplicate declarations introduced in the header.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • alandtse
  • doodlum

Poem

🐰 In code I nibble, nibble light,
A shader shrinks its stumbling height,
And WeatherEditor nods, "I see VR!"
Two little methods, near and far,
Hoppity-hop — changes bright ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: reducing offset depth in terrain blending to fix ground artifacts, which aligns with the primary modification in Utility.hlsl.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@github-actions
Copy link
Copy Markdown

Using provided base ref: e9462c2
Using base ref: e9462c2
Base commit date: 2026-01-30T00:39:41-08:00 (Friday, January 30, 2026 12:39 AM)
No actionable suggestions for changed features.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/Features/WeatherEditor.h (1)

20-30: ⚠️ Potential issue | 🔴 Critical

Duplicate SupportsVR() declaration will cause a compilation error.

The same virtual override is declared twice (lines 20 and 30). This is invalid C++ and will fail to compile.

Remove line 30 to keep consistency with the other inline overrides in this class.

🐛 Proposed fix
 	virtual inline bool SupportsVR() override { return true; }
 	virtual inline std::pair<std::string, std::vector<std::string>> GetFeatureSummary() override
 	{
 		return {
 			"Development tool for editing weather, testing weather transitions, and managing weather-related feature settings.",
 			{ "Provides weather editing functionality",
 				"Includes dynamic saving and loading of vanilla post processing and weather settings.",
 				"Real-time editing and previewing of effects" }
 		};
 	}
-	virtual bool SupportsVR() override { return true; }

 	virtual void DataLoaded() override;

@ParticleTroned
Copy link
Copy Markdown
Contributor Author

messed up my commits - will close and make a new PR. Sorry

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.

1 participant