diff --git a/src/app/break-window.cpp b/src/app/break-window.cpp index dad6ea2..4ed0afc 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 { @@ -203,16 +213,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 +270,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 +293,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 +309,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 c9615ed..590d4c7 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 b7626c7..db45d81 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,19 @@ 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(); 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 +200,8 @@ void BreakWindows::destroy() { w->deleteLater(); } m_windows.clear(); + m_activeBreak.reset(); + m_screenDebounce->stop(); clockUpdateTimer->stop(); } @@ -180,6 +223,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 +240,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 +288,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 1dc14b2..1a4e2e3 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 4140d95..c4c53b8 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 847dd31..910838d 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;