Re: [STUMP] add-head crashes

[David Bjergaard]

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.

Cheers,

    Dave

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