Skip to content

Skip placeholder and FALLBACK screens when creating bars - #13477

Closed
AnPod wants to merge 1 commit into
omacom:quattrofrom
AnPod:fix/bar-screens-skip-placeholder
Closed

AnPod wants to merge 1 commit into
omacom:quattrofrom
AnPod:fix/bar-screens-skip-placeholder

Conversation

@AnPod

@AnPod AnPod commented Sep 27, 2026

Copy link
Copy Markdown

Summary

When monitors sleep or are unplugged, Qt substitutes a nameless placeholder screen and Hyprland creates its headless FALLBACK output. A bar built for either outlives the monitors' return: the compositor drops it onto whichever monitor comes back first, where it stacks over that monitor's bar with duplicate ShellIpc handlers and leaked child processes (#13072).

This filters out nameless and FALLBACK screens in BarModel.realScreens, ensuring bar surfaces are only created for real outputs with confirmed geometry.

Fixes #13072

Testing

  • Updated test/shell.d/bar-test.sh with assertions for screen filtering.
  • Ran bash test/shell.d/bar-test.sh (all 98 assertions pass).

@llstrk

llstrk commented Sep 28, 2026

Copy link
Copy Markdown

Automated AI review

Community review: Independent automated community review, unaffiliated with the Omarchy team, intended to help prepare PRs for their review.

Verified: the bar filter does what the description says, and existing bars are not rebuilt when the filtered list is recomputed.

Screen Name Size Gets a bar
Qt placeholder (no outputs) "" 0x0 no
Hyprland FALLBACK FALLBACK 1920x1080 no (name check only)
Real monitor reported at 0x0 (defensive case, checked with a stand-in) DP-1 0x0 not yet; gets one when its size arrives
Real monitor / user headless output HDMI-A-1 / HEADLESS-2 real mode yes
  • In the dependency sources, Qt's placeholder screen has an empty name and empty geometry. Hyprland 0.56.2 advertises FALLBACK as an ordinary wl_output at 1920x1080, so only the name check excludes it. Qt Wayland names a real screen before adding it, so a real screen never reaches the filter without a name. A size that arrives or changes later is still tracked by the binding.
  • A headless Quickshell run with this BarModel.js, a realScreens binding and a Variants over stand-in screen objects behaved as follows. A screen whose size arrived later got an instance. A resolution change, and the placeholder and FALLBACK leaving the list, did not rebuild existing instances. Variants compares the same screen object as equal across recomputations.
  • bar-test.sh passes on this head and with the head merged into current quattro. Its new checks fail with the merge-base Bar.qml/BarModel.js, without the FALLBACK check, without the geometry check, or with one Variants reverted.

The review found an overlap with an open PR, an unrelated Windows VM commit, a "Fixes" link that covers only part of #13072, and a test guard that passes on zero matches.

Same change as open PR #13165

#13165 ("Skip placeholder screens when building bars", open since 24 Sep) makes the same bar change:

  • Bar.qml after this PR is byte-identical to Bar.qml at Skip placeholder screens when building bars #13165's head.
  • isRealScreen/realScreens in BarModel.js are character-identical. Here they sit at the end of the file with a reworded comment.
  • The bar-test.sh assertions are the same. This PR only adds blank lines around them.

Impact: there are two open PRs with the same bar code, linking different issues (#13072 here, #13163 there).

Suggested change: state what this PR adds over #13165, or consolidate the change in one PR.

Unrelated Windows VM commit

Commit 2b4b0eba ("Clear setgid when hardening Windows VM mount folders (#1)") changes bin/omarchy-windows-vm and three tests. The description does not mention it, and it is unrelated to the bar. The same commit is also in #13473, #13478 and #13480. Dedicated PRs with the same chmod 00700 are open as well (#13213 at both sites, #13154 at the caller-mount site), along with others that use a symbolic mode (for example #12323).

Verified: the fix itself is correct. With GNU chmod 9.11, chmod 0700 on a 2777 directory leaves 2700 and chmod 00700 gives 700, also through /proc/PID/fd. The new windows-vm-compose-test.sh case fails when the old line is restored.

Impact: the same unrelated change travels in several open PRs alongside the dedicated ones. Once one lands, the others become redundant or conflict at the same lines.

Suggested change: drop the commit from this PR. If it stays, the comments "Use a leading 0 in the mode" and "Leading 0 clears setuid/setgid" are imprecise, because 0700 already has a leading zero. What matters is the fifth digit (00700).

"Fixes #13072" covers only part of the issue

all outputs drop
  ├─ placeholder / FALLBACK bar            -> no longer built (this PR)
  ├─ real monitor's bar                    -> still destroyed, rebuilt when outputs return
  └─ background, notification windows      -> still created for every Quickshell.screens entry,
                                              placeholder included (Background.qml:210,
                                              notifications/Service.qml:953)

With the filter, no bar, bar widgets or widget IPC handlers are built for the placeholder or FALLBACK. The real monitor's bar is still removed from Variants when its screen leaves Quickshell.screens, and it is rebuilt when the screen returns. A widget that leaves a process behind on teardown can therefore still do so once per cycle. This would apply to the third-party plugin case in #13072 if its process outlives the widget's teardown; that plugin's code was not examined. The background window is always mapped, so the "Layershell screen does not correspond to a real screen" warning in #13072's log is still expected.

Impact: "Fixes #13072" would close an issue that the change resolves only partly.

Suggested change: use "Refs #13072", or state the partial scope in the description.

Count guard passes when nothing matches

test/shell.d/bar-test.sh:28 runs (( $(rg -c 'model:\s*root\.realScreens' ...) != 3 )). When rg finds nothing it prints nothing, so this becomes (( != 3 )). That is an arithmetic syntax error, the if condition is false, and the script continues even under set -euo pipefail.

A diagnostic mutant pointed all three Variants at an unfiltered readonly property var barScreens: Quickshell.screens and kept the realScreens line. bar-test.sh still exited 0 with 97 checks, and the only sign of the problem was the syntax error on stderr. Open PR #6501 changes these same three models to root.barScreens, so a conflict resolution in its favour on those three lines could drop the filter without failing this test.

Optional test improvement: default the count to 0. This version fails for the mutant above and passes on this head:

real_screen_models=$(rg -c 'model:\s*root\.realScreens' "$ROOT/shell/plugins/bar/Bar.qml" || true)
if (( ${real_screen_models:-0} != 3 )); then

Smaller note: the description says 98 assertions pass. This head prints 97 ok lines.


Review information

Test scope: Source review of the head, merge-base and current quattro, plus Quickshell 0.3.1, Qt 6.11.2 Wayland, Hyprland 0.56.2, and open PRs #13165, #6501 and the Windows VM setgid PRs. bar-test.sh, windows-vm-test.sh, windows-vm-compose-test.sh (with a synthetic account entry) and diagnostic mutants ran in an isolated sandbox on this head, the merge-base, and the head merged into quattro. GNU chmod 9.11 behaviour was checked by running it there. Screen handling was checked with a headless Quickshell run over stand-in screen objects. No compositor, real monitors or hotplug were available, so the unplug and replug sequence itself was not observed. windows-vm-mount-boundary-test.sh could not run in the sandbox.

AI process: Opus 5.5 Medium coordination and synthesis, Opus 5.5 Xhigh technical review and final fact check, GPT 6 Sol Xhigh search for related issues, Opus 5.5 Medium editorial check.

Opt out: To stop receiving these reviews, reply to this comment saying so.

Co-authored-by: AnPod <AnPod@users.noreply.github.com>
@cursor
cursor Bot force-pushed the fix/bar-screens-skip-placeholder branch from 8ae1446 to ec58ce8 Compare September 29, 2026 21:09
@omarchybot omarchybot added the bug Something isn't working label Oct 1, 2026
@omarchybot

Copy link
Copy Markdown
Collaborator

Closing this as a duplicate of #13165, which was opened on 24 September against quattro and makes the same change.

Comparing the two diffs line by line: Bar.qml is identical, isRealScreen and realScreens in BarModel.js are identical apart from the wording of the comment above them, and the bar-test.sh assertions are the same. The one code difference favours #13165: its check that all three Variants use root.realScreens defaults the rg -c count to 0, while the one here becomes (( != 3 )) when nothing matches, which is an arithmetic error that lets the test pass.

What this pull request adds is the link to #13072. Whether #13165 should also close that issue is for the maintainer to decide on #13165; the bar change only partly addresses #13072, since the background and notification windows are still created for the placeholder screen.

Compared by Claude Opus 5.5; Codex Medium was asked separately, from the two diffs alone, and agreed they are the same change (independence is not guaranteed). Nothing was run for this, because closing a duplicate needs only the comparison. If this is wrong, reopen it.

@omarchybot omarchybot added the duplicate This issue or pull request already exists label Oct 3, 2026
@omarchybot omarchybot closed this Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working duplicate This issue or pull request already exists

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Idle wake recreates shell panel components without destroying the old ones (duplicate IpcHandlers, leaked child processes)

4 participants