Made scale-screen add heads before removing heads

[tiberious726]

This resolves the crash in #647 and #763 .

Ultimately, this happens because group-remove-head can't handle there not being any frames in a group. If you try to remove the only head, group-remove-head will delete its frames from the group's tile-group-frame-tree, leaving it empty, then it will try to grab the group's first frame ;; just set current frame to whatever., which will, unfortunately, be nil, and proceed to set all of the windows' window-frame to nil, causing this problem.

https://github.com/stumpwm/stumpwm/blob/master/tile-group.lisp#L196-L208

This pull request solves the problem by simply preventing there from not being any heads during normal operation: adding new heads before removing the old ones if both happen in a single :CONFIGURE-NOTIFY event.

I do, however, think that stumpwm ought be able to handle not having any actual heads attached, at least temporarily, and this patch is really papering over the issues with that (and, in fact, prevents tile-window's frame slots from being set to nil if you have safety turned up on sbcl).

Furthermore, it doesn't looks like frames are properly adopted and scaled when moving between heads of different resolution.

[tiberious726]

I got a little bit further in tracking down the other issue tonight:

Visually, there is a frame that is correctly sized for the old head's resolution, but taking up the same visual space as the correctly sized frames (above or below them) on the new (only) head. You can create new frames incorrectly configured by generating them while this frame is highlighted, and cycle-through them with next-window. You can "jump" to a correctly configured frame by raising one of its windows.

STUMPWM>  (tile-group-frame-tree (current-group))
(#<FRAME 2 #<TILE-WINDOW "*sly-mrepl for sbcl*" #xA0013E> 0 0 2560 1440>)
STUMPWM> (window-frame (current-window))
#<FRAME 2 #<TILE-WINDOW "*sly-mrepl for sbcl*" #xA0013E> 0 0 2560 1440>
STUMPWM> (window-frame (current-window))
#<FRAME 0 #<TILE-WINDOW "*sly-mrepl for sbcl*" #xA002A2> 0 0 1920 2160>

The last two commands were run from repls different windows (the last being from the one in the messed-up frame). Notice the sizes.

Some windows are left associated with frames no longer in any group's frame-tree (and other than the per-group frame trees I don't think stump keeps a global list of frames anywhere, you could get it by walking calling window-frame on a list of all windows tho---probably from xlib too).

However, it's not just the frame list that's messed up:

STUMPWM> (current-screen)
#<SCREEN #<XLIB:SCREEN :0.0 6400x2160x24 TRUE-COLOR>>
STUMPWM> (screen-width (current-screen))
2560 (12 bits, #xA00)
STUMPWM> (screen-height (current-screen))
1440 (11 bits, #x5A0)
STUMPWM> (describe (screen-number (current-screen)))
#<XLIB:SCREEN :0.0 6400x2160x24 TRUE-COLOR>
  [structure-object]

Slots with :INSTANCE allocation:
  ROOT                           = #<XLIB:WINDOW :0 7C8>
  WIDTH                          = 6400
  HEIGHT                         = 2160
  WIDTH-IN-MILLIMETERS           = 1693
  HEIGHT-IN-MILLIMETERS          = 571
  DEPTHS                         = ((24 #<XLIB:VISUAL-INFO 8-bit TRUE-COLOR #1=:0 33>..
  ROOT-DEPTH                     = 24
  ROOT-VISUAL-INFO               = #<XLIB:VISUAL-INFO 8-bit TRUE-COLOR :0 33>
  DEFAULT-COLORMAP               = #<XLIB:COLORMAP TRUE-COLOR :0 32>
  WHITE-PIXEL                    = 16777215
  BLACK-PIXEL                    = 0
  MIN-INSTALLED-MAPS             = 1
  MAX-INSTALLED-MAPS             = 1
  BACKING-STORES                 = :WHEN-MAPPED
  SAVE-UNDERS-P                  = NIL
  EVENT-MASK-AT-OPEN             = 0
  PLIST                          = NIL

While the screen object knows what size it should be, the actual underlying xlib object is much too large, in fact, it's the exact size it should be if both monitors were on (xrandr confirms that only one is enabled (or even connected)).

This and a review of the code leads me to, unfortunately, believe that this isn't caused by inverting the order of adding and removing screens, but rather is a race in handling :CONFIGURE-NOTIFY events.

[tiberious726]

Does Stumpwm ever tell xlib about the display size? If not, at least the second part of the previous comment is a cl-xlib bug:

STUMPWM> (xlib:display-roots *display*)
(#<XLIB:SCREEN :0.0 6400x2160x24 TRUE-COLOR>)

$ xrandr
Screen 0: minimum 320 x 200, current 2560 x 1440, maximum 16384 x 16384
eDP-1 connected primary 2560x1440+0+0 (normal left inverted right x axis y axis) 309mm x 174mm

$ xwininfo -root

xwininfo: Window id: 0x7c8 (the root window) (has no name)

  Absolute upper-left X:  0
  Absolute upper-left Y:  0
  Relative upper-left X:  0
  Relative upper-left Y:  0
  Width: 2560
  Height: 1440
  Depth: 24
  Visual: 0x21
  Visual Class: TrueColor
  Border width: 0
  Class: InputOutput
  Colormap: 0x20 (installed)
  Bit Gravity State: ForgetGravity
  Window Gravity State: NorthWestGravity
  Backing Store State: NotUseful
  Save Under State: no
  Map State: IsViewable
  Override Redirect State: no
  Corners:  +0+0  -0+0  -0-0  +0-0
  -geometry 2560x1440+0+0

The frame issue can be (hackily) resolved by:

(let ((a-correct-frame  (window-frame (current-window))))
  (mapcar #'(lambda (w) (setf (window-frame w) a-correct-frame)) (all-windows)))

and then getting all windows to refresh/redisplay.

I'm not sure how significant the xlib object being too large is, but it looks like things should function correctly if we stop that single window from continuing to be assigned to a frame that shouldn't exist anymore.

[PuercoPop]

Does Stumpwm ever tell xlib about the display size?
Do, we just query it and use xrandr to keep it synced

[PuercoPop]

This pull request solves the problem by simply preventing there from not being any heads during normal operation: adding new heads before removing the old ones if both happen in a single :CONFIGURE-NOTIFY event.

I do, however, think that stumpwm ought be able to handle not having any actual heads attached, at least temporarily, and this patch is really papering over the issues with that (and, in fact, prevents tile-window's frame slots from being set to nil if you have safety turned up on sbcl).

I agree that ultimately having no monitors should be supported, at least as a transitory state but your workaround should cover most scenarios. Thanks!

[tiberious726]

Did a bit more digging, it's actually the xlib "screen" size that is incorrect: it's just bigger than the display size.

STUMPWM> (setf (xlib:screen-height (screen-number (current-screen))) 1440)
1440 (11 bits, #x5A0)
STUMPWM> (setf (xlib:screen-width (screen-number (current-screen))) 2560)
2560 (12 bits, #xA00)

Fixes the xlib screen size and

STUMPWM> (setf (xlib:screen-width-in-millimeters (screen-number (current-screen))) 677)
STUMPWM> (setf (xlib:screen-height-in-millimeters (screen-number (current-screen))) 318)

fixes the modeline scaling. Interestingly, these are both about 2.19x larger than the values reported by xrandr, must be something being smart about HiDPI along the way.

So whatever should be resizing the screen on head-changes isn't working---tho so long as no windows were assigned to orphaned frames, I'm not sure anyone would have noticed their screen being larger than their actual display---since you were still constrained to a frame within your head, splitting and such would work correctly.

I'll make either new issues or new pull requests to keep track of this and the orphaned frame, depending on how quickly I figure it out.

[tiberious726]

This should also resolve #840 and #841