fix(weather-editor): disable viewport when HDR Display enabled#2175
Merged
alandtse merged 4 commits intoApr 22, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughEditorWindow now uses an HDR-aware query Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ImGui as ImGui (Menu/UI)
participant Editor as EditorWindow
participant HDR as Globals::HDR (enableHDR)
participant Blur as BackgroundBlur
participant Renderer as ViewportRenderer
rect rgba(200,200,255,0.5)
User->>ImGui: Toggle "Window → Viewport" checkbox
ImGui->>Editor: Query IsViewportActive()
Editor->>HDR: Read enableHDR
HDR-->>Editor: enableHDR (true/false)
Editor-->>ImGui: Return active = settings.showViewport && !enableHDR
end
rect rgba(200,255,200,0.5)
Note over Editor,Blur: On open/close or HDR toggle
Editor->>Editor: Draw() / UpdateOpenState() detects active change
Editor->>Blur: SetWeatherEditorActive(IsViewportActive())
Blur-->>Editor: ACK
end
rect rgba(255,200,200,0.5)
Note over Renderer,Editor: Render loop
Renderer->>Editor: Request viewport render
Editor->>HDR: Check enableHDR
Editor-->>Renderer: Render only if IsViewportActive()
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/WeatherEditor/EditorWindow.cpp`:
- Around line 1413-1414: UpdateOpenState currently forces settings.showViewport
= false when globals::features::hdrDisplay.settings.enableHDR is true, which
persists the change to disk; remove that assignment so user preference isn't
mutated. Instead, leave settings.showViewport untouched and gate actual viewport
activation and rendering on the HDR runtime flag (e.g., check hdrActive or
globals::features::hdrDisplay.settings.enableHDR) inside the code paths that
enable/draw the viewport — specifically modify RenderUI (the branch that renders
the viewport) and any viewport activation logic to short-circuit rendering when
HDR is active while preserving settings.showViewport for serialization.
🪄 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: 170af604-b8af-40ba-9372-c8687390ba82
📒 Files selected for processing (1)
src/WeatherEditor/EditorWindow.cpp
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/WeatherEditor/EditorWindow.cpp (1)
1415-1424:⚠️ Potential issue | 🟠 MajorKeep background blur synced when HDR changes at runtime.
Line 1418 only runs on the open transition. If HDR is enabled/disabled while the editor stays open,
Draw()and layout react viaIsViewportActive(), butBackgroundBlur::weatherEditorActivekeeps the old value until close/reopen.🐛 Proposed fix
if (open && !wasOpen) { DisableVanityCamera(); HideGameMenus(); - BackgroundBlur::SetWeatherEditorActive(IsViewportActive()); - - } else if (!open && wasOpen) { + } + + if (open) { + BackgroundBlur::SetWeatherEditorActive(IsViewportActive()); + } else if (wasOpen) { RestoreVanityCamera(); ShowGameMenus(); BackgroundBlur::SetWeatherEditorActive(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/EditorWindow.cpp` around lines 1415 - 1424, The background blur flag is only updated on open/close transitions, so when HDR toggles while the editor remains open BackgroundBlur::weatherEditorActive is stale; update the blur state whenever the editor is open and viewport activity can change (e.g. on each Draw() or in the HDR-change handler) by calling BackgroundBlur::SetWeatherEditorActive(IsViewportActive()) when the window is open (in addition to the existing transition code in the block containing DisableVanityCamera/RestoreVanityCamera), ensuring you reference the existing functions BackgroundBlur::SetWeatherEditorActive and IsViewportActive() to keep the blur state in sync.
🧹 Nitpick comments (1)
src/WeatherEditor/EditorWindow.h (1)
1-1: Shorten the PR title and add an issue reference if one exists.The title is conventional, but it exceeds the 50-character guideline. Consider:
fix(weather-editor): disable HDR viewport. If this fixes a tracked bug, addFixes #<id>to the PR description.As per coding guidelines, “Conventional Commit Titles… Length: 50 characters limit for title” and “Issue References… Suggest adding appropriate GitHub keywords.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/EditorWindow.h` at line 1, Shorten the pull request title to follow the 50-character conventional-commit guideline (for example: "fix(weather-editor): disable HDR viewport") and, if this change resolves a tracked bug, add an issue reference line in the PR description using GitHub keywords (e.g., "Fixes #<id>"). Update the PR title field and the PR description (not source files) accordingly so the commit/PR metadata adheres to the project's Conventional Commits and issue reference rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/WeatherEditor/EditorWindow.cpp`:
- Around line 1415-1424: The background blur flag is only updated on open/close
transitions, so when HDR toggles while the editor remains open
BackgroundBlur::weatherEditorActive is stale; update the blur state whenever the
editor is open and viewport activity can change (e.g. on each Draw() or in the
HDR-change handler) by calling
BackgroundBlur::SetWeatherEditorActive(IsViewportActive()) when the window is
open (in addition to the existing transition code in the block containing
DisableVanityCamera/RestoreVanityCamera), ensuring you reference the existing
functions BackgroundBlur::SetWeatherEditorActive and IsViewportActive() to keep
the blur state in sync.
---
Nitpick comments:
In `@src/WeatherEditor/EditorWindow.h`:
- Line 1: Shorten the pull request title to follow the 50-character
conventional-commit guideline (for example: "fix(weather-editor): disable HDR
viewport") and, if this change resolves a tracked bug, add an issue reference
line in the PR description using GitHub keywords (e.g., "Fixes #<id>"). Update
the PR title field and the PR description (not source files) accordingly so the
commit/PR metadata adheres to the project's Conventional Commits and issue
reference rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5a7a8cea-d34c-4505-8fe0-a5f308a9ec78
📒 Files selected for processing (2)
src/WeatherEditor/EditorWindow.cppsrc/WeatherEditor/EditorWindow.h
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/WeatherEditor/EditorWindow.cpp (1)
1-1: PR metadata suggestion: shorten the title and add an issue link if available.Suggested title:
fix(weather-editor): gate viewport for hdr. If this fixes a tracked bug, addFixes #<issue>orCloses #<issue>to the PR body. As per coding guidelines, conventional commit titles should usetype(scope): descriptionand stay within the 50-character title limit, and bug-fix PRs should include GitHub issue keywords where applicable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/EditorWindow.cpp` at line 1, The PR title is too long and missing an issue reference; update the PR title to a concise conventional-commit style like "fix(weather-editor): gate viewport for hdr" and, if this change fixes a tracked bug, add an issue-closing keyword in the PR body such as "Fixes #<issue>" or "Closes #<issue>" so maintainers can auto-link and close the issue; apply this change to the PR that modifies src/WeatherEditor/EditorWindow.cpp and ensure the title stays within ~50 characters and follows type(scope): description format.
🤖 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/WeatherEditor/EditorWindow.cpp`:
- Around line 998-1008: The viewport disable logic uses
globals::features::hdrDisplay.settings.enableHDR only; change it to check the
feature is loaded and enabled (globals::features::hdrDisplay.loaded &&
globals::features::hdrDisplay.settings.enableHDR) before calling
ImGui::BeginDisabled()/EndDisabled() and before any IsViewportActive-style
checks so the viewport isn't blocked when HDR is configured but not actually
active; update the block around the ImGui::Checkbox("Viewport",
&settings.showViewport) and the other similar block in EditorWindow.cpp that
uses hdrDisplay.settings.enableHDR so both use hdrDisplay.loaded &&
hdrDisplay.settings.enableHDR and keep calls to
BackgroundBlur::SetWeatherEditorActive and Save() unchanged.
---
Nitpick comments:
In `@src/WeatherEditor/EditorWindow.cpp`:
- Line 1: The PR title is too long and missing an issue reference; update the PR
title to a concise conventional-commit style like "fix(weather-editor): gate
viewport for hdr" and, if this change fixes a tracked bug, add an issue-closing
keyword in the PR body such as "Fixes #<issue>" or "Closes #<issue>" so
maintainers can auto-link and close the issue; apply this change to the PR that
modifies src/WeatherEditor/EditorWindow.cpp and ensure the title stays within
~50 characters and follows type(scope): description format.
🪄 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: 2a287a23-3f9d-4cea-b848-616dfd6a0fe9
📒 Files selected for processing (1)
src/WeatherEditor/EditorWindow.cpp
alandtse
approved these changes
Apr 22, 2026
alandtse
changed the title
This was referenced Apr 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
the viewport has compatibility issues with HDR
Summary by CodeRabbit