Skip to content

perf(VR): reduce copy operations#2114

Merged
doodlum merged 2 commits into
community-shaders:devfrom
alandtse:dlss_perf_review
Apr 15, 2026
Merged

perf(VR): reduce copy operations#2114
doodlum merged 2 commits into
community-shaders:devfrom
alandtse:dlss_perf_review

Conversation

@alandtse
Copy link
Copy Markdown
Collaborator

@alandtse alandtse commented Apr 14, 2026

image

Red is pre-PR. Testing with 33% downscale. This is frame time if I'm reading it right

Summary by CodeRabbit

  • New Features

    • VR + FSR now produces per-eye linear depth and emits per-eye motion, reactive, and transparency masks to improve stereo fidelity and compatibility.
  • Refactor

    • Per-eye sampling and dispatch updated to avoid cross-eye contamination while preserving non‑VR behavior; VR input preparation simplified to reduce redundant copies and streamline per‑eye processing.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 14, 2026

📝 Walkthrough
📝 Walkthrough
🚥 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
Title check ✅ Passed The title 'perf(VR): reduce copy operations' directly corresponds to the main objective of the changeset: removing motion vector, reactive mask, and transparency inputs from per-eye preparation while introducing a shader variant for FSR depth output, effectively reducing redundant copy operations in VR+FSR pipelines.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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: 1

🧹 Nitpick comments (1)
src/Features/Upscaling.cpp (1)

1065-1072: Skip the DLSS-only depth copy on the VR FSR path.

Line 1068 still copies depthSrc into vrIntermediateDepth, but VR FSR now consumes vrIntermediateLinearDepth from EncodeTexturesCS and never reads vrIntermediateDepth. That means one of the old per-eye copies is still happening every FSR VR frame. Gate this copy on UpscaleMethod::kDLSS; once that lands, vrIntermediateDepth can become DLSS-only allocation as well.

♻️ Suggested change
-		// Depth copy keeps vrIntermediateDepth populated for DLSS (Streamline handles R24G8_TYPELESS).
-		// FSR uses vrIntermediateLinearDepth (R32_FLOAT) written by EncodeTexturesCS instead.
-		context->CopySubresourceRegion(vrIntermediateDepth[i]->resource.get(), 0, 0, 0, 0, depthSrc, 0, &srcBox);
+		if (GetUpscaleMethod() == UpscaleMethod::kDLSS) {
+			// Streamline consumes the per-eye typed depth copy.
+			context->CopySubresourceRegion(vrIntermediateDepth[i]->resource.get(), 0, 0, 0, 0, depthSrc, 0, &srcBox);
+		}
As per coding guidelines "Consider GPU workload and performance impact when implementing graphics features, with special attention to shader compilation and runtime performance".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Features/Upscaling.cpp` around lines 1065 - 1072, The per-eye depth copy
into vrIntermediateDepth (the context->CopySubresourceRegion call that copies
depthSrc into vrIntermediateDepth[i]) should be performed only for DLSS; wrap or
gate that copy behind a check using GetUpscaleMethod() == UpscaleMethod::kDLSS
so VR FSR frames skip the redundant depth copy (leave the
vrIntermediateMotionVectors copy logic unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Features/Upscaling/FidelityFX.cpp`:
- Around line 347-350: The early-return when fsrScratchBuffer is null in
FidelityFX.cpp silently skips the upscale and leaves the pipeline in a broken
state; instead replace the no-op return with a recovery path: attempt to
recreate the FSR resource (call your FSR init helper, e.g.,
InitFSRResources/CreateFSRScratchBuffer) and if recreation fails set a durable
fallback flag (e.g., Upscaling::disableFSR or Upscaling::fsrAvailable=false) or
return an error/status to the caller so PerformUpscaling() will not proceed into
UpscaleDepth(); also ensure Upscaling::PerformUpscaling() checks that flag or
the returned status before calling UpscaleDepth() so future frames either retry
creation or use the non-downscaled path.

---

Nitpick comments:
In `@src/Features/Upscaling.cpp`:
- Around line 1065-1072: The per-eye depth copy into vrIntermediateDepth (the
context->CopySubresourceRegion call that copies depthSrc into
vrIntermediateDepth[i]) should be performed only for DLSS; wrap or gate that
copy behind a check using GetUpscaleMethod() == UpscaleMethod::kDLSS so VR FSR
frames skip the redundant depth copy (leave the vrIntermediateMotionVectors copy
logic unchanged).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fb3d3c9c-44fe-40d2-9984-6d3227e1f52e

📥 Commits

Reviewing files that changed from the base of the PR and between 824db38 and f08138e.

📒 Files selected for processing (5)
  • features/Upscaling/Shaders/Upscaling/EncodeTexturesCS.hlsl
  • src/Features/Upscaling.cpp
  • src/Features/Upscaling.h
  • src/Features/Upscaling/FidelityFX.cpp
  • src/Features/Upscaling/Streamline.cpp

Comment thread src/Features/Upscaling/FidelityFX.cpp Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 14, 2026

✅ A pre-release build is available for this PR:
Download

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: 1

🧹 Nitpick comments (1)
src/Features/Upscaling.h (1)

154-155: vrIntermediateLinearDepth is misnamed.

This resource is an R32_FLOAT copy of device depth, not a linearized depth buffer. The current name/comment makes it easy to apply a second linearization later or bind it under the wrong assumption. Renaming it to something like vrIntermediateTypedDepth — or at least correcting the comment — would make the contract clearer.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Features/Upscaling.h` around lines 154 - 155, The member
vrIntermediateLinearDepth is misnamed — it is a typed R32_FLOAT copy of device
depth, not a linearized depth buffer — so rename vrIntermediateLinearDepth (and
its uses) to vrIntermediateTypedDepth (or similar) and update the comment to
"per-eye render resolution (R32_FLOAT, typed copy of device depth)" to avoid
confusion; ensure you update all references/usages in code (bindings,
shader/resource setup, and any checks that assumed "linear" semantics) and keep
the related vrIntermediateDepth and Texture2D symbol usages consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Features/Upscaling.cpp`:
- Around line 1066-1072: Unconditional per-eye depth CopySubresourceRegion is
performed into vrIntermediateDepth even when using FSR (which consumes
vrIntermediateLinearDepth produced by EncodeTexturesCS), so change the logic to
skip the depth copy when GetUpscaleMethod() == UpscaleMethod::kFSR;
specifically, guard the context->CopySubresourceRegion call that writes
vrIntermediateDepth[i]->resource with a check that only runs it for DLSS (or
when GetUpscaleMethod() != UpscaleMethod::kFSR), leaving the existing motion
vector copy logic and references to depthSrc and srcBox unchanged.

---

Nitpick comments:
In `@src/Features/Upscaling.h`:
- Around line 154-155: The member vrIntermediateLinearDepth is misnamed — it is
a typed R32_FLOAT copy of device depth, not a linearized depth buffer — so
rename vrIntermediateLinearDepth (and its uses) to vrIntermediateTypedDepth (or
similar) and update the comment to "per-eye render resolution (R32_FLOAT, typed
copy of device depth)" to avoid confusion; ensure you update all
references/usages in code (bindings, shader/resource setup, and any checks that
assumed "linear" semantics) and keep the related vrIntermediateDepth and
Texture2D symbol usages consistent.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0bdc32f6-a71c-4523-93ec-4981a7df88f2

📥 Commits

Reviewing files that changed from the base of the PR and between f08138e and 3c14c3e.

📒 Files selected for processing (5)
  • features/Upscaling/Shaders/Upscaling/EncodeTexturesCS.hlsl
  • src/Features/Upscaling.cpp
  • src/Features/Upscaling.h
  • src/Features/Upscaling/FidelityFX.cpp
  • src/Features/Upscaling/Streamline.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/Features/Upscaling/FidelityFX.cpp
  • src/Features/Upscaling/Streamline.cpp

Comment thread src/Features/Upscaling.cpp
@alandtse alandtse marked this pull request as draft April 14, 2026 07:56
@alandtse alandtse marked this pull request as ready for review April 15, 2026 06:08
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.

🧹 Nitpick comments (1)
src/Features/Upscaling.cpp (1)

1066-1073: Consider caching GetUpscaleMethod() before the loop.

GetUpscaleMethod() is called twice per iteration (lines 1068 and 1072). While it's a simple getter, hoisting it before the loop improves clarity and consistency with the pattern suggested in the past review.

♻️ Proposed refactor
 void Upscaling::PreparePerEyeInputs(ID3D11Resource* colorSrc, ID3D11Resource* depthSrc)
 {
 	if (!globals::game::isVR)
 		return;

 	auto state = globals::state;
 	if (state->frameAnnotations)
 		state->BeginPerfEvent("VR Upscaling Prepare");

 	auto context = globals::d3d::context;
 	auto renderSize = Util::ConvertToDynamic(globals::state->screenSize);

 	uint32_t eyeWidthIn = (uint32_t)(renderSize.x / 2);
 	uint32_t eyeHeightIn = (uint32_t)renderSize.y;

 	// Textures guaranteed to exist: EnsureVRIntermediateTextures() was called in Upscale()
 	auto& depthTexture = globals::game::renderer->GetDepthStencilData().depthStencils[RE::RENDER_TARGETS_DEPTHSTENCIL::kMAIN];
 	auto& motionVectorRT = globals::game::renderer->GetRuntimeData().renderTargets[RE::RENDER_TARGETS::kMOTION_VECTOR];

+	const auto method = GetUpscaleMethod();
 	for (uint32_t i = 0; i < 2; ++i) {
 		uint32_t offsetXIn = (i == 1) ? eyeWidthIn : 0;
 		D3D11_BOX srcBox = { offsetXIn, 0, 0, offsetXIn + eyeWidthIn, eyeHeightIn, 1 };

 		context->CopySubresourceRegion(vrIntermediateColorIn[i]->resource.get(), 0, 0, 0, 0, colorSrc, 0, &srcBox);
 		// Depth copy keeps vrIntermediateDepth populated for DLSS (Streamline handles R24G8_TYPELESS).
 		// FSR uses vrIntermediateLinearDepth (R32_FLOAT) written by EncodeTexturesCS instead.
-		if (GetUpscaleMethod() == UpscaleMethod::kDLSS)
+		if (method == UpscaleMethod::kDLSS)
 			context->CopySubresourceRegion(vrIntermediateDepth[i]->resource.get(), 0, 0, 0, 0, depthSrc, 0, &srcBox);
 		// DLSS motion vectors are written per-eye by EncodeTexturesCS with 5x5 dilation.
 		// FSR uses a raw copy here since it does not use the dilated output.
-		if (GetUpscaleMethod() != UpscaleMethod::kDLSS)
+		if (method != UpscaleMethod::kDLSS)
 			context->CopySubresourceRegion(vrIntermediateMotionVectors[i]->resource.get(), 0, 0, 0, 0, motionVectorRT.texture, 0, &srcBox);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Features/Upscaling.cpp` around lines 1066 - 1073, Hoist the repeated call
to GetUpscaleMethod() out of the loop by calling it once into a local variable
(e.g., auto upscaleMethod = GetUpscaleMethod()) before the loop and then use
upscaleMethod in the two conditionals that currently compare to
UpscaleMethod::kDLSS; update the branches that reference vrIntermediateDepth,
vrIntermediateMotionVectors and the CopySubresourceRegion calls to use the
cached upscaleMethod to avoid calling GetUpscaleMethod() twice per iteration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/Features/Upscaling.cpp`:
- Around line 1066-1073: Hoist the repeated call to GetUpscaleMethod() out of
the loop by calling it once into a local variable (e.g., auto upscaleMethod =
GetUpscaleMethod()) before the loop and then use upscaleMethod in the two
conditionals that currently compare to UpscaleMethod::kDLSS; update the branches
that reference vrIntermediateDepth, vrIntermediateMotionVectors and the
CopySubresourceRegion calls to use the cached upscaleMethod to avoid calling
GetUpscaleMethod() twice per iteration.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2a56c318-8d6a-4498-90cc-9039fedf224c

📥 Commits

Reviewing files that changed from the base of the PR and between 3c14c3e and 0c64859.

📒 Files selected for processing (1)
  • src/Features/Upscaling.cpp

@alandtse
Copy link
Copy Markdown
Collaborator Author

@coderabbitai the point of this PR was to reduce the copy operations in VR. Can you please confirm that is the effect? Please identify the number and type before and after. Please also consider whether the texture changes have any impact. Please focus on the actual code changes and critically review any comments made.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 15, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all copy operations in the upscaling-related files
echo "=== CopyResource / CopySubresource calls in Upscaling.cpp ==="
rg -n "CopyResource|CopySubresource|CopyTexture" src/Features/Upscaling.cpp

echo ""
echo "=== CopyResource / CopySubresource calls in FidelityFX.cpp ==="
rg -n "CopyResource|CopySubresource|CopyTexture" src/Features/Upscaling/FidelityFX.cpp

echo ""
echo "=== CopyResource / CopySubresource calls in Streamline.cpp ==="
rg -n "CopyResource|CopySubresource|CopyTexture" src/Features/Upscaling/Streamline.cpp

echo ""
echo "=== Full PreparePerEyeInputs implementation in Upscaling.cpp ==="
rg -n -A 60 "PreparePerEyeInputs" src/Features/Upscaling.cpp

echo ""
echo "=== EnsureVRIntermediateTextures ==="
rg -n -A 40 "EnsureVRIntermediateTextures" src/Features/Upscaling.cpp

echo ""
echo "=== VR intermediate texture recreation section ==="
rg -n -B2 -A10 "vrIntermediate" src/Features/Upscaling.cpp | head -200

Length of output: 769


🏁 Script executed:

#!/bin/bash
# Get the git diff to see exactly what was changed
git diff origin/dev...HEAD -- src/Features/Upscaling.cpp src/Features/Upscaling.h src/Features/Upscaling/FidelityFX.cpp src/Features/Upscaling/Streamline.cpp features/Upscaling/Shaders/Upscaling/EncodeTexturesCS.hlsl

Length of output: 24013


@alandtse: Yes, the PR does achieve a net reduction in GPU copy operations in VR. Here is a precise before/after breakdown based on the diff.


Copy Operations: Before vs. After

Old PreparePerEyeInputs (5 parameters)

For each of 2 eyes, 5 CopySubresourceRegion calls were issued:

# Resource Notes
1 vrIntermediateColorIn[i] color
2 vrIntermediateDepth[i] depth
3 vrIntermediateMotionVectors[i] motion vectors
4 vrIntermediateTransparencyMask[i] transparency
5 vrIntermediateReactiveMask[i] reactive

Total: 10 CopySubresourceRegion calls per frame (5 × 2 eyes).


New PreparePerEyeInputs (2 parameters)

For each of 2 eyes, at most 2 CopySubresourceRegion calls are issued:

# Resource Condition
1 vrIntermediateColorIn[i] always
2 vrIntermediateDepth[i] DLSS only
2 vrIntermediateMotionVectors[i] FSR only

Total: 4 CopySubresourceRegion calls per frame (2 × 2 eyes), regardless of method.

Net reduction: 6 CopySubresourceRegion calls eliminated per frame.


What Replaces the Eliminated Copies?

The 3 removed copies per eye (reactive, transparency, and conditionally depth or mvec) are replaced by direct compute shader UAV writes in EncodeTexturesCS:

Eliminated copy Replaced by
vrIntermediateReactiveMask copy EncodeTexturesCS writes UAV u0 directly per eye
vrIntermediateTransparencyMask copy EncodeTexturesCS writes UAV u1 directly per eye
vrIntermediateDepth copy (FSR only) EncodeTexturesCS DEPTH_OUTPUT variant writes vrIntermediateLinearDepth UAV u3
vrIntermediateMotionVectors copy (DLSS only) Already written by EncodeTexturesCS (5×5 dilated mvec), no change

The EncodeTexturesCS dispatch goes from 1 dispatch (full stereo width) to 2 dispatches (per-eye width) in VR, but the total texels processed is identical — only the dispatch API call count doubles, which is negligible overhead.


Texture Changes and Their Impact

Three categories of texture changes:

1. vrIntermediateDepth: R32_TYPELESSR24G8_TYPELESS (correctness fix)

This is a genuine bug fix. The code comment explains that R32_TYPELESS is in a different DXGI cast group from the game's D24S8_TYPELESS depth buffer. CopySubresourceRegion requires matching typeless format cast groups, so the old format would produce silent zero-copy failures on some drivers. The SRV was updated accordingly (R32_FLOATR24_UNORM_X8_TYPELESS). This fix is needed to make the DLSS depth copy actually work.

2. New vrIntermediateLinearDepth[2]: R32_FLOAT (FSR depth path)

Added per-eye R32_FLOAT texture with both SRV and UAV. Required because FSR's GetFfxResourceDescriptionDX11() returns FFX_SURFACE_FORMAT_UNKNOWN for R24G8_TYPELESS — it cannot consume the typeless format directly. EncodeTexturesCS (VR+FSR variant) reads from the typeless SRV and writes the depth value as R32_FLOAT into this texture. Note the depth value written is the raw non-linear depth, not a linearized value despite the texture name vrIntermediateLinearDepth. The name may be mildly misleading.

3. vrIntermediateMotionVectors/ReactiveMask/TransparencyMask: UAV now enabled

Previously created with createUAV = false; now true. This is a required prerequisite for EncodeTexturesCS to write directly into these per-eye buffers.


Other Observations

  • Null constant buffer slot fix: The CB null/unbind after dispatch was clearing slot 7 instead of slot 0. Now correctly clears slot 0. This was likely causing the constant buffer to remain bound on the wrong slot.
  • EnsureVRIntermediateTextures() is now called from Upscale() before the EncodeTexturesCS dispatch, meaning the textures are guaranteed to exist when the CS writes into them — which is important since PreparePerEyeInputs previously handled recreation.
  • The recreation condition for VR intermediates now includes !vrIntermediateLinearDepth[0], correctly triggering recreation if the new texture is missing.

Conclusion: The PR does reduce copy operations — from 10 to 4 CopySubresourceRegion calls per VR frame. The depth format fix is a correctness improvement that likely also resolves a latent zero-depth bug for DLSS in VR. The trade-off is 1 extra compute dispatch per frame in VR (2 instead of 1), which is negligible since the total work is identical.


🧠 Learnings used
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.

Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 2084
File: src/ShaderCache.cpp:1646-1649
Timestamp: 2026-04-10T07:34:27.849Z
Learning: In `src/ShaderCache.cpp` (doodlum/skyrim-community-shaders PR `#2084`), `GetImagespaceShaderDescriptor` intentionally returns `false` for VR when a shader is not reverse-engineered. Returning `false` causes `ResolveImageSpaceDescriptor` to return `false`, which makes all callers (GetVertexShader, GetPixelShader, GetComputeShader, CompileShader) skip custom shader handling and defer to the vanilla shader. This is the correct pattern for un-RE'd imagespace shaders on VR — do not flag it as "skipping all custom VR handling" or suggest gating on HDR feature state.

Learnt from: vrnord
Repo: doodlum/skyrim-community-shaders PR: 1982
File: package/Shaders/Lighting.hlsl:3089-3095
Timestamp: 2026-03-16T13:13:11.490Z
Learning: In `package/Shaders/Lighting.hlsl` (doodlum/skyrim-community-shaders), the `TREE_ANIM` alpha test path intentionally uses a hardcoded `0.1` floor check (`if (alpha < 0.1) discard;`) and `AlphaTestRefRS` rather than `SharedData::VRAlphaTestThreshold`. `VRAlphaTestThreshold` is wired into `DistantTree.hlsl` and `RunGrass.hlsl` only. The threshold wiring for `Lighting.hlsl`'s `TREE_ANIM` path is deferred to a separate VR foliage PR.

Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-25T04:43:36.075Z
Learning: Applies to src/**/*.{h,hpp,cpp} : Ensure code changes work across SE/AE/VR Skyrim variants using runtime detection and `REL::RelocateMember()` patterns for compatibility

Learnt from: vrnord
Repo: doodlum/skyrim-community-shaders PR: 1982
File: src/Features/VR/StereoBlend.cpp:98-109
Timestamp: 2026-03-16T13:13:31.383Z
Learning: In `src/Features/VR/StereoBlend.cpp` (doodlum/skyrim-community-shaders), debug modes 4/5 ("Overwrite", "Overwrite Eye1") in the `StereoBlendDebugMode` selector are NOT separate shader variants. They use the same `stereoBlendOverwriteCS` shader with different cbuffer parameters. When `vrStereoOptActive` is true, `stereoBlendOverwriteCS` is always selected unconditionally and `StereoBlendDebugMode` values 4/5 do not map to any distinct code path.

Learnt from: Dlizzio
Repo: doodlum/skyrim-community-shaders PR: 1950
File: package/Shaders/Lighting.hlsl:2008-2012
Timestamp: 2026-03-07T00:53:21.267Z
Learning: In doodlum/skyrim-community-shaders PR `#1950`, the maintainer (Dlizzio) confirmed that per-axis normal reorientation for triplanar-projected normals/detail in package/Shaders/Lighting.hlsl is not required for their use case; the current approach intentionally mixes samples without axis-specific swizzle/sign correction.

Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-25T04:43:36.075Z
Learning: Applies to src/Features/**/*.{h,hpp,cpp,hlsl} : Consider GPU workload and performance impact when implementing graphics features, with special attention to shader compilation and runtime performance

Learnt from: Dlizzio
Repo: doodlum/skyrim-community-shaders PR: 2000
File: src/WeatherEditor/EditorWindow.cpp:1271-1274
Timestamp: 2026-03-22T18:40:49.633Z
Learning: Repo: doodlum/skyrim-community-shaders — Maintainer preference (PR `#2000` on 2026-03-22): In src/WeatherEditor/EditorWindow.cpp within EditorWindow::RenderUI, keep `resetLayout = false;` at the end of the frame (after PaletteWindow::Draw). The reset is intentionally one-shot for windows rendered that frame; closed windows are allowed to reopen with their previous geometry. Do not move the clear later or broadcast resets to closed windows.

Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.

Learnt from: vrnord
Repo: doodlum/skyrim-community-shaders PR: 2003
File: features/Upscaling/Shaders/Upscaling/DepthUpscalePS.hlsl:55-61
Timestamp: 2026-03-24T04:30:32.809Z
Learning: In `features/Upscaling/Shaders/Upscaling/DepthUpscalePS.hlsl` (doodlum/skyrim-community-shaders PR `#2003`), the shader is dispatched per-eye using per-eye dimensions (DstWidth/DstHeight). The UV (`input.TexCoord`) is derived from `dispatchID / DstDim`, which is already eye-local — not full SBS/packed stereo coordinates. The per-eye UV clamping logic (isRight, halfScale) operates entirely in eye-local space, and there is no stereo seam issue. Do not flag SBS seam concerns for this shader.

Learnt from: jiayev
Repo: doodlum/skyrim-community-shaders PR: 1947
File: package/Shaders/DeferredCompositeCS.hlsl:234-243
Timestamp: 2026-03-14T08:35:42.651Z
Learning: In `package/Shaders/DeferredCompositeCS.hlsl` (doodlum/skyrim-community-shaders), the fallback non-IBL SKYLIGHTING specular path intentionally double-weights with `skylightingSpecular`: `dalcScaled` is computed as `IrradianceToGamma(IrradianceToLinear(directionalAmbientColorSpecular) * skylightingSpecular)`, and the final `finalIrradiance` is then `lerp(specularIrradiance, specularIrradianceReflections, skylightingSpecular)`. This is original engine logic preserved from before PR `#1947` and should not be flagged as a double-attenuation bug.

@doodlum doodlum merged commit 34224d3 into community-shaders:dev Apr 15, 2026
30 checks passed
YtzyFvra pushed a commit to YtzyFvra/skyrim-community-shaders that referenced this pull request Apr 19, 2026
ParticleTroned added a commit to ParticleTroned/skyrim-community-shaders that referenced this pull request May 2, 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