bug#74750: clone-frame and make-frame pixelwise issues

bug#74750: clone-frame and make-frame pixelwise issues

[Ship Mints]

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

While trying to reconcile pixelwise frame sizing behaviors, I narrowed down
two related issues.

clone-frame does not correctly clone frames on a pixelwise basis.

make-frame's text-pixels geometry support does not produce specified
pixelwise geometry. This also impacts frameset-restore's ability to
precisely reproduce pixelwise frame sizes.

I consider these to be related as clone-frame's use of make-frame could be
using text-pixels but if that doesn't work then pixelwise cloning won't
work. I did read through the code base as best as I could but could not
find the source of the text-pixels issue.

The following reproducer, under -Q, shows the same results on 29.4 and
30.0.92. My main platform is NS and I also did some testing on GTK. GTK's
issues seem a bit "messier" and I didn't spend any time trying to
understand them in depth as I was more interested to know if GTK worked
correctly or not, which it doesn't.

(switch-to-buffer "*Messages*")
(let ((target-text-width 1700)
      (target-text-height 1000)
      (native-width)
      (native-height)
      (msg (lambda (s frame)
             (message "%s text-width=%d (Δ%d) text-height=%d (Δ%d)
native-width=%d (Δ%d) native-height %d (Δ%d)\n"
                      s
                      (frame-text-width frame) (- (frame-text-width frame)
target-text-width)
                      (frame-text-height frame) (- (frame-text-height
frame) target-text-height)
                      (frame-native-width frame) (- (frame-native-width
frame) native-width)
                      (frame-native-height frame) (- (frame-native-height
frame) native-height)))))
  (set-frame-position nil 0 0)
  (set-frame-size nil target-text-width target-text-height 'pixelwise)
  (setq native-width (frame-native-width)
        native-height (frame-native-height))
  (message "Targets: text-width=%d text-height=%d\n" target-text-width
target-text-height)
  (funcall msg "orig" (selected-frame))

  (message "clone-frame under frame-resize-pixelwise nil; expectation: use
lines/columns geometry; outcome: met")
  (let ((frame-resize-pixelwise nil))
    (let ((new-frame (clone-frame)))
      (funcall msg "new" new-frame)
      (delete-frame new-frame)))

  (message "clone-frame under frame-resize-pixelwise t; expectation:
pixelwise geometry; outcome: unmet")
  (let ((frame-resize-pixelwise t))
    (let ((new-frame (clone-frame)))
      (funcall msg "new" new-frame)
      (delete-frame new-frame)))

  (message "clone-frame followed by manual resize; expectation: pixelwise
geometry; outcome: met (but two steps)")
  (let ((new-frame (clone-frame)))
    (set-frame-size new-frame target-text-width target-text-height
'pixelwise)
    (funcall msg "new" new-frame)
    (delete-frame new-frame))

  (message "manual clone under frame-resize-pixelwise using text-pixels;
expectation: pixelwise geometry; outcome: unmet")
  ;; code lifted from clone-frame
  ;; incorrect width offset seems to be equal to frame-scroll-bar-width
  (let* ((frame-resize-pixelwise t)
         (frame (selected-frame))
         (no-windows nil)
         (windows (unless no-windows
                    (window-state-get (frame-root-window frame))))
         (default-frame-alist
          (seq-remove (lambda (elem)
                        (memq (car elem) frame-internal-parameters))
                      (frame-parameters frame)))
         (new-frame))
    (when (and (display-graphic-p frame) frame-resize-pixelwise)
      (push (cons 'width (cons 'text-pixels (frame-text-width frame)))
default-frame-alist)
      (push (cons 'height (cons 'text-pixels (frame-text-height frame)))
default-frame-alist))
    (setq new-frame (make-frame))
    (when windows
      (window-state-put windows (frame-root-window new-frame) 'safe))
    (unless (display-graphic-p frame)
      (select-frame new-frame))
    (funcall msg "new" new-frame)
    (delete-frame new-frame)))

This is an implementation of clone-frame that uses text-pixels under
make-frame. This depends on make-frame text-pixels being corrected. Happy
to supply this as a patch should the discussion of these issues progress in
that direction.

(defun clone-frame (&optional frame no-windows pixelwise)
  "Make a new frame with the same parameters and windows as FRAME.
With a prefix arg NO-WINDOWS, don't clone the window configuration.  When
PIXELWISE is non-nil or if `frame-resize-pixelwise' is non-nil, and frame
is not text-only, clone the originating frame's pixel size.

FRAME defaults to the selected frame.  The frame is created on the
same terminal as FRAME.  If the terminal is a text-only terminal then
also select the new frame."
  (interactive (list (selected-frame) current-prefix-arg))
  (let* ((frame (or frame (selected-frame)))
         (windows (unless no-windows
                    (window-state-get (frame-root-window frame))))
         (default-frame-alist
          (seq-remove (lambda (elem)
                        (memq (car elem) frame-internal-parameters))
                      (frame-parameters frame)))
         (new-frame))
    (when (and (display-graphic-p frame)
               (or pixelwise frame-resize-pixelwise))
      (push (cons 'width (cons 'text-pixels (frame-text-width frame)))
            default-frame-alist)
      (push (cons 'height (cons 'text-pixels (frame-text-height frame)))
            default-frame-alist))
    (setq new-frame (make-frame))
    (when windows
      (window-state-put windows (frame-root-window new-frame) 'safe))
    (unless (display-graphic-p frame)
      (select-frame new-frame))
    new-frame))

-Stephane

[-- Attachment #2: Type: text/html, Size: 6564 bytes --]

bug#74750: clone-frame and make-frame pixelwise issues

[Eli Zaretskii]

> From: Ship Mints <shipmints@gmail.com>
> Date: Mon, 9 Dec 2024 10:51:23 -0500
> 
> While trying to reconcile pixelwise frame sizing behaviors, I narrowed down
> two related issues.
> 
> clone-frame does not correctly clone frames on a pixelwise basis.
> 
> make-frame's text-pixels geometry support does not produce specified
> pixelwise geometry. This also impacts frameset-restore's ability to
> precisely reproduce pixelwise frame sizes.
> 
> I consider these to be related as clone-frame's use of make-frame could be
> using text-pixels but if that doesn't work then pixelwise cloning won't
> work. I did read through the code base as best as I could but could not
> find the source of the text-pixels issue.
> 
> The following reproducer, under -Q, shows the same results on 29.4 and
> 30.0.92. My main platform is NS and I also did some testing on GTK. GTK's
> issues seem a bit "messier" and I didn't spend any time trying to
> understand them in depth as I was more interested to know if GTK worked
> correctly or not, which it doesn't.
> 
> (switch-to-buffer "*Messages*")
> (let ((target-text-width 1700)
>       (target-text-height 1000)
>       (native-width)
>       (native-height)
>       (msg (lambda (s frame)
>              (message "%s text-width=%d (Δ%d) text-height=%d (Δ%d)
> native-width=%d (Δ%d) native-height %d (Δ%d)\n"
>                       s
>                       (frame-text-width frame) (- (frame-text-width frame)
> target-text-width)
>                       (frame-text-height frame) (- (frame-text-height
> frame) target-text-height)
>                       (frame-native-width frame) (- (frame-native-width
> frame) native-width)
>                       (frame-native-height frame) (- (frame-native-height
> frame) native-height)))))
>   (set-frame-position nil 0 0)
>   (set-frame-size nil target-text-width target-text-height 'pixelwise)
>   (setq native-width (frame-native-width)
>         native-height (frame-native-height))
>   (message "Targets: text-width=%d text-height=%d\n" target-text-width
> target-text-height)
>   (funcall msg "orig" (selected-frame))
> 
>   (message "clone-frame under frame-resize-pixelwise nil; expectation: use
> lines/columns geometry; outcome: met")
>   (let ((frame-resize-pixelwise nil))
>     (let ((new-frame (clone-frame)))
>       (funcall msg "new" new-frame)
>       (delete-frame new-frame)))
> 
>   (message "clone-frame under frame-resize-pixelwise t; expectation:
> pixelwise geometry; outcome: unmet")
>   (let ((frame-resize-pixelwise t))
>     (let ((new-frame (clone-frame)))
>       (funcall msg "new" new-frame)
>       (delete-frame new-frame)))
> 
>   (message "clone-frame followed by manual resize; expectation: pixelwise
> geometry; outcome: met (but two steps)")
>   (let ((new-frame (clone-frame)))
>     (set-frame-size new-frame target-text-width target-text-height
> 'pixelwise)
>     (funcall msg "new" new-frame)
>     (delete-frame new-frame))
> 
>   (message "manual clone under frame-resize-pixelwise using text-pixels;
> expectation: pixelwise geometry; outcome: unmet")
>   ;; code lifted from clone-frame
>   ;; incorrect width offset seems to be equal to frame-scroll-bar-width
>   (let* ((frame-resize-pixelwise t)
>          (frame (selected-frame))
>          (no-windows nil)
>          (windows (unless no-windows
>                     (window-state-get (frame-root-window frame))))
>          (default-frame-alist
>           (seq-remove (lambda (elem)
>                         (memq (car elem) frame-internal-parameters))
>                       (frame-parameters frame)))
>          (new-frame))
>     (when (and (display-graphic-p frame) frame-resize-pixelwise)
>       (push (cons 'width (cons 'text-pixels (frame-text-width frame)))
> default-frame-alist)
>       (push (cons 'height (cons 'text-pixels (frame-text-height frame)))
> default-frame-alist))
>     (setq new-frame (make-frame))
>     (when windows
>       (window-state-put windows (frame-root-window new-frame) 'safe))
>     (unless (display-graphic-p frame)
>       (select-frame new-frame))
>     (funcall msg "new" new-frame)
>     (delete-frame new-frame)))
> 
> This is an implementation of clone-frame that uses text-pixels under
> make-frame. This depends on make-frame text-pixels being corrected. Happy
> to supply this as a patch should the discussion of these issues progress in
> that direction.
> 
> (defun clone-frame (&optional frame no-windows pixelwise)
>   "Make a new frame with the same parameters and windows as FRAME.
> With a prefix arg NO-WINDOWS, don't clone the window configuration.  When
> PIXELWISE is non-nil or if `frame-resize-pixelwise' is non-nil, and frame
> is not text-only, clone the originating frame's pixel size.
> 
> FRAME defaults to the selected frame.  The frame is created on the
> same terminal as FRAME.  If the terminal is a text-only terminal then
> also select the new frame."
>   (interactive (list (selected-frame) current-prefix-arg))
>   (let* ((frame (or frame (selected-frame)))
>          (windows (unless no-windows
>                     (window-state-get (frame-root-window frame))))
>          (default-frame-alist
>           (seq-remove (lambda (elem)
>                         (memq (car elem) frame-internal-parameters))
>                       (frame-parameters frame)))
>          (new-frame))
>     (when (and (display-graphic-p frame)
>                (or pixelwise frame-resize-pixelwise))
>       (push (cons 'width (cons 'text-pixels (frame-text-width frame)))
>             default-frame-alist)
>       (push (cons 'height (cons 'text-pixels (frame-text-height frame)))
>             default-frame-alist))
>     (setq new-frame (make-frame))
>     (when windows
>       (window-state-put windows (frame-root-window new-frame) 'safe))
>     (unless (display-graphic-p frame)
>       (select-frame new-frame))
>     new-frame))

Thanks.  I hope Martin (CC'ed) will have some useful inputs.

bug#74750: clone-frame and make-frame pixelwise issues

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 >> clone-frame does not correctly clone frames on a pixelwise basis.
 >>
 >> make-frame's text-pixels geometry support does not produce specified
 >> pixelwise geometry. This also impacts frameset-restore's ability to
 >> precisely reproduce pixelwise frame sizes.

This would be a bug in frameset.el.  One problem I see is that
'frame-resize-pixelwise' must be set "very early" when restoring a
session - at least _before_ the first time we send size hints to the
window manager.

 >> I consider these to be related as clone-frame's use of make-frame could be
 >> using text-pixels but if that doesn't work then pixelwise cloning won't
 >> work. I did read through the code base as best as I could but could not
 >> find the source of the text-pixels issue.
 >>
 >> The following reproducer, under -Q, shows the same results on 29.4 and
 >> 30.0.92. My main platform is NS and I also did some testing on GTK. GTK's
 >> issues seem a bit "messier" and I didn't spend any time trying to
 >> understand them in depth as I was more interested to know if GTK worked
 >> correctly or not, which it doesn't.

Your code binds 'frame-resize-pixelwise' temporarily.  This cannot work
reliably.  That variable should never change in an Emacs session because
its value affects the way we send size hint increments to the window
manager.

This is, admittedly, a design error that would have to be fixed as
follows:

- Implement a new frame parameter 'resize-pixelwise'.

- Send size hints according to the value of this parameter.  When the
   parameter is set, new size hints must be sent.

Alternatively, we could send new size hints for all live frames whenever
'frame-resize-pixelwise' is changed.  This would have to be done with a
variable watcher.  Still, let-binding this variable would confuse the
hell out of our interactions with the window manager.  When the scope of
the let-binding is left, we would have to send size hints again.

 >> This is an implementation of clone-frame that uses text-pixels under
 >> make-frame. This depends on make-frame text-pixels being corrected. Happy
 >> to supply this as a patch should the discussion of these issues progress in
 >> that direction.
 >>
 >> (defun clone-frame (&optional frame no-windows pixelwise)

What would the WM do in a situation where PIXELWISE is non-nil and
'frame-resize-pixelwise' is nil?

martin

bug#74750: clone-frame and make-frame pixelwise issues

[Ship Mints]

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

On Tue, Dec 10, 2024 at 10:56 AM martin rudalics <rudalics@gmx.at> wrote:

>  >> clone-frame does not correctly clone frames on a pixelwise basis.
>  >>
>  >> make-frame's text-pixels geometry support does not produce specified
>  >> pixelwise geometry. This also impacts frameset-restore's ability to
>  >> precisely reproduce pixelwise frame sizes.
>
> This would be a bug in frameset.el.  One problem I see is that
> 'frame-resize-pixelwise' must be set "very early" when restoring a
> session - at least _before_ the first time we send size hints to the
> window manager.
>

The goal is, I think, to achieve pixelwise harmony among clone, make, and
restore. FWIW, my GUI sessions always run with frame-resize-pixelwise set
to t and see these sizing issues consistently. My reproducer is intended to
engender the conversation as an illustration. I have resorted to manual
labor to set frame sizes in various places and including squirreling away
frame pixel size in frameset-save just to manually reset it after
restoration due to text-pixel reliability issues. Somewhere in the bowels
of frame.c, et.al., is an issue where text-pixels behaves differently than
set-frame-size which is reliable on NS where make-frame using text-pixels
is incorrect by the vertical scroll bar width. GTK it's off, too, but I
didn't analyze that further.


>  >> I consider these to be related as clone-frame's use of make-frame
> could be
>  >> using text-pixels but if that doesn't work then pixelwise cloning won't
>  >> work. I did read through the code base as best as I could but could not
>  >> find the source of the text-pixels issue.
>  >>
>  >> The following reproducer, under -Q, shows the same results on 29.4 and
>  >> 30.0.92. My main platform is NS and I also did some testing on GTK.
> GTK's
>  >> issues seem a bit "messier" and I didn't spend any time trying to
>  >> understand them in depth as I was more interested to know if GTK worked
>  >> correctly or not, which it doesn't.
>
> Your code binds 'frame-resize-pixelwise' temporarily.  This cannot work
> reliably.  That variable should never change in an Emacs session because
> its value affects the way we send size hint increments to the window
> manager.
>

Again, this is just a reproducer for discussion and is intended to
illustrate the issues, not to specifically discuss user habits for
frame-resize-pixelwise across a GUI session. I think most people these days
probably do have it set to t, even if it's become reflexive.

This is, admittedly, a design error that would have to be fixed as
> follows:
>
> - Implement a new frame parameter 'resize-pixelwise'.
>

I certainly prefer something more explicit such as this.

- Send size hints according to the value of this parameter.  When the
>    parameter is set, new size hints must be sent.
>
> Alternatively, we could send new size hints for all live frames whenever
> 'frame-resize-pixelwise' is changed.  This would have to be done with a
> variable watcher.  Still, let-binding this variable would confuse the
> hell out of our interactions with the window manager.  When the scope of
> the let-binding is left, we would have to send size hints again.
>

No, please.

 >> This is an implementation of clone-frame that uses text-pixels under
>  >> make-frame. This depends on make-frame text-pixels being corrected.
> Happy
>  >> to supply this as a patch should the discussion of these issues
> progress in
>  >> that direction.
>  >>
>  >> (defun clone-frame (&optional frame no-windows pixelwise)
>
> What would the WM do in a situation where PIXELWISE is non-nil and
> 'frame-resize-pixelwise' is nil?
>

The simple interim code (vs. the resize-pixelwise proposal) respects
frame-resize-pixelwise as the user's preference. Same with being explicit
by saying 'pixelwise, it's user intention.

    (when (and (display-graphic-p frame)
               (or pixelwise frame-resize-pixelwise))

FWIW, there are surely weird WM issues but at the very least, we're talking
about the inner geometry text-width and text-height that Emacs controls and
I stayed away from external geometry to start off at least seeing if we can
correct for what Emacs controls completely. GTK is definitely worse than NS
but at least in NS every user has the same experience as the WM options and
behaviors are more constrained than X11-derived kind of free-for-all WMs.

martin
>

[-- Attachment #2: Type: text/html, Size: 6900 bytes --]

bug#74750: clone-frame and make-frame pixelwise issues

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

 > Somewhere in the bowels
 > of frame.c, et.al., is an issue where text-pixels behaves differently than
 > set-frame-size which is reliable on NS where make-frame using text-pixels
 > is incorrect by the vertical scroll bar width. GTK it's off, too, but I
 > didn't analyze that further.

If you mean that with

(progn
   (setq frame-resize-pixelwise t)
   (make-frame '((width . (text-pixels . 800)))))

then

(frame-text-width)

evaluates to 784, please try the attached patch.

Thanks, martin

[-- Attachment #2: make-frame.diff --]
[-- Type: text/x-patch, Size: 671 bytes --]

diff --git a/src/frame.c b/src/frame.c
index f6053fca3ef..4c21ce5b51b 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -168,8 +168,10 @@ frame_inhibit_resize (struct frame *f, bool horizontal, Lisp_Object parameter)
 	  || (!horizontal
 	      && !NILP (fullscreen) && !EQ (fullscreen, Qfullwidth))
 	  || FRAME_TERMCAP_P (f) || FRAME_MSDOS_P (f))
-       : ((horizontal && f->inhibit_horizontal_resize)
-	  || (!horizontal && f->inhibit_vertical_resize)));
+     : ((horizontal && f->inhibit_horizontal_resize
+	 && !EQ (parameter, Qscroll_bar_width))
+	|| (!horizontal && f->inhibit_vertical_resize
+	    && !EQ (parameter, Qscroll_bar_height))));
 
   return inhibit;
 }