Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
KETHERCORTEX opened this issue Sep 24, 2024 · 5 comments
Labels
bug Undesirable behavior

Comments

@KETHERCORTEX
Copy link

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:
- paperwm@paperwm.github.com
- dash-to-dock@micxgx.gmail.com
- NotificationCounter@coolllsk
- switcher@landau.fi
- windowIsReady_Remover@nunofarruca@gmail.com
- ddterm@amezin.github.com
- undecorate@sun.wxg@gmail.com
- window-title-is-back@fthx
- ubuntu-appindicators@ubuntu.com
@KETHERCORTEX KETHERCORTEX added the bug Undesirable behavior label Sep 24, 2024
@AquariusDue
Copy link

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

@Lythenas
Copy link
Collaborator

Lythenas commented Dec 3, 2024

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. :(

@lost-melody
Copy link
Collaborator

lost-melody commented Jan 10, 2025

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 paperwm@paperwm.github.com
  3. enable paperwm: gnome enable paperwm@paperwm.github.com

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

@Thesola10
Copy link
Collaborator

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)

@Lythenas
Copy link
Collaborator

Lythenas commented Jan 10, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Undesirable behavior
Projects
None yet
Development

No branches or pull requests

5 participants