Repository navigation
Conversation
📝 WalkthroughWalkthroughChangesThe window API now supports fullscreen, maximize, restore, and state queries. The Wayland backend tracks compositor fullscreen state and sends fullscreen, maximize, and restore requests. Public methods delegate to the backend with null-implementation guards. Wayland window control contracts
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Window
participant WaylandWindow
participant Compositor
Window->>WaylandWindow: SetFullScreen(value)
WaylandWindow->>Compositor: Send fullscreen request
Compositor-->>WaylandWindow: Configure toplevel state
WaylandWindow->>WaylandWindow: Update m_isFullScreen
Window->>WaylandWindow: GetState()
WaylandWindow-->>Window: Return WindowState
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Graphics/Window/WaylandWindow.cpp`:
- Around line 1068-1079: Update the Wayland configure handling to assign
m_isMaximized directly from the current configured maximized value on every
configure, including when XDG_TOPLEVEL_STATE_MAXIMIZED is omitted. Locate the
configure state update feeding WaylandWindow::GetState(), and preserve the
existing maximize detection while ensuring tiled configurations clear the flag.
- Around line 1215-1225: Update WaylandWindow::SetFullScreen so it only sends
fullscreen requests and no longer changes m_isFullScreen directly; track
requested-state deduplication separately if needed. In
xdg_toplevel_handle_configure(), update m_isFullScreen from the
compositor-provided XDG fullscreen state, ensuring GetState() reflects committed
configure state rather than the pending request.
In `@src/Graphics/Window/Window.cpp`:
- Around line 151-173: Update Window::SetFullScreen(), Window::Maximize(), and
Window::Restore() to reject commands until the platform implementation is fully
initialized, not merely when m_windowImpl is non-null. Add the appropriate
validity guard for the underlying window state before invoking the corresponding
m_windowImpl methods, while preserving the existing error-and-return behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cae895f3-62d8-4d7b-8653-313fffe41611
📒 Files selected for processing (5)
inc/Graphics/Window/BasicWindowImpl.hinc/Graphics/Window/WaylandWindow.hinc/Graphics/Window/Window.hsrc/Graphics/Window/WaylandWindow.cppsrc/Graphics/Window/Window.cpp
Comment on lines
+1068
to
+1079
| WindowState WaylandWindow::GetState() const { | ||
| if (m_isFullScreen) { | ||
| return WindowState::FullScreen; | ||
| } | ||
| if (m_isMaximized) { | ||
| return WindowState::Maximized; | ||
| } | ||
| if (m_collapsed) { | ||
| return WindowState::Collapsed; | ||
| } | ||
| return WindowState::Default; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'WaylandWindow|Window|WindowState' | sed -n '1,120p'
echo "== WaylandWindow relevant lines =="
wc -l src/Graphics/Window/WaylandWindow.cpp
sed -n '1020,1245p' src/Graphics/Window/WaylandWindow.cpp
echo "== public/getstate references =="
rg -n "GetState|IsMaximized|SetMaximized|m_isMaximized|m_isFullScreen" src/Graphics/Window -SRepository: SpaRcle-Studio/SRRender
Length of output: 11000
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
# Read-only semantic probe for the configured-branch logic around state flags.
# This mirrors the branch conditions as text without running repository code.
def old_branch_is_maximized_changed(state_before, floating, maximized):
"""
Simulates the parsed conditions found in xdg_toplevel_configured callback:
pWindow->IsMaximized() tests m_isMaximized.
If not maximized and incoming maximized -> set true.
Else if maximized and floating -> set false.
"""
changed = False
if not state_before and maximized:
state_before = True
changed = True
elif state_before and floating:
state_before = False
changed = True
return state_before, changed
cases = [
("maximized -> tiled", True, False, False),
("tiled -> tiled", False, False, False),
("tiled -> maximized", False, False, True),
("maximized -> fullscreen-ish", True, True, False),
]
for name, before, floating, maximized in cases:
after, changed = old_branch_is_maximized_changed(before, floating, maximized)
print(f"{name}: before={before}, floating={floating}, maximized={maximized} => after={after}, changed={changed}")
PYRepository: SpaRcle-Studio/SRRender
Length of output: 542
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '500,550p' src/Graphics/Window/WaylandWindow.cpp
sed -n '170,185p' inc/Graphics/Window/Window.h
sed -n '170,185p' src/Graphics/Window/Window.cpp
sed -n '1,120p' inc/Graphics/Window/WaylandWindow.hRepository: SpaRcle-Studio/SRRender
Length of output: 8029
Clear m_isMaximized when the configure omits XDG_TOPLEVEL_STATE_MAXIMIZED.
A tiled configure sets floating = 0; this does not clear m_isMaximized after a previous maximize. As a result, GetState() returns WindowState::Maximized and Window::IsMaximized() reports stale state after xorg_mode maximize-to-tile transitions. Set m_isMaximized directly from the configured maximized value for every configure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Graphics/Window/WaylandWindow.cpp` around lines 1068 - 1079, Update the
Wayland configure handling to assign m_isMaximized directly from the current
configured maximized value on every configure, including when
XDG_TOPLEVEL_STATE_MAXIMIZED is omitted. Locate the configure state update
feeding WaylandWindow::GetState(), and preserve the existing maximize detection
while ensuring tiled configurations clear the flag.
Comment on lines
+1215
to
+1225
| void WaylandWindow::SetFullScreen(bool value) { | ||
| if (value && !m_isFullScreen) { | ||
| xdg_toplevel_set_fullscreen(m_xdgToplevel, nullptr); | ||
| m_isFullScreen = true; | ||
| SR_LOG("WaylandWindow::SetFullScreen() : entered fullscreen mode"); | ||
| } else if (!value && m_isFullScreen) { | ||
| xdg_toplevel_unset_fullscreen(m_xdgToplevel); | ||
| m_isFullScreen = false; | ||
| SR_LOG("WaylandWindow::SetFullScreen() : exited fullscreen mode"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant file and nearby method usages without running repo code.
fd -a 'WaylandWindow.cpp|WaylandWindow.h' . | sed 's#^\./##'
echo '--- outline ---'
ast-grep outline src/Graphics/Window/WaylandWindow.cpp --view compact || true
echo '--- focus lines 1160-1260 ---'
sed -n '1160,1260p' src/Graphics/Window/WaylandWindow.cpp | cat -n
echo '--- read state/query methods references ---'
rg -n "IsFullScreen|GetState|m_isFullScreen|xdg_toplevel_set_fullscreen|xdg_toplevel_unset_fullscreen|configure|Configure|SetFullScreen" src/Graphics/Window/WaylandWindow.cpp src/Graphics/Window -S || trueRepository: SpaRcle-Studio/SRRender
Length of output: 14740
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- WaylandWindow.cpp key sections ---'
sed -n '100,135p' src/Graphics/Window/WaylandWindow.cpp | cat -n
sed -n '450,550p' src/Graphics/Window/WaylandWindow.cpp | cat -n
sed -n '1020,1080p' src/Graphics/Window/WaylandWindow.cpp | cat -n
sed -n '1208,1230p' src/Graphics/Window/WaylandWindow.cpp | cat -n
echo '--- declarations ---'
sed -n '1,220p' src/Graphics/Window/WaylandWindow.h | cat -n
echo '--- public IsFullScreen implementations ---'
sed -n '128,190p' src/Graphics/Window/Window.cpp | cat -n
rg -n "virtual bool IsFullScreen|IsFullScreen\(\) const|IsFullscreen|getWindowState|GetState" src/Graphics/Window -S || trueRepository: SpaRcle-Studio/SRRender
Length of output: 11324
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- locate WaylandWindow.h ---'
fd -a 'WaylandWindow.h' . | sed 's#^\./##'
echo '--- WaylandWindow.h sections ---'
WAYLAND_H="$(fd 'WaylandWindow.h' . | head -n1)"
sed -n '1,240p' "$WAYLAND_H" | cat -n
echo '--- SetFullscreenState/SetFullScreen symbols ---'
rg -n "SetFullscreenState|SetFullScreen|IsFullScreen\(|m_isFullScreen" . -S || trueRepository: SpaRcle-Studio/SRRender
Length of output: 15941
Keep fullscreen state synchronized with compositor configure.
SetFullScreen() currently changes m_isFullScreen before xdg_toplevel_handle_configure() receives the XDG fullscreen state entry. GetState() reads m_isFullScreen, so the window can report WindowState::FullScreen while the compositor has not committed the actual fullscreen state. Update m_isFullScreen only in configure processing, and keep the requested value deduplication if needed with a separate pending flag.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Graphics/Window/WaylandWindow.cpp` around lines 1215 - 1225, Update
WaylandWindow::SetFullScreen so it only sends fullscreen requests and no longer
changes m_isFullScreen directly; track requested-state deduplication separately
if needed. In xdg_toplevel_handle_configure(), update m_isFullScreen from the
compositor-provided XDG fullscreen state, ensuring GetState() reflects committed
configure state rather than the pending request.
Comment on lines
+151
to
+173
| void Window::SetFullScreen(bool value) { | ||
| if (!m_windowImpl) { | ||
| SR_ERROR("Window::SetFullScreen() : window implementation is nullptr."); | ||
| return; | ||
| } | ||
| m_windowImpl->SetFullScreen(value); | ||
| } | ||
|
|
||
| void Window::Maximize() { | ||
| if (!m_windowImpl) { | ||
| SR_ERROR("Window::Maximize() : window implementation is nullptr."); | ||
| return; | ||
| } | ||
| m_windowImpl->Maximize(); | ||
| } | ||
|
|
||
| void Window::Restore() { | ||
| if (!m_windowImpl) { | ||
| SR_ERROR("Window::Restore() : window implementation is nullptr."); | ||
| return; | ||
| } | ||
| m_windowImpl->Restore(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg 'Window\.cpp$|Window\.hpp$|Window\.h$|Wayland|window_impl|WindowImpl|Impl' | sed -n '1,160p'
echo "== outline Window.cpp =="
ast-grep outline src/Graphics/Window/Window.cpp --view expanded 2>/dev/null || true
echo "== relevant Window.cpp lines =="
cat -n src/Graphics/Window/Window.cpp | sed -n '1,240p'
echo "== search IsValid / Initialize / Open across Window =="
rg -n "IsValid|Is[Aa]vail|Initialize|Open\(|SetFullScreen|Maximize\(|Restore\(|m_windowImpl|m_xdgToplevel|xdg_toplevel|SetFullscreen|Fullscreen|FullScreen" src/Graphics src 2>/dev/null | sed -n '1,240p'Repository: SpaRcle-Studio/SRRender
Length of output: 33778
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== BasicWindowImpl header =="
cat -n inc/Graphics/Window/BasicWindowImpl.h | sed -n '1,180p'
echo "== BasicWindowImpl implementation =="
cat -n src/Graphics/Window/BasicWindowImpl.cpp | sed -n '1,120p'
echo "== WaylandWindow header =="
cat -n inc/Graphics/Window/WaylandWindow.h | sed -n '1,220p'
echo "== WaylandWindow relevant implementation sections =="
cat -n src/Graphics/Window/WaylandWindow.cpp | sed -n '380,480p'
cat -n src/Graphics/Window/WaylandWindow.cpp | sed -n '620,770p'
cat -n src/Graphics/Window/WaylandWindow.cpp | sed -n '770,920p'
echo "== static verifier for lifecycle guard in public methods =="
python3 - <<'PY'
from pathlib import Path
import re
cpp = Path("src/Graphics/Window/Window.cpp").read_text()
h = Path("inc/Graphics/Window/BasicWindowImpl.h").read_text()
init_pat = re.compile(r'bool\s+Window::Initialize\s*\([^)]*\)\s*\{(?P<body>.*?)\n\s*\}', re.S)
open_pat = re.compile(r'bool\s+Window::Open\s*\([^)]*\)\s*\{(?P<body>.*?)\n\s*\}', re.S)
set_full = re.compile(r'void\s+Window::SetFullScreen\s*\([^)]*\)\s*\{(?P<body>.*?)\n\s*\}', re.S)
max_pat = re.compile(r'void\s+Window::Maximize\s*\([^)]*\)\s*\{(?P<body>.*?)\n\s*\}', re.S)
restore_pat = re.compile(r 'void\s+Window::Restore\s*\([^)]*\)\s*\{(?P<body>.*?)\n\s*\}', re.S)
is_valid_decl = 'bool IsValid() const;' in h
is_valid_def = 'bool BasicWindowImpl::IsValid()' in Path("src/Graphics/Window/BasicWindowImpl.cpp").read_text()
init = init_pat.search(cpp)
open_ = open_pat.search(cpp)
sets = [pat.search(cpp) for pat in [set_full, max_pat, restore_pat]]
print({"init_uses_mName_mInitialSize_only": bool(init) and not bool(re.search(r'm_windowImpl->Initialize|m_windowImpl->Create|Mwindow|m_xdgToplevel', init.group("body"), re.S))})
print({"open_uses_m_windowImpl_Initialize": bool(open_) and bool(re.search(r'm_windowImpl->Initialize\s*\(', open_.group("body"), re.S))})
print({"initial_state_has_implementation": bool(init) and bool(re.search(r'm_windowImpl\s*=\s*BasicWindowImpl::CreatePlatformWindow', init.group("body"), re.S))})
print({"initial_state_is_uninitialized_before_open": bool(init) and bool(open_) and bool(re.search(r'm_object = BasicWindowObject::Create<.*WindowObject', init.group("body"), re.S))})
for name, pat in [("SetFullScreen", set_full), ("Maximize", max_pat), ("Restore", restore_pat)]:
mb = pat.search(cpp).group("body") if pat.search(cpp) else ""
print({name: {"has_implementation_guard": bool(re.search(r'if\s*\(\s*!\s*m_windowImpl\s*\)', mb)), "has_valid_guard": bool(re.search(r'IsValid\s*\(\s*\)', mb)), "delegates": bool(re.search(r'm_windowImpl\s*->', mb))}})
print({"IsValid_declared_in_public_interface": is_valid_decl})
print({"IsValid_impl_present_in_BasicWindowImpl": is_valid_def})
PYRepository: SpaRcle-Studio/SRRender
Length of output: 42553
Reject window-state commands before the platform window is initialized.
Window::Initialize() creates m_windowImpl, but the Wayland platform window is not initialized until Window::Open() calls m_windowImpl->Initialize(...). SetFullScreen(true), Maximize(), and Restore() can reach WaylandWindow before m_xdgToplevel exists, so add a valid implementation guard in SetFullScreen(), Maximize(), and Restore().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Graphics/Window/Window.cpp` around lines 151 - 173, Update
Window::SetFullScreen(), Window::Maximize(), and Window::Restore() to reject
commands until the platform implementation is fully initialized, not merely when
m_windowImpl is non-null. Add the appropriate validity guard for the underlying
window state before invoking the corresponding m_windowImpl methods, while
preserving the existing error-and-return behavior.
Member
|
Как ты это тестировал? Лично у меня кнопка При ресайзах движок также должен проверять эти стейты. |
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.
Summary by CodeRabbit
New Features
Bug Fixes