[PATCH] Fix fit-frame-to-buffer for multi-monitor setup.

[PATCH] Fix fit-frame-to-buffer for multi-monitor setup.

[Сергей Трофимов]

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]

I've noticed `fit-frame-to-buffer` results in incorrect frame position
when the frame is not a child frame. I've looked into the code and
seems that there are two issues: first is that frame monitor is
ignored, always first monitor in `display-monitor-attributes-list` is
used. Second is that `workarea` and `geometry` are in the form of `(X
Y WIDTH HEIGHT)` but code treats them as `(X1 Y1 X2 Y2)`. I'm
providing a patch that fixes both issues.

[-- Attachment #2: 0001-Fix-fit-frame-to-buffer-for-multi-monitor-setup.patch --]
[-- Type: text/x-patch, Size: 2424 bytes --]

From 59a83f32fe5bb8920351bdf8e2fbcdd0dee1aba7 Mon Sep 17 00:00:00 2001
From: Sergey Trofimov <sarg@sarg.org.ru>
Date: Sat, 29 Feb 2020 14:00:38 +0100
Subject: [PATCH] Fix fit-frame-to-buffer for multi-monitor setup.

---
 lisp/window.el | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index bd825c0..3190c15 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8806,8 +8806,7 @@ parameters of FRAME."
            (parent (frame-parent frame))
            (monitor-attributes
             (unless parent
-              (car (display-monitor-attributes-list
-                    (frame-parameter frame 'display)))))
+              (frame-monitor-attributes frame)))
            ;; FRAME'S parent or display sizes.  Used in connection
            ;; with margins.
            (geometry
@@ -8816,11 +8815,11 @@ parameters of FRAME."
            (parent-or-display-width
             (if parent
                 (frame-native-width parent)
-              (- (nth 2 geometry) (nth 0 geometry))))
+              (nth 2 geometry)))
            (parent-or-display-height
             (if parent
                 (frame-native-height parent)
-              (- (nth 3 geometry) (nth 1 geometry))))
+              (nth 3 geometry)))
            ;; FRAME's parent or workarea sizes.  Used when no margins
            ;; are specified.
            (parent-or-workarea
@@ -8882,13 +8881,13 @@ parameters of FRAME."
                                 (window--sanitize-margin
                                  (nth 2 margins) left-margin
                                  parent-or-display-width))
-                           (nth 2 parent-or-workarea)))
+                           (+ (nth 0 parent-or-workarea) (nth 2 parent-or-workarea))))
            (bottom-margin (if (nth 3 margins)
                               (- parent-or-display-height
                                  (window--sanitize-margin
                                   (nth 3 margins) top-margin
                                   parent-or-display-height))
-                            (nth 3 parent-or-workarea)))
+                            (+ (nth 1 parent-or-workarea) (nth 3 parent-or-workarea))))
            ;; Minimum and maximum sizes specified for FRAME.
            (sizes (or (frame-parameter frame 'fit-frame-to-buffer-sizes)
                       fit-frame-to-buffer-sizes))
-- 
2.25.0

Re: [PATCH] Fix fit-frame-to-buffer for multi-monitor setup.

[martin rudalics]

 > I've noticed `fit-frame-to-buffer` results in incorrect frame position
 > when the frame is not a child frame. I've looked into the code and
 > seems that there are two issues: first is that frame monitor is
 > ignored, always first monitor in `display-monitor-attributes-list` is
 > used.

Good.  I never really checked that since I always use only one monitor.
Besides, 'frame-monitor-attributes' should be preferable anyway.

 > Second is that `workarea` and `geometry` are in the form of `(X
 > Y WIDTH HEIGHT)` but code treats them as `(X1 Y1 X2 Y2)`.

I believe the reasoning was to avoid moving the frame whenever it
already appears within its margins and thus constrain it to the region
to the right of and below (X, Y).  But my memory may fail.  Could you
provide an example where the original code results in bad placement?

 > I'm
 > providing a patch that fixes both issues.

Thanks, martin

Re: [PATCH] Fix fit-frame-to-buffer for multi-monitor setup.

[Sergey Trofimov]

On Sat, 29 Feb 2020 at 17:05, martin rudalics <rudalics@gmx.at> wrote:
>
>  > I've noticed `fit-frame-to-buffer` results in incorrect frame position
>  > when the frame is not a child frame. I've looked into the code and
>  > seems that there are two issues: first is that frame monitor is
>  > ignored, always first monitor in `display-monitor-attributes-list` is
>  > used.
>
> Good.  I never really checked that since I always use only one monitor.
> Besides, 'frame-monitor-attributes' should be preferable anyway.
>
>  > Second is that `workarea` and `geometry` are in the form of `(X
>  > Y WIDTH HEIGHT)` but code treats them as `(X1 Y1 X2 Y2)`.
>
> I believe the reasoning was to avoid moving the frame whenever it
> already appears within its margins and thus constrain it to the region
> to the right of and below (X, Y).  But my memory may fail.

Yes, the intention is correct, but few formulas are written in a wrong
assumption of return value of (display-monitor-attributes-list).

> Could you  provide an example where the original code results in bad placement?

I have used this snippet to debug. My setup is:
Screen 0: current 1920 x 1980
LVDS-1 connected 1600x900+0+1080
HDMI-1 connected primary 1920x1080+0+0

LVDS-1 is placed under HDMI-1

;; I'm create a frame on LVDS1 in the upper area of the monitor
(setq sarg/frame
      (posframe-show
       "*scratch*"
       :width 20
       :height 20
       :min-width 20
       :background-color "#ff0000"
       :min-height 20
       :position '(100 . 1200)
       :override-parameters '((parent-frame nil))))

;; this function works correctly and returns size of LVDS-1 where the
frame is shown
(frame-monitor-workarea sarg/frame)
;; (0 1080 1600 900) - (X Y W H)
;; notice that the formula for parent-or-display-width
;;  (- (nth 3 geometry) (nth 1 geometry))
;; is producing incorrect value

(mapcar (lambda (p) (frame-parameter sarg/frame p)) '(left top width height))
;; (100 1200 20 20) - frame is sized and placed correctly

;; the buffer contents are slightly bigger, so the frame should be
resized accordingly
(fit-frame-to-buffer sarg/frame)

(mapcar (lambda (p) (frame-parameter sarg/frame p)) '(left top width height))
;; (100 1004 50 4)
;; new size is correct but the frame jumped on the first monitor, 1004 < 1080

(posframe-delete-all)

Re: [PATCH] Fix fit-frame-to-buffer for multi-monitor setup.

[martin rudalics]

 > (frame-monitor-workarea sarg/frame)
 > ;; (0 1080 1600 900) - (X Y W H)
 > ;; notice that the formula for parent-or-display-width
 > ;;  (- (nth 3 geometry) (nth 1 geometry))
 > ;; is producing incorrect value

Thanks, I understand now.

Eli, is it OK to install Sergey's patch into Emacs 27?

Thanks again, martin

Re: [PATCH] Fix fit-frame-to-buffer for multi-monitor setup.

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Sun, 1 Mar 2020 09:52:25 +0100
> 
> Eli, is it OK to install Sergey's patch into Emacs 27?

Yes, thanks.

Re: [PATCH] Fix fit-frame-to-buffer for multi-monitor setup.

[martin rudalics]

 >> Eli, is it OK to install Sergey's patch into Emacs 27?
 >
 > Yes, thanks.

Pushed now.

Sergey: I installed this as a tiny change based on the assumption that
you have not signed papers for Emacs yet.  I think it would be a good
idea to sign them so we can accept larger changes from you in the
future.

Thanks, martin