Skip to content

compositor: cleanup fading out on expired monitor #13179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gulafaran wants to merge 1 commit into
base: main
Choose a base branch
from
+17 −3
Open

compositor: cleanup fading out on expired monitor #13179

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/Compositor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1288,10 +1288,18 @@ void CCompositor::changeWindowZOrder(PHLWINDOW pWindow, bool top) {
}

void CCompositor::cleanupFadingOut(const MONITORID& monid) {
bool windowsFadingDirty = false;

for (auto const& ww : m_windowsFadingOut) {

auto w = ww.lock();

if (!w || !w->m_monitor) {
windowsFadingDirty = true;
Log::logger->log(Log::DEBUG, "Cleanup: m_windowsFadingOut had expired window or monitor");
continue;
}

if (w->monitorID() != monid && w->m_monitor)
continue;

Expand All @@ -1311,14 +1319,18 @@ void CCompositor::cleanupFadingOut(const MONITORID& monid) {
}
}

if (windowsFadingDirty)
std::erase_if(m_windowsFadingOut, [](const auto& el) { return el.expired() || !el->m_monitor; });
Copy link
Member

@vaxerski vaxerski Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, do not remove a window just because it has no monitor. It could still be alive but lost for some reason. If it's destroyed AND no monitor, sure.

Copy link
Contributor Author

@gulafaran gulafaran Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

problem doing && here would mean the el might be expired but monitor might not

Copy link
Member

@vaxerski vaxerski Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be yes, but in general you cant remove a window until it's marked as ready for destrtoy

Copy link
Contributor Author

@gulafaran gulafaran Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well yeah but some of these containers get stuck with with some stuff when i hotplug the monitor. and && might segfault, el.expired && el->m_monitor will nullptr deref. but i might be going about this the wrong way then, perhaps m_layers or m_windows contains something that it shouldnt so it never expires on hotplugging the monitor hm

Copy link
Member

@vaxerski vaxerski Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdym by stuck, whats their state


bool layersDirty = false;

for (auto const& lsr : m_surfacesFadingOut) {

auto ls = lsr.lock();

if (!ls) {
if (!ls || !ls->m_monitor) {
Copy link
Member

@vaxerski vaxerski Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

layersDirty = true;
Log::logger->log(Log::DEBUG, "Cleanup: m_surfacesFadingOut had expired layer or monitor");
continue;
}

Expand Down Expand Up @@ -1349,8 +1361,10 @@ void CCompositor::cleanupFadingOut(const MONITORID& monid) {
}
}

if (layersDirty)
std::erase_if(m_surfacesFadingOut, [](const auto& el) { return el.expired(); });
if (layersDirty) {
std::erase_if(m_surfacesFadingOut, [](const auto& el) { return el.expired() || !el->m_monitor; });
std::erase_if(m_layers, [](const auto& el) { return !el->m_monitor; });
}
}

void CCompositor::addToFadingOutSafe(PHLLS pLS) {
Expand Down
Loading