feat(LLF): add debug overlay in llf visualiser#1666
Conversation
📝 WalkthroughWalkthroughThe LightLimitFix feature is refactored to inherit from OverlayFeature, adding visual debugging capabilities through an overlay rendering system. New methods expose per-frame visualization settings, and a DrawOverlay() implementation renders a debug ImGui window when enabled. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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: 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/LightLimitFix.cpp (1)
10-13: Critical: AddEnableLightsVisualisationto serialization.The
EnableLightsVisualisationsetting is not included in the serialization macro, which means the user's debug visualization preference won't persist across game restarts. This could lead to confusion, especially since the feature is described as needing a warning to prevent users from thinking their game broke.🔎 Proposed fix
NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT( LightLimitFix::Settings, EnableContactShadows, + EnableLightsVisualisation, LightsVisualisationMode)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Features/LightLimitFix.cppsrc/Features/LightLimitFix.h
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Features/LightLimitFix.cppsrc/Features/LightLimitFix.h
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Features/LightLimitFix.cppsrc/Features/LightLimitFix.h
**/*
⚙️ CodeRabbit configuration file
**/*: When reviewing PRs, please provide suggestions for:
Conventional Commit Titles (if not following https://www.conventionalcommits.org/ or
if the existing title does not describe the code changes):
Format: type(scope): description
Length: 50 characters limit for title, 72 for body
Style: lowercase description, no ending period
Examples:
- feat(vr): add cross-eye sampling
- fix(water): resolve flowmap bug
- docs: update shader documentation
Issue References (if PR fixes bugs or implements features):
Suggest adding appropriate GitHub keywords:Otherwise, use your standard review approach focusing on code quality.
Files:
src/Features/LightLimitFix.cppsrc/Features/LightLimitFix.h
🧠 Learnings (1)
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/**/*.{cpp,cxx,cc,h,hpp,hxx} : Feature classes must inherit from Feature (src/Feature.h) and implement DrawSettings(), LoadSettings(), and SaveSettings()
Applied to files:
src/Features/LightLimitFix.h
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (4)
src/Features/LightLimitFix.h (2)
4-6: LGTM! Clean refactor to overlay feature.The inheritance change from
FeaturetoOverlayFeatureis appropriate for adding overlay rendering capabilities. The include change aligns with the new base class.
159-160: LGTM! Proper overlay interface implementation.The
DrawOverlay()andIsOverlayVisible()overrides correctly implement the overlay feature interface. The inline implementation ofIsOverlayVisible()is clean and appropriate.src/Features/LightLimitFix.cpp (2)
56-64: LGTM! Clear and prominent debug warning.The overlay implementation correctly:
- Guards against rendering when disabled (early return)
- Uses appropriate ImGui flags for a fixed debug overlay
- Displays a prominent, color-coded warning message
The message "DEBUG FEATURE - LIGHT LIMIT VISUALISATION ENABLED" aligns well with the PR objective to warn users and prevent confusion.
66-73: LGTM! Proper per-frame data propagation.The implementation correctly populates all three fields in the
PerFramestruct:
EnableLightsVisualisationfrom settingsLightsVisualisationModefrom settingsClusterSizearray usingstd::copyThis ensures shader code receives the current visualization state.
|
✅ A pre-release build is available for this PR: |
|
iirc when RenderDoc is on it has a constant overlay on the top right, you might able to copy over most of that logic to keep the design consistent |
* build: bump eastl to 3.27.01 (community-shaders#1673) Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> * fix(grass collision): ignore hkpListShape (community-shaders#1661) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore(upscaling): update SDKs (community-shaders#1684) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore(terrain shadows): modernize (community-shaders#1678) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * feat: add user wiki link to readme (community-shaders#1689) * feat(LLF): add debug overlay in llf visualiser (community-shaders#1666) * feat: linear lighting (community-shaders#1359) Co-authored-by: jiayev <jiayev@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * feat: weather and imagespace editor (community-shaders#1630) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(ui): load previously selected theme on startup (community-shaders#1664) --------- Co-authored-by: Alan Tse <alandtse@users.noreply.github.com> Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: jiayev <l936249247@hotmail.com> Co-authored-by: jiayev <jiayev@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
just should add a text to the top left stating debug mode on to stop new users accidentally toggling it and thinking their game broke.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.