Re: [STUMP] add-head crashes

[Eric Abrahamsen]

Thanks to you both for the clarification! That makes a lot more sense,
particularly the bit about Xinerama. So I guess the practical upshot is,
if you're not using Xinerama you have move between monitors using
sother/snext/etc, instead of the frame movement commands.

Anyway, I've been fooling with this patch, and as far as I can tell,
simply changing the sort key in add-head is enough to make it work here.
I've tested adding and removing heads, starting with one group with one
frame, one group with two frames, two groups with one frame each, and
two groups with two frames. Seems like that should cover everything.

Mind you, I am totally in the dark as to 1) why the original pull
request used such a complicated sort key in add-head, and 2) how on
earth changing the sort key could alter the value of the variable 'head'
that gets passed to `group-add-head'. Doesn't make sense to me.

But no errors, anyway...


z_axis <address@hidden> writes:

> ; A head is a physical monitor. A screen is a section of video memory
> ; that heads are mapped on to. When using xinerama you generally have
> ; 1 screen and many heads. When not using xinerama each screen is a
> ; physical monitor so you have many screens with only one head on them.
>
> Regards!
> 在 Tue, 25 Feb 2014 11:35:00 +0800，Eric Abrahamsen
> <address@hidden> 写道:
>
>> David Bjergaard <address@hidden> writes:
>>
>>> Hi Eric,
>>>
>>> This is in the experimental branch but not in master. I just tested it
>>> and it appears to work.  Can you clarify what you mean by:
>>>> Yes, that does solve it (though obviously the frame on the new
>>>> head goes
>>>> back to being mis-sized).
>>> The original bug was that if you unplugged an external monitor the
>>> frames were not being deleted properly, the patch introduced the bug you
>>> found. Namely that add-head was getting a list of frames instead of a
>>> frame itself.
>>>
>>> As far as I can tell from the code, this was due to the way the heads
>>> were being keyed. I reverted that functionality, and left the remainder
>>> of the patch in place.
>>>
>>> I want to make sure that my patch doesn't cause a regression of the
>>> original pull request, and that my patch fixes the crash you're seeing.
>>
>> Okay, I'd like to be a little more helpful in testing this and maybe
>> providing a solution, and I realized to my embarrassment that the main
>> problem is that I don't really understand what a screen *is*. The manual
>> says that screens logically contain heads, which doesn't get me much of
>> anywhere. It seems that no matter how many monitors, groups, and frames
>> I've got, I've only ever got one screen.
>>
>> If someone could take a second to help explain this, that would be much
>> appreciated...
>>
>> Thanks,
>> Eric
>>
>>> Eric Abrahamsen <address@hidden> writes:
>>>
>>>> David Bjergaard <address@hidden> writes:
>>>>
>>>>> Hi Eric,
>>>>>
>>>>> Can you try the attached patch and let me know if it fixes it?
>>>>>
>>>>> Cheers,
>>>>>
>>>>>     Dave
>>>>
>>>> Yes, that does solve it (though obviously the frame on the new
>>>> head goes
>>>> back to being mis-sized).
>>>>
>>>> Do you still want a note on the wiki?
>>>>
>>>> E
>>>>
>>>>> diff --git a/head.lisp b/head.lisp
>>>>> index 01b2dc4..c9fd50a 100644
>>>>> --- a/head.lisp
>>>>> +++ b/head.lisp
>>>>> @@ -115,9 +115,7 @@
>>>>>  (defun add-head (screen head)
>>>>>    (dformat 1 "Adding head #~D~%" (head-number head))
>>>>>    (setf (screen-heads screen) (sort (push head (screen-heads
>>>>> screen)) #'<
>>>>> -                                    :key (lambda (head)
>>>>> -                                           (+ (* (head-x head)
>>>>> (screen-height (current-screen)))
>>>>> -                                              (head-y head)))))
>>>>> +                                    :key 'head-number))
>>>>>    (dolist (group (screen-groups screen))
>>>>>      (group-add-head group head)))
>>>>>
>>>>> @@ -142,6 +140,14 @@
>>>>>  (defun scale-screen (screen heads)
>>>>>    "Scale all frames of all groups of SCREEN to match the
>>>>> dimensions of HEADS."
>>>>>    (let ((oheads (screen-heads screen)))
>>>>> +    (when (< (length heads) (length oheads))
>>>>> +      ;; Some heads were removed (or cloned), try to guess which.
>>>>> +      (dolist (oh oheads)
>>>>> +        (dolist (nh heads)
>>>>> +          (when (and (= (head-x nh) (head-x oh))
>>>>> +                     (= (head-y nh) (head-y oh)))
>>>>> +          ;; Same screen position; probably the same head.
>>>>> +            (setf (head-number nh) (head-number oh))))))
>>>>>      (dolist (h (set-difference oheads heads :test '= :key
>>>>> head-number))
>>>>>        (remove-head screen h))
>>>>>      (dolist (h (set-difference heads oheads :test '= :key
>>>>> head-number))
>>>>>
>>>>>
>>>>>
>>>>> Eric Abrahamsen <address@hidden> writes:
>>>>>
>>>>>> David Bjergaard <address@hidden> writes:
>>>>>>
>>>>>>> I've had unexpected problems with stumpwm handling extra
>>>>>>> screens too, I
>>>>>>> just assumed it was happening elsewhere. I'll look into this.
>>>>>>> In the
>>>>>>> mean time, (if you have time) you could try reverting that code
>>>>>>> to what
>>>>>>> it was before the pull request and report your results (just to make
>>>>>>> sure it was introduced by that pull request).
>>>>>>
>>>>>> Yup, I reverted head.lisp to the commit just prior to that merge (it
>>>>>> didn't look like that file has really been touched since the
>>>>>> merge), and
>>>>>> recompiled, and it worked fine. Switched back to HEAD, and
>>>>>> breakage --
>>>>>> repeatable breakage, too. So long as there is more than one
>>>>>> frame in the
>>>>>> other group, it borks.
>>>>>>
>>>>>>>
>>>>>>> Eric Abrahamsen <address@hidden> writes:
>>>>>>>
>>>>>>>> Since the changes in f168e3b7116c106ded81343f07ebd29bc784f2a0
>>>>>>>> (I can
>>>>>>>> only assume that's it), stump crashes when I add an extra
>>>>>>>> screen. I'm
>>>>>>>> using stump on X with no DE, and adding the screen with this
>>>>>>>> script,
>>>>>>>> bound to a keypress:
>>>>>>>>
>>>>>>>> #!/bin/sh
>>>>>>>> xrandr --output LVDS1 --mode 1280x800 --pos 1440x100 --rotate
>>>>>>>> normal --output DP1 --off --output VGA1 --mode 1440x900 --pos
>>>>>>>> 0x0 --rotate normal
>>>>>>>>
>>>>>>>> This used to work all right, but now crashes stumpwm. I can't
>>>>>>>> copy the
>>>>>>>> actual backtrace, since copy-unhandled-error doesn't seem to
>>>>>>>> actually
>>>>>>>> copy it in this case, but here's the top few lines. At the
>>>>>>>> time of the
>>>>>>>> crash, I had two groups, one with a single frame, and the
>>>>>>>> other with two
>>>>>>>> frames (one of the frames empty, the other with Pidgin's
>>>>>>>> "Buddy List" in
>>>>>>>> it).
>>>>>>>>
>>>>>>>> The value
>>>>>>>>   (#S(frame 0 #S(TILE-WINDOW "Buddy List" #x1200056) 0 0 640 800)
>>>>>>>>    #S(frame 1 nil 640 800))
>>>>>>>> is not of type
>>>>>>>>   STUMPWM::FRAME.
>>>>>>>>
>>>>>>>> Backtrace for: etc
>>>>>>>> 0: ((:METHOD GROUP-ADD-HEAD (TILE-GROUP t)) #<TILE-GROUP
>>>>>>>> {etc}> #S(frame 1 NIL 0 0 1440 900)) [fast-method]
>>>>>>>> 1: (ADD-HEAD #S<screen #<XLIB:SCREEN :0.0 1280x800x24
>>>>>>>> TRUE-COLOR>> #S(frame 1 nil 0 0 1440 900))
>>>>>>>> 2: (SCALE-SCREEN #S<screen #<XLIB:SCREEN :0.0 1280x800x24
>>>>>>>> TRUE-COLOR>> (#S(frame 0 NIL 1440 800) #S(frame 1 NIL 0 0 1440
>>>>>>>> 900)))
>>>>>>>>
>>>>>>>> So, whatever's going on, stump isn't expecting two frames in
>>>>>>>> the group
>>>>>>>> being added, but I really don't know how this is meant to
>>>>>>>> work. Hope
>>>>>>>> this is useful...
>>>>>>>>
>>>>>>>> Eric
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Stumpwm-devel mailing list
>>>>>>>> address@hidden
>>>>>>>> https://lists.nongnu.org/mailman/listinfo/stumpwm-devel
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Stumpwm-devel mailing list
>>>>>> address@hidden
>>>>>> https://lists.nongnu.org/mailman/listinfo/stumpwm-devel
>>>>> _______________________________________________
>>>>> Stumpwm-devel mailing list
>>>>> address@hidden
>>>>> https://lists.nongnu.org/mailman/listinfo/stumpwm-devel
>>>>
>>>>
>>>> _______________________________________________
>>>> Stumpwm-devel mailing list
>>>> address@hidden
>>>> https://lists.nongnu.org/mailman/listinfo/stumpwm-devel
>>
>>
>> _______________________________________________
>> Stumpwm-devel mailing list
>> address@hidden
>> https://lists.nongnu.org/mailman/listinfo/stumpwm-devel