PaperWM icon indicating copy to clipboard operation
PaperWM copied to clipboard

Full screen windows end up on Scratch layer after turning the screens off and on again

Open KETHERCORTEX opened this issue 1 year ago • 6 comments

Describe the bug Full screen applications end up on a scratch layer after the screens get turned off either by GNOME's saving (Settings > Power Saving > Screen Blank) or manually executing xdg-screensaver activate.

To Reproduce Steps to reproduce the behavior:

  1. Open a full screen window that is not on scratch layer
  2. Either wait for GNOME's Screen Blank timeout or launch xdg-screensaver activate
  3. Wait until the screens will turn off
  4. Move mouse or press keys to wake the GNOME back up
  5. See that full screen windows have Scratch option turned on

Expected behavior Windows should keep their Scratch option/state unchanged after turning the screen off and on again

System information:

Distribution: Ubuntu 24.04.1 LTS (Noble Numbat)
GNOME Shell: 46.0
Display server: Wayland
PaperWM version: 47.0.0
Enabled extensions:
- [email protected]
- [email protected]
- NotificationCounter@coolllsk
- [email protected]
- windowIsReady_Remover@[email protected]
- [email protected]
- [email protected]@gmail.com
- window-title-is-back@fthx
- [email protected]

KETHERCORTEX avatar Sep 24 '24 12:09 KETHERCORTEX

It's happening to me too on Ubuntu 24.04 LTS with similar extensions.

AquariusDue avatar Dec 02 '24 17:12 AquariusDue

I can also confirm this. I tried to debug this a bit, but unfortunately I didn't really find anything useful. I also can't see any logs that are related to the problem. :(

Lythenas avatar Dec 03 '24 17:12 Lythenas

So the minimum reproduce is:

  1. make a window full-screen (or mark it "Always on Top")
  2. disable paperwm while the full-screen window is focused: gnome disable [email protected]
  3. enable paperwm: gnome enable [email protected]

The reason is that paperwm doesn't really remember windows' scratch state across extension lifetimes, instead it saves the state to meta_window[float], where float is initialized on enabled and zeroed on disabled:

moves always-on-top windows into scratch layer upon paperwm enabled:

and makes above full-screen windows upon focused:

To "fix" this issue, we could tweak the conditions at tilling.js:L2120 like this:

if (meta_window.above && !meta_window.fullscreen
  || meta_window.above && meta_window.fullscreen && meta_window._fullscreen_above
  || meta_window.minimized) {
  // ...
}

or we could unmake_above full-screen windows before disabling paperwm, but these don't actually fix the issue because paperwm still doesn't remember windows' scratch states.

To "restore windows' scratch states across paperwm re-enabling", we'll need to tweak a bunch of things around the scratch mechanism.

What do you think? @Lythenas, @Thesola10

lost-melody avatar Jan 10 '25 04:01 lost-melody

Nice find @lost-melody, that sounds about right (almost)

PaperWM does seem capable of remembering which windows are tiled and where (to an extent) or deduces it from window location on startup, meaning the remaining cases are ambiguous situations like maximized and full screen windows.

Ideally we could find a way to keep the extension fully alive while locked, as this would also address much prolonged unlock times (PaperWM takes a while to start, especially on a busy desktop)

Thesola10 avatar Jan 10 '25 06:01 Thesola10

Keeping the extension enable should be technically doable (see also here: https://gjs.guide/extensions/overview/anatomy.html#session-modes) but I'm not 100% sure if we might get problems when they review the extension for EGO. I suspect that this is considered bad practice. According to the docs enable and disable are still called, even with this set. So not sure if that would even help.

I think that float Symbol (and probably all others) could also be just a const at the top level, instead of initializing in enable. In JS, Symbols are immutable anyway, so it would be like writing const MAGIC_NUMBER = 42 at the top level. And there should be nothing wrong with that IMO. But it's possible the current implementation exists because of an EGO review. If I remember correctly there is a guideline that says we need to initialize global variable in enable. But I think constants like these symbols or magic numbers should be an exception.

Alternatively we could also use Symbol.for("key"), which always returns the same symbol. But that kind of defeats the purpose of symbols. Then we could just use a string key to index instead of a symbol.

Lythenas avatar Jan 10 '25 17:01 Lythenas

Rather than keeping the extension enabled, could SaveState() be used to keep track of scratch windows when the screen is locked? It appears to save state for the next enable().

wisdom-in-snow avatar Apr 09 '25 12:04 wisdom-in-snow