fix(terrain blending): VR and when VL disabled#1738
Conversation
📝 WalkthroughWalkthroughAdds a call to update camera data (SetCameraData) on the global graphics state at the start of TerrainBlending's Main_RenderDepth hook, and changes TerrainBlending::SupportsVR() to return true. Changes
Sequence Diagram(s)sequenceDiagram
participant RenderThunk as Main_RenderDepth (thunk)
participant GraphicsState as globals::game::graphicsState
participant Camera as RE::NiCamera
participant RenderLogic as Depth / AvgEyePos
RenderThunk->>GraphicsState: SetCameraData(state, Camera, flags)
GraphicsState-->>RenderThunk: camera data updated
RenderThunk->>RenderLogic: compute average eye position
RenderLogic-->>RenderThunk: continue depth rendering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/Features/TerrainBlending.cpp`:
- Line 214: Guard the call to singleton.gSetCameraData by first checking that
the function pointer/resolver (singleton.gSetCameraData) is non-null and that
RE::Main::WorldRootCamera() returns a non-null pointer before invoking it; if
either check fails, skip the call and degrade gracefully (e.g., return early or
log via globals::game::graphicsState) to avoid a null deref/crash. Ensure you
reference singleton.gSetCameraData, RE::Main::WorldRootCamera(), and
globals::game::graphicsState when adding the checks so the intent and fallback
are clear.
🧹 Nitpick comments (1)
src/Features/TerrainBlending.h (1)
59-59: Conventional commit title & issue referenceThe PR title is missing the colon after the scope. Consider something like:
fix(terrain-blending): restore blending w/ vl off(fits the 50‑char target).
If there’s a tracking issue, please add “Fixes #” in the PR description.
|
✅ A pre-release build is available for this PR: |
|
has this been tested? should probably work on vr now too |
Yes, tested AE and SE uses same code |
|
I've confirmed the function is in VR too. Will add to commonlib. Would be good to know how to test this or we can blind merge it for VR. |
Pretty much just go into VR with terrain blending on and see if it works when adding this function. Probably though this function is just marking something as DIRTY, so might not be needed at all if we use state update flags. |
|
Confirmed it works in flat, but it breaks VR's VL. Probably need some more RE. EDIT: Double checking, it may not be this that broke VL in VR. It may no do anything in VR though. Can't figure out TB testing. |
I just looked, TBs feature flag for VR support set to false so i assume it's never been compatible. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/Features/TerrainBlending.h`:
- Line 102: The SupportsVR override in TerrainBlending.h currently
unconditionally returns true (method: SupportsVR), which breaks volumetric
lighting in VR; change it to return false until VR volumetric lighting issues
are resolved, or implement a runtime guard in SupportsVR (or an associated
initialization path in the TerrainBlending class) that detects VR + volumetric
lighting and returns false in that configuration so the feature gracefully
degrades; reference and update the SupportsVR() override and any TerrainBlending
initialization hooks to add the conditional check or revert to false.
- Around line 59-60: The function pointer gSetCameraData is assigned from
REL::RelocationID(75694, 77503).address() without validation and is later
invoked unconditionally; add a null-check to avoid a crash or handle the failed
relocation. In SetupResources() check the relocation result and log an error if
REL::RelocationID(...) fails (and leave gSetCameraData nullptr), and before any
invocation of gSetCameraData (the call site that currently assumes it is valid)
guard the call with if (gSetCameraData) { gSetCameraData(...) } else { log a
concise warning and skip the call } so the code degrades gracefully when the
relocation is unresolved.
|
@Dawntic apologies. I was just making a comment for the record. I test in developer mode so the flag is ignored. If it did fix VR, I would've made the edit to bring it back on. Regardless, go ahead and turn off VR support for now to keep this PR focused for merge. |
|
Ok after further testing including reenabling the terrain blending enable toggle, I can confirm this fixes the reported issue and reenables VR. I'll do a PR for TB toggling and push some commits here for the VR support. |
|
Actually, didn't end up being able to toggle the VR support again. Please go ahead and reenable and we can point some VR testers here. The toggle pr is #1755. Ultimately before merging this though, I think we should get commonlib merged so we can convert to using the new state class function SetCameraData. #1736 |
a82b2ff to
48ecee3
Compare
okay so TB works for VR now? I've changed it to use clib function and enabled VR again, should be all good hopefully. |
This PR addresses an issue where terrain blending breaks when volumetric lighting is disabled.
The issue is caused by a missing function call that updates camera data. When volumetric lighting is turned off the given function is never called(see pic below).
This PR adds a call to the missing function inside the main depth hook.
Func ref: https://github.com/Nukem9/skyrimse-test/blob/master/skyrim64_test/src/patches/TES/BSGraphics/BSGraphicsState.cpp
Control flow:

With VL disabled before

With VL disabled now

Summary by CodeRabbit
Refactor
New Features
✏️ Tip: You can customize this high-level summary in your review settings.