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

Weird window behavior on screen unlock #182

Closed
kitlaan opened this issue Feb 22, 2023 · 9 comments · Fixed by #183
Closed

Weird window behavior on screen unlock #182

kitlaan opened this issue Feb 22, 2023 · 9 comments · Fixed by #183

Comments

@kitlaan
Copy link
Contributor

kitlaan commented Feb 22, 2023

It's me again; with another tweak to #177. The root cause is definitely related to DisplayPort, but still not entirely sure why.

Either way, another tweak to the code around these lines may help.
https://github.com/jmmaranan/forge/blob/4f090ae4789eca0a12c615378a99176b44e129eb/window.js#L177-L186

For example,

        display.connect("workareas-changed", (_display) => {
          if (global.display.get_n_monitors() == 0) {
            Logger.debug(`workareas-changed: no monitors, ignoring signal`);
            return;
          }
          if (this.tree.getNodeByType("WINDOW").length > 0) {

Basically, if there's no valid monitor, computing anything in renderTree() is probably going to result in odd state changes, so when the monitor/workspace comes back "for real", that state will have already been wedged.

That being said, I have no clue what side-effects just ignoring this event has. Ideas?

@jmmaranan
Copy link
Collaborator

jmmaranan commented Feb 22, 2023

Hi @kitlaan, work areas change event has minimal impact on panel resize, and impact if you are using dynamic workspaces.

This is also used by the extension when it first activates.

It looks like attach/detach of display also triggers this event as what you seem to have mentioned.

Just curious, do you have a power saving extension or setting turned on? Are you on laptop or desktop machine?

@kitlaan
Copy link
Contributor Author

kitlaan commented Feb 22, 2023

For example, here's what the tree looks like if you let that code through when there's "no monitor attached":

Forge: [DEBUG] workareas-changed: monitorcount=0 workarea-height=0 workarea-width=0
Forge: [DEBUG] render tree from workareas-changed
...
Forge: [DEBUG] #--> ROOT#- @
Forge: [DEBUG]     |
Forge: [DEBUG]     *--> WORKSPACE#0 @ws0
Forge: [DEBUG]     |   |
Forge: [DEBUG]     |   *--> MONITOR#0 @mo0ws0,layout:HSPLIT,rect:0x0+0+0
Forge: [DEBUG]     |   |   |
Forge: [DEBUG]     |   |   *--> WINDOW#0 @class:'Google-chrome',title:'Terminal Right',string:'[object instance wrapper GType:MetaWindowX11 jsobj@0x2fd792de8ad8 native@0x5650d7e02fd0]',rect:0x0+0+0
...

Sometimes when I come back to the computer with this symptom, I lose windows (either behind others, or slightly off the screen, or something-hard-to-describe). And a second later a "window-entered-monitor" event happens with gnome having everything proper again.

So far by dropping the "workareas-changed" event when there's no monitor (and thus the rect is 0x0), things are behaving "better"; though I haven't run this patch for long enough to draw any conclusions yet.

@kitlaan
Copy link
Contributor Author

kitlaan commented Feb 22, 2023

Just curious, do you have a power saving extension or setting turned on? Are you on laptop or desktop machine?

Just a default Ubuntu 22.04 desktop gnome install, with a nvidia card, and a single DisplayPort monitor. It tends to happen after locking the session and walking away for "a while" (20 min? more? overnight?)

@jmmaranan
Copy link
Collaborator

I think this is solution for now. Since Forge by default should not allow invalid conditions as much as possible.

@jmmaranan
Copy link
Collaborator

To clarify, if the monitor or its sizes are invalid, do not trigger the workareas change event or even more so, you might need to spend more time after this new change to see what else has issues with displayport being invalid.

I think it is better for you to send the PR since you can replicate it on your end. Let me know

@kitlaan
Copy link
Contributor Author

kitlaan commented Feb 23, 2023

I'll do a PR in a few days of testing; just wanted to get your expertise before spending more time on it.

FYI: the monitor-count being 0 will cause the sizes to be invalid. I haven't seen zero height/width values otherwise yet.

@kitlaan
Copy link
Contributor Author

kitlaan commented Feb 24, 2023

Digging into the weird DisplayPort behavior, turns up posts (some multi-years old). Most relate to power-savings (DPMS, monitor detection, etc) but no real resolution even today. Even found some posts that say to plug in a dummy HDMI/DisplayPort plug to fake a connected monitor!

For reference, just google "gnome-shell locking screen causes windows to be moved"

@jmmaranan
Copy link
Collaborator

Hi @kitlaan - I still get empty tab titles occasionally on unlock, but dragging affected to another window tab fixes it. So your fix makes the user still be able to continue instead of restarting the shell.

@kitlaan
Copy link
Contributor Author

kitlaan commented Apr 2, 2023

Glad to know.

FYI: I'm currently chasing three weird behaviors, but trying to gather some more info before filing an issue (or making a PR):

  • after a fresh reboot, where starting Chrome afresh and restoring the previous session... Forge seems to arrange the windows in a weird way -- I think Forge is tracking all the workspace positions on Workspace 0, even though the windows are scattered to their proper workspaces.
  • sometimes resizing tracks the floating window... I have Zoom floating, and sometimes when moving it around or resizing windows, it affects the other windows. I haven't paid attention with the latest release though.
  • performance issue, where if you're playing with window positioning, you can get a massive chain of 1-child containers. This is probably easy to optimize/reap.

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

Successfully merging a pull request may close this issue.

2 participants