diff --git a/bin/omarchy-restart-shell b/bin/omarchy-restart-shell index 8d3d6ce9..0794d6b0 100755 --- a/bin/omarchy-restart-shell +++ b/bin/omarchy-restart-shell @@ -77,15 +77,32 @@ if notifications_ready; then fi # Each kill stops the oldest matching instance and only returns once it has -# fully exited, so the no-duplicate launch below can't race a dying shell. -while timeout 5 quickshell kill -p "$CONFIG_DIR" --any-display >/dev/null 2>&1; do :; done +# fully exited. A large bar can take longer than a few seconds to tear down, +# so the client gets time to see that through, and the wait below covers a +# client that timed out anyway: the fresh shell refuses to start next to a +# still-dying instance (-n), which is how a restart used to end with no bar. +while timeout 30 quickshell kill -p "$CONFIG_DIR" --any-display >/dev/null 2>&1; do :; done + +shell_instance_running() { + quickshell list -a -j 2>/dev/null | + jq -e --arg dir "$CONFIG_DIR/" 'any(.[]; .config_path | startswith($dir))' >/dev/null 2>&1 +} + +deadline=$((SECONDS + 30)) +while (( SECONDS < deadline )) && shell_instance_running; do + sleep 0.1 +done # Spawn from Hyprland so the shell inherits the canonical session environment, # not transient variables from a terminal, SSH connection, or development tool. hyprctl dispatch 'hl.dsp.exec_cmd("omarchy-launch-shell")' >/dev/null shell_ready=0 -for (( attempt = 0; attempt < 20; attempt++ )); do +# The shell answers once its plugins have loaded, which on a large bar is +# well past ten seconds, so wait on a deadline rather than a fixed handful +# of attempts. +deadline=$((SECONDS + 60)) +while (( SECONDS < deadline )); do if OMARCHY_PATH="$session_omarchy_path" OMARCHY_SHELL_IPC_TIMEOUT=0.5s omarchy-shell shell ping >/dev/null 2>&1; then shell_ready=1 break diff --git a/shell/plugins/bar/Bar.qml b/shell/plugins/bar/Bar.qml index e5baec22..4d5771ab 100644 --- a/shell/plugins/bar/Bar.qml +++ b/shell/plugins/bar/Bar.qml @@ -110,7 +110,11 @@ Item { property var clickTargets: [] property var moduleSlots: [] property var pluginBarApis: ({}) - property var pluginObjectOwners: [] + // target -> { target, pluginId, clickTarget, popout }. Keyed by object so a + // registration edits one entry in place instead of copying and rescanning an + // array in QML; Qt's Map still finds a key by a native linear scan. + readonly property var pluginObjectOwners: new Map() + property bool pluginBarApiSyncQueued: false Component { id: pluginBarApiComponent @@ -138,20 +142,20 @@ Item { root.syncPluginBarApiObjects(api) } - function syncPluginBarApiObjects(api) { + // Each api keeps its own detached copy of the layout. A flush serialises + // the layout once and passes the string in so it is not re-serialised for + // every plugin. + function syncPluginBarApiObjects(api, layoutSnapshot) { if (!api) return api.activePopout = root.pluginOwnsBarObject(api.pluginId, root.activePopout) ? root.activePopout : (root.activePopout ? api.foreignPopoutMarker : null) api.clickTargets = root.pluginClickTargets(api.pluginId) - api.layoutConfig = root.publicLayoutConfig() + api.layoutConfig = layoutSnapshot !== undefined + ? JSON.parse(layoutSnapshot) : root.publicLayoutConfig() } function pluginObjectRecord(target) { - for (var i = 0; i < pluginObjectOwners.length; i++) { - var record = pluginObjectOwners[i] - if (record && record.target === target) return record - } - return null + return target ? (pluginObjectOwners.get(target) || null) : null } function markPluginObject(pluginId, target, role) { @@ -159,31 +163,20 @@ Item { if (!key || !target) return false var record = root.pluginObjectRecord(target) if (record && record.pluginId !== key) return false - var next = [] - for (var i = 0; i < pluginObjectOwners.length; i++) { - var existing = pluginObjectOwners[i] - if (!existing || existing.target !== target) next.push(existing) + if (!record) { + record = { target: target, pluginId: key, clickTarget: false, popout: false } + pluginObjectOwners.set(target, record) } - var updated = record || { target: target, pluginId: key, clickTarget: false, popout: false } - updated[role] = true - next.push(updated) - pluginObjectOwners = next + record[role] = true return true } function unmarkPluginObject(pluginId, target, role) { var key = String(pluginId || "") - var next = [] - for (var i = 0; i < pluginObjectOwners.length; i++) { - var record = pluginObjectOwners[i] - if (!record || record.target !== target || record.pluginId !== key) { - next.push(record) - continue - } - record[role] = false - if (record.clickTarget || record.popout) next.push(record) - } - pluginObjectOwners = next + var record = root.pluginObjectRecord(target) + if (!record || record.pluginId !== key) return + record[role] = false + if (!record.clickTarget && !record.popout) pluginObjectOwners.delete(target) } function pluginOwnsBarObject(pluginId, target) { @@ -200,8 +193,25 @@ Item { return out } + // Every click target registration, popout change and layout change used to + // resync every plugin api synchronously. Startup alone is hundreds of + // registrations, each walking every api and every target, so coalesce them + // into one resync per event-loop turn (as onModuleSlotsChanged already does + // for prunePluginBarApis). bindPluginBarApi still syncs a brand-new api + // directly so a widget never sees an empty api on its first read. + function schedulePluginBarApiSync() { + if (pluginBarApiSyncQueued) return + pluginBarApiSyncQueued = true + Qt.callLater(root.syncAllPluginBarApiObjects) + } + function syncAllPluginBarApiObjects() { - for (var id in pluginBarApis) root.syncPluginBarApiObjects(pluginBarApis[id]) + pluginBarApiSyncQueued = false + var layoutSnapshot = JSON.stringify(root.layoutConfig || {}) + for (var id in pluginBarApis) { + var api = pluginBarApis[id] + if (api) root.syncPluginBarApiObjects(api, layoutSnapshot) + } } function registerPluginClickTarget(pluginId, target) { @@ -215,15 +225,20 @@ Item { root.unmarkPluginObject(pluginId, target, "clickTarget") } + // A plugin may read api.activePopout right after asking for its popout, so + // its own api is brought up to date inline; everyone else waits for the + // coalesced resync. function requestPluginPopout(pluginId, owner) { if (!root.markPluginObject(pluginId, owner, "popout")) return root.requestPopout(owner) + root.syncPluginBarApiObjects(pluginBarApis[String(pluginId || "")]) } function releasePluginPopout(pluginId, owner) { if (!root.pluginOwnsBarObject(pluginId, owner)) return root.releasePopout(owner) root.unmarkPluginObject(pluginId, owner, "popout") + root.syncPluginBarApiObjects(pluginBarApis[String(pluginId || "")]) } function pluginBarApiFor(pluginId, moduleName, registered) { @@ -287,16 +302,14 @@ Item { } function releasePluginObjects(pluginId) { - var owned = pluginObjectOwners.slice() + var owned = Array.from(pluginObjectOwners.values()) for (var i = 0; i < owned.length; i++) { var record = owned[i] if (!record || record.pluginId !== pluginId) continue if (record.clickTarget) root.unregisterClickTarget(record.target) if (record.popout && root.activePopout === record.target) root.releasePopout(record.target) + pluginObjectOwners.delete(record.target) } - pluginObjectOwners = pluginObjectOwners.filter(function(record) { - return record && record.pluginId !== pluginId - }) } function prunePluginBarApis() { @@ -313,9 +326,9 @@ Item { pluginBarApis = next } - onActivePopoutChanged: syncAllPluginBarApiObjects() - onClickTargetsChanged: syncAllPluginBarApiObjects() - onLayoutConfigChanged: syncAllPluginBarApiObjects() + onActivePopoutChanged: schedulePluginBarApiSync() + onClickTargetsChanged: schedulePluginBarApiSync() + onLayoutConfigChanged: schedulePluginBarApiSync() onModuleSlotsChanged: Qt.callLater(prunePluginBarApis) Component.onDestruction: { @@ -1914,7 +1927,9 @@ Item { acceptedButtons: Qt.LeftButton enabled: slot.visible && slot.width > 0 && slot.height > 0 propagateComposedEvents: true - cursorShape: root.moduleClickTargetAt(slot, mouseX, mouseY) ? Qt.PointingHandCursor : Qt.ArrowCursor + // Only the hovered slot needs the hit test; without the guard every + // slot on every monitor re-ran it on each click target change. + cursorShape: moduleHover.hovered && root.moduleClickTargetAt(slot, mouseX, mouseY) ? Qt.PointingHandCursor : Qt.ArrowCursor // Do not assign drag.target here: ModuleSlot is owned by Row/Column // positioners, and mutating slot.x/slot.y can leave stale offsets that // make neighboring modules overlap after a small aborted drag. diff --git a/shell/plugins/bar/widgets/Indicators.qml b/shell/plugins/bar/widgets/Indicators.qml index 00417eed..c4fd9224 100644 --- a/shell/plugins/bar/widgets/Indicators.qml +++ b/shell/plugins/bar/widgets/Indicators.qml @@ -158,12 +158,12 @@ BarWidget { onIndicatorEntriesChanged: syncActiveIndicatorOrder() - implicitWidth: root.vertical - ? Math.max(activeVerticalBlock.implicitWidth, inactiveVerticalArea.implicitWidth) - : activeHorizontalBlock.implicitWidth + inactiveHorizontalArea.implicitWidth - implicitHeight: root.vertical - ? activeVerticalBlock.implicitHeight + inactiveVerticalArea.implicitHeight - : Math.max(activeHorizontalBlock.implicitHeight, inactiveHorizontalArea.implicitHeight) + // Only the tree for the current orientation exists (see orientationLoader + // below); its wrapper keeps the stock sizing expressions, so the root's + // implicit size still follows the blocks synchronously rather than waiting + // on a positioner's polish. + implicitWidth: orientationLoader.item ? orientationLoader.item.implicitWidth : 0 + implicitHeight: orientationLoader.item ? orientationLoader.item.implicitHeight : 0 ShellIpc { target: "omarchy.indicators" @@ -184,89 +184,117 @@ BarWidget { Component.onCompleted: root.refreshRequested() - Row { - id: horizontalIndicators + // Instantiating both orientation trees and toggling `visible` doubled every + // indicator per bar (and every process an indicator spawns, once per + // monitor). Load only the tree that matches the bar orientation instead. + Loader { + id: orientationLoader + sourceComponent: root.vertical ? verticalIndicatorsTree : horizontalIndicatorsTree + } - visible: !root.vertical - spacing: 0 - - HoverHandler { - onHoveredChanged: root.setIndicatorAreaHovered(hovered) - } + Component { + id: horizontalIndicatorsTree Item { - id: inactiveHorizontalArea - - implicitWidth: root.revealInactiveIndicators ? inactiveHorizontalBlock.implicitWidth : 0 - implicitHeight: Math.max(inactiveHorizontalBlock.implicitHeight, root.barSize) + implicitWidth: activeHorizontalBlock.implicitWidth + inactiveHorizontalArea.implicitWidth + implicitHeight: Math.max(activeHorizontalBlock.implicitHeight, inactiveHorizontalArea.implicitHeight) width: implicitWidth height: implicitHeight - clip: true - IndicatorBlock { - id: inactiveHorizontalBlock - anchors.verticalCenter: parent.verticalCenter - indicatorsModule: root - indicatorEntries: root.indicatorEntries - indicatorBlock: "inactive" - horizontal: true - reportActiveState: !root.vertical + Row { + id: horizontalIndicators + + spacing: 0 + + HoverHandler { + onHoveredChanged: root.setIndicatorAreaHovered(hovered) + } + + Item { + id: inactiveHorizontalArea + + implicitWidth: root.revealInactiveIndicators ? inactiveHorizontalBlock.implicitWidth : 0 + implicitHeight: Math.max(inactiveHorizontalBlock.implicitHeight, root.barSize) + width: implicitWidth + height: implicitHeight + clip: true + + IndicatorBlock { + id: inactiveHorizontalBlock + anchors.verticalCenter: parent.verticalCenter + indicatorsModule: root + indicatorEntries: root.indicatorEntries + indicatorBlock: "inactive" + horizontal: true + reportActiveState: !root.vertical + } + + HoverHandler { + onHoveredChanged: root.setIndicatorAreaHovered(hovered) + } + } + + ActiveIndicatorBlock { + id: activeHorizontalBlock + indicatorsModule: root + indicatorModel: activeIndicatorModel + horizontal: true + reportActiveState: !root.vertical + } } - - HoverHandler { - onHoveredChanged: root.setIndicatorAreaHovered(hovered) - } - } - - ActiveIndicatorBlock { - id: activeHorizontalBlock - indicatorsModule: root - indicatorModel: activeIndicatorModel - horizontal: true - reportActiveState: !root.vertical } } - Column { - id: verticalIndicators - - visible: root.vertical - spacing: 0 - - HoverHandler { - onHoveredChanged: root.setIndicatorAreaHovered(hovered) - } + Component { + id: verticalIndicatorsTree Item { - id: inactiveVerticalArea - - implicitWidth: Math.max(inactiveVerticalBlock.implicitWidth, root.barSize) - implicitHeight: root.revealInactiveIndicators ? inactiveVerticalBlock.implicitHeight : 0 + implicitWidth: Math.max(activeVerticalBlock.implicitWidth, inactiveVerticalArea.implicitWidth) + implicitHeight: activeVerticalBlock.implicitHeight + inactiveVerticalArea.implicitHeight width: implicitWidth height: implicitHeight - clip: true - IndicatorBlock { - id: inactiveVerticalBlock - anchors.horizontalCenter: parent.horizontalCenter - indicatorsModule: root - indicatorEntries: root.indicatorEntries - indicatorBlock: "inactive" - horizontal: false - reportActiveState: root.vertical + Column { + id: verticalIndicators + + spacing: 0 + + HoverHandler { + onHoveredChanged: root.setIndicatorAreaHovered(hovered) + } + + Item { + id: inactiveVerticalArea + + implicitWidth: Math.max(inactiveVerticalBlock.implicitWidth, root.barSize) + implicitHeight: root.revealInactiveIndicators ? inactiveVerticalBlock.implicitHeight : 0 + width: implicitWidth + height: implicitHeight + clip: true + + IndicatorBlock { + id: inactiveVerticalBlock + anchors.horizontalCenter: parent.horizontalCenter + indicatorsModule: root + indicatorEntries: root.indicatorEntries + indicatorBlock: "inactive" + horizontal: false + reportActiveState: root.vertical + } + + HoverHandler { + onHoveredChanged: root.setIndicatorAreaHovered(hovered) + } + } + + ActiveIndicatorBlock { + id: activeVerticalBlock + indicatorsModule: root + indicatorModel: activeIndicatorModel + horizontal: false + reportActiveState: root.vertical + } } - - HoverHandler { - onHoveredChanged: root.setIndicatorAreaHovered(hovered) - } - } - - ActiveIndicatorBlock { - id: activeVerticalBlock - indicatorsModule: root - indicatorModel: activeIndicatorModel - horizontal: false - reportActiveState: root.vertical } } diff --git a/test/shell.d/bar-test.sh b/test/shell.d/bar-test.sh index 0b06258a..7820fcca 100644 --- a/test/shell.d/bar-test.sh +++ b/test/shell.d/bar-test.sh @@ -30,6 +30,29 @@ if ! perl -0ne 'exit(/onPressAndHold:\s*function[^{]*\{[^}]*?\bpressed\b[^}]*?\b fi pass "bar move ignores a press-and-hold propagated from a widget above" +# Every click target registration used to resync every plugin api inline. With +# six monitors' worth of widgets that is hundreds of full walks at startup, so +# the change handlers coalesce into one deferred resync and ownership lookups +# are keyed by target instead of scanning an array. +if ! rg -q 'onClickTargetsChanged: schedulePluginBarApiSync\(\)' "$ROOT/shell/plugins/bar/Bar.qml"; then + fail "bar coalesces plugin api resyncs instead of running one per click target change" +fi +if ! rg -q 'Qt\.callLater\(root\.syncAllPluginBarApiObjects\)' "$ROOT/shell/plugins/bar/Bar.qml"; then + fail "bar defers the coalesced plugin api resync to the event loop" +fi +if ! rg -q 'pluginObjectOwners\.get\(target\)' "$ROOT/shell/plugins/bar/Bar.qml"; then + fail "bar looks plugin object ownership up by target instead of scanning" +fi +pass "bar coalesces plugin api resyncs and looks ownership up by target" + +# Both bar orientations used to be instantiated and toggled with `visible`, +# doubling every indicator (and every process an indicator spawns) per bar. +if ! rg -q 'sourceComponent: root\.vertical \? verticalIndicatorsTree : horizontalIndicatorsTree' \ + "$ROOT/shell/plugins/bar/widgets/Indicators.qml"; then + fail "indicators instantiate only the tree for the current bar orientation" +fi +pass "indicators instantiate only the tree for the current bar orientation" + run_node_test <<'JS' const fs = require('fs') const bar = requireFromRoot('shell/plugins/bar/BarModel.js')