From d9ad6935a46ee0594f025678194fca4778c1ffbc Mon Sep 17 00:00:00 2001 From: Allan Chain <36528777+AllanChain@users.noreply.github.com> Date: Sat, 18 Jul 2026 20:21:44 +0800 Subject: [PATCH 1/2] add/remove break windows on display change --- src/app/break-window.cpp | 32 ++++-- src/app/break-window.h | 6 ++ src/app/break-windows.cpp | 203 ++++++++++++++++++++++++++++-------- src/app/break-windows.h | 46 +++++++- src/app/heads-up-window.cpp | 3 + src/app/heads-up-window.h | 3 + 6 files changed, 242 insertions(+), 51 deletions(-) diff --git a/src/app/break-window.cpp b/src/app/break-window.cpp index dad6ea23..2370b801 100644 --- a/src/app/break-window.cpp +++ b/src/app/break-window.cpp @@ -203,16 +203,24 @@ void BreakWindow::showFullScreen() { setProperty("color", m_data.theme.mainBackground); ui->breakLabel->setText(m_data.message.fullScreen); - if (m_data.isPostponed) ui->postponeLabel->setVisible(true); - if (m_data.show.countdown) ui->countDownLabel->setVisible(true); + // Set visibility explicitly from the preferences rather than only showing when the + // flag is true. showFlashPrompt() normally hides these first, but a window created + // directly into the full-screen phase (e.g. on display hot-plug) skips that step, so + // without these explicit hides every widget would stay visible regardless of the + // small/bigBreakShow* preferences. The progress bar had the symmetric problem: it + // was never managed here at all, so it stayed at the .ui default (visible) and + // showed on hot-plugged windows even when show.progressBar was false. + ui->progressBar->setVisible(m_data.show.prograssBar); + ui->postponeLabel->setVisible(m_data.isPostponed); + ui->countDownLabel->setVisible(m_data.show.countdown); + ui->clock->setVisible(m_data.show.clock); if (m_data.show.clock) { - ui->clock->setVisible(true); ui->clock->setStyleSheet( QString("QLabel { background: transparent; font-size: %1px; }") .arg(m_data.show.countdown ? 50 : 100)); } - if (m_data.show.endTime) ui->breakEndTimeLabel->setVisible(true); - if (m_data.show.buttons) ui->buttons->setVisible(true); + ui->breakEndTimeLabel->setVisible(m_data.show.endTime); + ui->buttons->setVisible(m_data.show.buttons); QPropertyAnimation* resizeAnim = new QPropertyAnimation(m_waylandWorkaround ? mainWidget : this, "geometry"); @@ -252,9 +260,10 @@ void BreakWindow::showFlashPrompt() { } ui->breakLabel->setText(m_data.message.prompt); - if (!m_data.show.prograssBar) { - ui->progressBar->setVisible(false); - } + // Set visibility explicitly (idempotent) so a window that skips the + // prompt->fullscreen animation, or re-enters prompt from fullscreen, has the right + // state. + ui->progressBar->setVisible(m_data.show.prograssBar); ui->postponeLabel->setVisible(false); ui->countDownLabel->setVisible(false); ui->clock->setVisible(false); @@ -274,6 +283,7 @@ void BreakWindow::showFlashPrompt() { } void BreakWindow::initSize(QScreen* screen) { + m_screen = screen; if (m_waylandWorkaround) { QRect rect = screen->availableGeometry(); // Avoid using full height when initializing the main window. GNOME will refuse to @@ -289,6 +299,12 @@ void BreakWindow::initSize(QScreen* screen) { SMALL_WINDOW_WIDTH, SMALL_WINDOW_HEIGHT); } createWinId(); + // Pin the surface to the requested output. On Wayland + layer-shell this makes + // LayerShellQt place the surface on the right output (it reads QWindow::screen() + // by default); on other platforms it keeps screen() authoritative after creation. + // Must come after createWinId() so windowHandle() is valid, and before show() so + // the surface is created on the right output from the start. + if (windowHandle() && screen) windowHandle()->setScreen(screen); #ifdef Q_OS_MACOS macSetAllWorkspaces(windowHandle()); #endif diff --git a/src/app/break-window.h b/src/app/break-window.h index c9615ed9..590d4c7e 100644 --- a/src/app/break-window.h +++ b/src/app/break-window.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -42,6 +43,10 @@ class BreakWindow : public QMainWindow { void showButtons(AbstractBreakWindows::Buttons buttons, bool show = true); void initSize(QScreen* screen); + // Raw screen identity captured in initSize. QWidget::screen() is unreliable after + // a screen is removed (Qt migrates windows to the primary screen), so we match by + // this pointer instead. Never dereference a screen that has been removed. + QScreen* screenIdentity() const { return m_screen; } QColor color() const; void setColor(const QColor& color); @@ -61,6 +66,7 @@ class BreakWindow : public QMainWindow { bool m_waylandWorkaround = false; bool m_supportTransparentInput = true; int m_totalSeconds; + QScreen* m_screen = nullptr; static void colorizeButton(QPushButton* button, QColor color); }; diff --git a/src/app/break-windows.cpp b/src/app/break-windows.cpp index b7626c7e..3532c548 100644 --- a/src/app/break-windows.cpp +++ b/src/app/break-windows.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -26,6 +27,7 @@ #include "core/break-windows.h" #include "core/flags.h" #include "core/preferences.h" +#include "heads-up-window.h" #include "idle/factory.h" #ifdef Q_OS_MACOS @@ -39,11 +41,57 @@ #include "lib/linux/system-check.h" #endif +namespace { +// BreakWindow and HeadsUpWindow both expose screenIdentity(), close(), and +// deleteLater(), so the collect-then-mutate removal and the "has window on screen" +// lookup are identical across the two types. Templatizing avoids four copy-pasted +// bodies. +template +void removeWindowsIf(QList& windows, Pred pred) { + QList toDestroy; + for (T* w : std::as_const(windows)) + if (pred(w)) toDestroy << w; + for (T* w : std::as_const(toDestroy)) { + windows.removeOne(w); + w->close(); + w->deleteLater(); + } +} +template +bool hasWindowOn(const QList& windows, QScreen* screen) { + for (T* w : windows) + if (w->screenIdentity() == screen) return true; + return false; +} +} // namespace + BreakWindows::BreakWindows(QObject* parent) : AbstractBreakWindows(parent) { soundPlayer = new SoundPlayer(this); clockUpdateTimer = new QTimer(this); connect(clockUpdateTimer, &QTimer::timeout, this, &BreakWindows::updateClocks); + // Reconcile break/heads-up windows with the current set of screens when displays + // are hot-plugged mid-break. Removal is handled immediately (synchronous, pointer + // comparison only — safe before the QScreen is destroyed); addition is debounced so + // we don't read transient/invalid geometry during macOS mid-EDID negotiation or + // non-transactional multi-screen bursts. + m_screenDebounce = new QTimer(this); + m_screenDebounce->setSingleShot(true); + m_screenDebounce->setInterval(400); + connect(m_screenDebounce, &QTimer::timeout, this, [this] { reconcileScreens(); }); + connect(qApp, &QGuiApplication::screenRemoved, this, [this](QScreen* screen) { + // Immediate (synchronous) removal — safe before QScreen destruction because we + // only compare pointers, never dereference the dying screen. + removeWindowsIf(m_windows, + [screen](auto* w) { return w->screenIdentity() == screen; }); + removeWindowsIf(m_headsUpWindows, + [screen](auto* w) { return w->screenIdentity() == screen; }); + }); + connect(qApp, &QGuiApplication::screenAdded, this, [this](QScreen*) { + // Nothing to reconcile when no break or heads-up windows are active. + if (m_activeBreak || m_activeHeadsUp) m_screenDebounce->start(); + }); + #ifdef Q_OS_LINUX if (QGuiApplication::platformName() == "wayland" && LinuxSystemSupport::layerShell) { QPluginLoader loader("libsanebreak_layer_shell"); @@ -125,26 +173,13 @@ BreakWindowData BreakWindows::createData(BreakType type, SanePreferences* prefer } } void BreakWindows::create(BreakWindowData data) { + m_activeBreak.emplace(); + m_activeBreak->data = data; + // A pending debounce from a previous break is harmless (reconcile finds every + // screen covered) but stopping it avoids a redundant pass. + m_screenDebounce->stop(); QList screens = QApplication::screens(); - for (QScreen* screen : std::as_const(screens)) { - BreakWindow* w = new BreakWindow(data); - m_windows.append(w); - w->initSize(screen); -#ifdef Q_OS_LINUX - if (layerShell) layerShell->layout(w->windowHandle()); -#endif - // GNOME mutter will make the window black if show full screen - // See https://gitlab.gnome.org/GNOME/mutter/-/issues/2520 - // GNOME mutter will also refuse to make a window always on top if maximized. - // Therefore, we use the same `show()` with and without Wayland workaround. - w->show(); - } - for (auto w : std::as_const(m_windows)) { - connect(w, &BreakWindow::lockScreenRequested, this, - &BreakWindows::lockScreenRequested); - connect(w, &BreakWindow::exitForceBreakRequested, this, - &BreakWindows::exitForceBreakRequested); - } + for (QScreen* screen : std::as_const(screens)) createOnScreen(screen); updateClocks(); // Set the initial clock clockUpdateTimer->start(3000); } @@ -159,6 +194,8 @@ void BreakWindows::destroy() { w->deleteLater(); } m_windows.clear(); + m_activeBreak.reset(); + m_screenDebounce->stop(); clockUpdateTimer->stop(); } @@ -180,6 +217,11 @@ void BreakWindows::setTime(int remainingTime) { estimatedEndTime = estimatedEndTime.addMSecs(500); } QString endTime = QLocale::system().toString(estimatedEndTime, QLocale::ShortFormat); + // Remember so windows added mid-break (hot-plug) start already time-synced. + if (m_activeBreak) { + m_activeBreak->remainingTime = remainingTime; + m_activeBreak->endTime = endTime; + } for (auto w : std::as_const(m_windows)) { w->setTime(remainingTime, endTime); } @@ -192,43 +234,44 @@ void BreakWindows::updateClocks() { } } void BreakWindows::showFullScreen() { + if (m_activeBreak) m_activeBreak->phase = Phase::FullScreen; for (auto w : std::as_const(m_windows)) w->showFullScreen(); } void BreakWindows::showFlashPrompt() { + if (m_activeBreak) m_activeBreak->phase = Phase::Prompt; for (auto w : std::as_const(m_windows)) w->showFlashPrompt(); } void BreakWindows::showButtons(Buttons buttons, bool show) { + if (m_activeBreak) { + m_activeBreak->buttons = buttons; + m_activeBreak->buttonsVisible = show; + } for (auto w : std::as_const(m_windows)) w->showButtons(buttons, show); } void BreakWindows::showHeadsUp(int totalSeconds, BreakType breakType, SanePreferences* preferences) { - if (!m_headsUpWindows.isEmpty() && - (m_headsUpTotalSeconds != totalSeconds || m_headsUpBreakType != breakType)) { + if (!m_headsUpWindows.isEmpty() && m_activeHeadsUp && + (m_activeHeadsUp->totalSeconds != totalSeconds || + m_activeHeadsUp->breakType != breakType)) { hideHeadsUp(); } if (!m_headsUpWindows.isEmpty()) return; - m_headsUpTotalSeconds = totalSeconds; - m_headsUpBreakType = breakType; + m_activeHeadsUp.emplace(ActiveHeadsUp{ + .totalSeconds = totalSeconds, + .breakType = breakType, + .remaining = totalSeconds, + .bgColor = preferences->backgroundColor->get(), + .highlightColor = breakType == BreakType::Small + ? preferences->smallHighlightColor->get() + : preferences->bigHighlightColor->get(), + .textColor = preferences->messageColor->get(), + }); + m_screenDebounce->stop(); QList screens = QApplication::screens(); - for (QScreen* screen : std::as_const(screens)) { - auto* w = new HeadsUpWindow(totalSeconds, preferences->backgroundColor->get(), - breakType == BreakType::Small - ? preferences->smallHighlightColor->get() - : preferences->bigHighlightColor->get(), - preferences->messageColor->get()); - m_headsUpWindows.append(w); - w->initSize(screen); -#ifdef Q_OS_LINUX - if (layerShell) layerShell->layout(w->windowHandle(), QMargins(0, 16, 0, 0)); -#endif - w->show(); -#ifdef Q_OS_MACOS - macSetAllWorkspaces(w->windowHandle()); -#endif - connect(w, &HeadsUpWindow::clicked, this, &BreakWindows::startBreakRequested); - } + for (QScreen* screen : std::as_const(screens)) createHeadsUpOnScreen(screen); } void BreakWindows::setHeadsUpTime(int remainingTime) { + if (m_activeHeadsUp) m_activeHeadsUp->remaining = remainingTime; for (auto* w : std::as_const(m_headsUpWindows)) { w->setTime(remainingTime); } @@ -239,5 +282,83 @@ void BreakWindows::hideHeadsUp() { w->deleteLater(); } m_headsUpWindows.clear(); - m_headsUpTotalSeconds = 0; + m_activeHeadsUp.reset(); +} + +bool BreakWindows::isValidScreen(QScreen* screen) { + return screen && !screen->geometry().isEmpty() && + !screen->availableGeometry().isEmpty(); +} + +void BreakWindows::createOnScreen(QScreen* screen) { + if (!isValidScreen(screen)) return; + BreakWindow* w = new BreakWindow(m_activeBreak->data); + m_windows.append(w); + w->initSize(screen); // records screen identity + pins QWindow::screen() +#ifdef Q_OS_LINUX + if (layerShell) layerShell->layout(w->windowHandle()); +#endif + // GNOME mutter will make the window black if show full screen + // See https://gitlab.gnome.org/GNOME/mutter/-/issues/2520 + // GNOME mutter will also refuse to make a window always on top if maximized. + // Therefore, we use the same `show()` with and without Wayland workaround. + w->show(); + // Create the window directly into the current phase so a hot-plugged display + // doesn't pop in as a small prompt and then animate to fullscreen. + if (m_activeBreak->phase == Phase::FullScreen) + w->showFullScreen(); + else + w->showFlashPrompt(); + w->showButtons(m_activeBreak->buttons, m_activeBreak->buttonsVisible); + w->setTime(m_activeBreak->remainingTime, m_activeBreak->endTime); + connect(w, &BreakWindow::lockScreenRequested, this, + &BreakWindows::lockScreenRequested); + connect(w, &BreakWindow::exitForceBreakRequested, this, + &BreakWindows::exitForceBreakRequested); + connect(w, &QObject::destroyed, this, [this](QObject* obj) { + // destroyed fires from ~QObject; only use the pointer for identity (removeOne), + // never dereference it as a BreakWindow. + m_windows.removeOne(static_cast(obj)); + }); +} + +void BreakWindows::createHeadsUpOnScreen(QScreen* screen) { + if (!isValidScreen(screen)) return; + auto* w = + new HeadsUpWindow(m_activeHeadsUp->totalSeconds, m_activeHeadsUp->bgColor, + m_activeHeadsUp->highlightColor, m_activeHeadsUp->textColor); + m_headsUpWindows.append(w); + w->initSize(screen); +#ifdef Q_OS_LINUX + if (layerShell) layerShell->layout(w->windowHandle(), QMargins(0, 16, 0, 0)); +#endif + w->show(); +#ifdef Q_OS_MACOS + macSetAllWorkspaces(w->windowHandle()); +#endif + w->setTime(m_activeHeadsUp->remaining); + connect(w, &HeadsUpWindow::clicked, this, &BreakWindows::startBreakRequested); + connect(w, &QObject::destroyed, this, [this](QObject* obj) { + m_headsUpWindows.removeOne(static_cast(obj)); + }); +} + +void BreakWindows::reconcileScreens() { + // Add pass only — removal is handled immediately by the screenRemoved handler + // and per-window by the destroyed signal, so there is nothing to catch here. + QList current = QGuiApplication::screens(); + if (m_activeBreak) { + for (QScreen* screen : current) { + if (!isValidScreen(screen)) continue; + if (hasWindowOn(m_windows, screen)) continue; + createOnScreen(screen); + } + } + if (m_activeHeadsUp) { + for (QScreen* screen : current) { + if (!isValidScreen(screen)) continue; + if (hasWindowOn(m_headsUpWindows, screen)) continue; + createHeadsUpOnScreen(screen); + } + } } diff --git a/src/app/break-windows.h b/src/app/break-windows.h index 1dc14b24..1a4e2e34 100644 --- a/src/app/break-windows.h +++ b/src/app/break-windows.h @@ -8,7 +8,9 @@ #include #include +#include #include +#include #include "app/break-window.h" #include "app/heads-up-window.h" @@ -45,12 +47,52 @@ class BreakWindows : public AbstractBreakWindows { void hideHeadsUp() override; private: + // Current break phase, remembered so windows created mid-break (e.g. after a + // display hot-plug) are created directly into the running phase instead of + // popping in as a prompt and animating to fullscreen. + enum class Phase { Prompt, FullScreen }; + + // Runtime state of the active break, remembered so windows created mid-break + // (e.g. after a display hot-plug) can be created directly into the current + // phase/time without resetting the countdown or replaying the enter sound. + // Engaged while a break is active; disengaged by destroy(). + struct ActiveBreak { + BreakWindowData data; + Phase phase = Phase::Prompt; + int remainingTime = 0; + QString endTime; + AbstractBreakWindows::Buttons buttons{}; + bool buttonsVisible = false; + }; + std::optional m_activeBreak; + + // HeadsUpWindow constructor arguments + remaining time, remembered so + // hot-plugged screens get a correctly-stated heads-up pill. + struct ActiveHeadsUp { + int totalSeconds = 0; + BreakType breakType = BreakType::Small; + int remaining = 0; + QColor bgColor; + QColor highlightColor; + QColor textColor; + }; + std::optional m_activeHeadsUp; + + void createOnScreen(QScreen* screen); + void createHeadsUpOnScreen(QScreen* screen); + // Creates windows on newly-attached screens for the active break/heads-up. + // Debounced via m_screenDebounce; removal is handled elsewhere (screenRemoved + // handler + per-window destroyed signal). + void reconcileScreens(); + static bool isValidScreen(QScreen* screen); + QList m_windows; QList m_headsUpWindows; - int m_headsUpTotalSeconds = 0; - BreakType m_headsUpBreakType = BreakType::Small; SoundPlayer* soundPlayer; QTimer* clockUpdateTimer; + // Debounces screenAdded so we don't read transient/invalid geometry during macOS + // mid-EDID negotiation or non-transactional multi-screen bursts. + QTimer* m_screenDebounce = nullptr; void updateClocks(); #ifdef Q_OS_LINUX diff --git a/src/app/heads-up-window.cpp b/src/app/heads-up-window.cpp index 4140d958..c4c53b82 100644 --- a/src/app/heads-up-window.cpp +++ b/src/app/heads-up-window.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include HeadsUpWindow::HeadsUpWindow(int totalSeconds, QColor bgColor, QColor highlightColor, @@ -69,9 +70,11 @@ void HeadsUpWindow::setTime(int remainingSeconds) { } void HeadsUpWindow::initSize(QScreen* screen) { + m_screen = screen; QRect geo = screen->availableGeometry(); move(geo.x() + (geo.width() - PILL_WIDTH) / 2, geo.y() + 16); createWinId(); + if (windowHandle() && screen) windowHandle()->setScreen(screen); } void HeadsUpWindow::paintEvent(QPaintEvent*) { diff --git a/src/app/heads-up-window.h b/src/app/heads-up-window.h index 847dd31d..910838da 100644 --- a/src/app/heads-up-window.h +++ b/src/app/heads-up-window.h @@ -20,6 +20,8 @@ class HeadsUpWindow : public QWidget { QColor textColor, QWidget* parent = nullptr); void initSize(QScreen* screen); void setTime(int remainingSeconds); + // Raw screen identity captured in initSize; see BreakWindow::screenIdentity(). + QScreen* screenIdentity() const { return m_screen; } signals: void clicked(); @@ -38,6 +40,7 @@ class HeadsUpWindow : public QWidget { QColor m_textColor; QPropertyAnimation* m_progressAnim; QAbstractAnimation* m_flashAnim; + QScreen* m_screen = nullptr; static const int PILL_WIDTH = 280; static const int PILL_HEIGHT = 56; From 1971f07f6069efe23ab0b09a05a25be544769e11 Mon Sep 17 00:00:00 2001 From: Allan Chain <36528777+AllanChain@users.noreply.github.com> Date: Sun, 19 Jul 2026 11:31:33 +0800 Subject: [PATCH 2/2] fix progress bar not showing on initial display The progress bar was driven by a long-running QPropertyAnimation started in the constructor, which was then scrubbed via setCurrentTime() in setTime(). This design was fragile: setCurrentTime() does not update the animated property when the animation is Stopped, and the animation stops when it reaches its end. During initial break window creation, remainingTime defaulted to 0, so setTime(0, ...) pushed the animation to its end, permanently freezing the progress bar at 0 (invisible with transparent background). Hot-plugged displays worked only because setTime() was called with a real remaining time while the animation was still running. Redesign setTime() as the single source of truth: each call starts a short 1-second interpolating animation from the bar's current value to the new target, giving smooth sub-second motion without long-running state that can desync. Also initialize remainingTime to the full break duration in create() so freshly created windows don't get setTime(0) before the real time is set. --- src/app/break-window.cpp | 26 ++++++++++++++++++-------- src/app/break-windows.cpp | 6 ++++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/app/break-window.cpp b/src/app/break-window.cpp index 2370b801..4ed0afc4 100644 --- a/src/app/break-window.cpp +++ b/src/app/break-window.cpp @@ -104,13 +104,8 @@ BreakWindow::BreakWindow(BreakWindowData data, QWidget* parent) ui->postponeLabel->setVisible(false); - m_progressAnim = new QPropertyAnimation(ui->progressBar, "value"); - m_progressAnim->setStartValue(ui->progressBar->maximum()); - m_progressAnim->setEndValue(0); - - this->m_totalSeconds = data.totalSeconds; - m_progressAnim->setDuration(m_totalSeconds * 1000); - m_progressAnim->start(); + m_progressAnim = new QPropertyAnimation(ui->progressBar, "value", this); + m_totalSeconds = data.totalSeconds; int baseDuration = data.theme.flashAnimationDuration; if (baseDuration <= 0) { @@ -167,7 +162,22 @@ void BreakWindow::setColor(const QColor& color) { } void BreakWindow::setTime(int remainingTime, QString estimatedEndTime) { - m_progressAnim->setCurrentTime((m_totalSeconds - remainingTime) * 1000); + // Drive the progress bar from the authoritative remaining time. Each call + // starts a short animation from the bar's current value to the new target, + // giving smooth sub-second interpolation without a long-running animation + // whose state can desync from the real countdown (e.g. when the animation + // reaches its end and stops, after which setCurrentTime no longer updates + // the property). + int targetValue = + m_totalSeconds > 0 + ? static_cast(ui->progressBar->maximum() * + static_cast(remainingTime) / m_totalSeconds) + : 0; + m_progressAnim->stop(); + m_progressAnim->setStartValue(ui->progressBar->value()); + m_progressAnim->setEndValue(targetValue); + m_progressAnim->setDuration(1000); + m_progressAnim->start(); if (m_totalSeconds <= 60) { ui->countDownLabel->setText(QString("%1").arg(remainingTime)); } else { diff --git a/src/app/break-windows.cpp b/src/app/break-windows.cpp index 3532c548..db45d812 100644 --- a/src/app/break-windows.cpp +++ b/src/app/break-windows.cpp @@ -175,6 +175,12 @@ BreakWindowData BreakWindows::createData(BreakType type, SanePreferences* prefer void BreakWindows::create(BreakWindowData data) { m_activeBreak.emplace(); m_activeBreak->data = data; + // Initialize remainingTime to the full duration: a freshly created break has + // not started counting down yet. Without this, remainingTime defaults to 0 + // (the struct default), so createOnScreen() would call setTime(0, ...) before + // AppStateBreak::enter() sets the real time, causing a brief visual glitch + // where the progress bar animates to empty and then back to full. + m_activeBreak->remainingTime = data.totalSeconds; // A pending debounce from a previous break is harmless (reconcile finds every // screen covered) but stopping it avoids a redundant pass. m_screenDebounce->stop();