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;
 }

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

[Ship Mints]

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

That patch works to address make-frame's respect for text-pixels, at least
on NS (the only platform I tested).

Now to address the clone-frame implementation to respect pixelwise, and/or
as you suggested, perhaps a formal frame parameter resize-pixelwise. If we
adopt the change I proposed it could be used in Emacs 30 or 31 since this
is a bug (but not a regression). If we go for resize-pixelwise, it'll be
31, right?

On Wed, Dec 11, 2024 at 4:38 AM martin rudalics <rudalics@gmx.at> wrote:

>  > 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: Type: text/html, Size: 1675 bytes --]

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

[Eli Zaretskii]

> From: Ship Mints <shipmints@gmail.com>
> Date: Wed, 11 Dec 2024 17:41:01 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, 74750@debbugs.gnu.org
> 
> That patch works to address make-frame's respect for text-pixels, at least on NS (the only platform I tested).
> 
> Now to address the clone-frame implementation to respect pixelwise, and/or as you suggested, perhaps a
> formal frame parameter resize-pixelwise. If we adopt the change I proposed it could be used in Emacs 30 or
> 31 since this is a bug (but not a regression). If we go for resize-pixelwise, it'll be 31, right?

Actually, it's too late for Emacs 30, unless the bug is very serious
and the fix is very safe.  So I think this should end up on master
regardless.

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

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

 > That patch works to address make-frame's respect for text-pixels, at least
 > on NS (the only platform I tested).

I don't like it much and I have to further test the behavior with fringes.
Maybe I find  better solution.

 > Now to address the clone-frame implementation to respect pixelwise, and/or
 > as you suggested, perhaps a formal frame parameter resize-pixelwise. If we
 > adopt the change I proposed it could be used in Emacs 30 or 31 since this
 > is a bug (but not a regression). If we go for resize-pixelwise, it'll be
 > 31, right?

Everything we do here would have to go into Emacs 31.

martin

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: 420 bytes --]

 >  > That patch works to address make-frame's respect for text-pixels, at least
 >  > on NS (the only platform I tested).
 >
 > I don't like it much and I have to further test the behavior with fringes.
 > Maybe I find  better solution.

I attach a patch that does away with a frame's inhibit_horizontal_resize
and inhibit_vertical_resize slots.  Please test it.  If it works for
you, I'll install it on master.

martin

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

diff --git a/src/frame.c b/src/frame.c
index f6053fca3ef..2cf5fefa77c 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -158,20 +158,15 @@ get_frame_param (struct frame *frame, Lisp_Object prop)
 frame_inhibit_resize (struct frame *f, bool horizontal, Lisp_Object parameter)
 {
   Lisp_Object fullscreen = get_frame_param (f, Qfullscreen);
-  bool inhibit
-    = (f->after_make_frame
-       ? (EQ (frame_inhibit_implied_resize, Qt)
+
+  return (EQ (frame_inhibit_implied_resize, Qt)
 	  || (CONSP (frame_inhibit_implied_resize)
 	      && !NILP (Fmemq (parameter, frame_inhibit_implied_resize)))
 	  || (horizontal
 	      && !NILP (fullscreen) && !EQ (fullscreen, Qfullheight))
 	  || (!horizontal
 	      && !NILP (fullscreen) && !EQ (fullscreen, Qfullwidth))
-	  || FRAME_TERMCAP_P (f) || FRAME_MSDOS_P (f))
-       : ((horizontal && f->inhibit_horizontal_resize)
-	  || (!horizontal && f->inhibit_vertical_resize)));
-
-  return inhibit;
+	  || FRAME_TERMCAP_P (f) || FRAME_MSDOS_P (f));
 }
 
 
@@ -957,8 +952,6 @@ make_frame (bool mini_p)
   f->garbaged = true;
   f->can_set_window_size = false;
   f->after_make_frame = false;
-  f->inhibit_horizontal_resize = false;
-  f->inhibit_vertical_resize = false;
   f->tab_bar_redisplayed = false;
   f->tab_bar_resized = false;
   f->tool_bar_redisplayed = false;
@@ -3128,8 +3121,6 @@ DEFUN ("frame-after-make-frame",
 {
   struct frame *f = decode_live_frame (frame);
   f->after_make_frame = !NILP (made);
-  f->inhibit_horizontal_resize = false;
-  f->inhibit_vertical_resize = false;
   return made;
 }
 
@@ -5918,7 +5909,6 @@ gui_figure_window_size (struct frame *f, Lisp_Object parms, bool tabbar_p,
 		xsignal1 (Qargs_out_of_range, XCDR (width));
 
 	      text_width = XFIXNUM (XCDR (width));
-	      f->inhibit_horizontal_resize = true;
 	    }
 	  else if (FLOATP (width))
 	    {
@@ -5954,7 +5944,6 @@ gui_figure_window_size (struct frame *f, Lisp_Object parms, bool tabbar_p,
 		xsignal1 (Qargs_out_of_range, XCDR (height));
 
 	      text_height = XFIXNUM (XCDR (height));
-	      f->inhibit_vertical_resize = true;
 	    }
 	  else if (FLOATP (height))
 	    {
diff --git a/src/frame.h b/src/frame.h
index 1d920d1a6bc..172eb5eca99 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -519,10 +519,6 @@ #define EMACS_FRAME_H
   bool_bf tool_bar_redisplayed : 1;
   bool_bf tool_bar_resized : 1;
 
-  /* Inhibit implied resize before after_make_frame is set.  */
-  bool_bf inhibit_horizontal_resize : 1;
-  bool_bf inhibit_vertical_resize : 1;
-
   /* Non-zero if this frame's faces need to be recomputed.  */
   bool_bf face_change : 1;
 

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

[Ship Mints]

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

Thank you for the patch and continued focus on this. The following is the
behavior I see on NS with the patch applied with target width 1700 and
height 1000. These are results from clone-frame using text-pixels which
I've included below without explicit pixelwise argument so it respects
frame-resize-pixelwise.

Note: frame-inhibit-implied-resize=(tab-bar-lines) is the default setting
on NS:

#if defined (USE_GTK) || defined (HAVE_NS)
  frame_inhibit_implied_resize = list1 (Qtab_bar_lines);

frame-resize-pixelwise=nil frame-inhibit-implied-resize=nil text-width=1692
(Δ-8) text-height=984 (Δ-16) native-width=1727 (Δ-8) native-height 988
(Δ-16)

Result: respects lines/cols as expected.

frame-resize-pixelwise=t frame-inhibit-implied-resize=(tab-bar-lines)
text-width=1700 (Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0)
native-height 1004 (Δ0)

Result: Okay by accident, I think, only because tab-bar-lines parameter is
nil during adjust_frame_height invocations?

frame-resize-pixelwise=t frame-inhibit-implied-resize=nil text-width=1700
(Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0) native-height 1004 (Δ0)

Result: Okay but with frame-inhibit-implied-resize nil, I'd have expected
rows/cols vs. pixelwise.

frame-resize-pixelwise=t frame-inhibit-implied-resize=t text-width=1685
(Δ-15) text-height=1000 (Δ0) native-width=1720 (Δ-15) native-height 1004
(Δ0)

Result: I think this case remains broken needing the adjustment from your
first patch that you wanted to also account for fringes?

My default GUI setup is frame-resize-pixelwise t
frame-inhibit-implied-resize t as I expect many people have adopted these
days.

(defun my/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))

On Fri, Dec 13, 2024 at 5:30 AM martin rudalics <rudalics@gmx.at> wrote:

>  >  > That patch works to address make-frame's respect for text-pixels, at
> least
>  >  > on NS (the only platform I tested).
>  >
>  > I don't like it much and I have to further test the behavior with
> fringes.
>  > Maybe I find  better solution.
>
> I attach a patch that does away with a frame's inhibit_horizontal_resize
> and inhibit_vertical_resize slots.  Please test it.  If it works for
> you, I'll install it on master.
>
> martin

[-- Attachment #2: Type: text/html, Size: 5404 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]

 > Thank you for the patch and continued focus on this. The following is the
 > behavior I see on NS with the patch applied with target width 1700 and
 > height 1000. These are results from clone-frame using text-pixels which
 > I've included below without explicit pixelwise argument so it respects
 > frame-resize-pixelwise.

Thank you for conducting these experiments.

 > Note: frame-inhibit-implied-resize=(tab-bar-lines) is the default setting
 > on NS:
 >
 > #if defined (USE_GTK) || defined (HAVE_NS)
 >    frame_inhibit_implied_resize = list1 (Qtab_bar_lines);
 >
 > frame-resize-pixelwise=nil frame-inhibit-implied-resize=nil text-width=1692
 > (Δ-8) text-height=984 (Δ-16) native-width=1727 (Δ-8) native-height 988
 > (Δ-16)
 >
 > Result: respects lines/cols as expected.

It's problematic to clone a frame made with 'frame-resize-pixelwise'
non-nil in a setting with 'frame-resize-pixelwise' nil.  I always have
'frame-resize-pixelwise' t and never change it.

 > frame-resize-pixelwise=t frame-inhibit-implied-resize=(tab-bar-lines)
 > text-width=1700 (Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0)
 > native-height 1004 (Δ0)
 >
 > Result: Okay by accident, I think, only because tab-bar-lines parameter is
 > nil during adjust_frame_height invocations?

adjust_frame_size you mean, I suppose.  Does your frame have a tab bar?

 > frame-resize-pixelwise=t frame-inhibit-implied-resize=nil text-width=1700
 > (Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0) native-height 1004 (Δ0)
 >
 > Result: Okay but with frame-inhibit-implied-resize nil, I'd have expected
 > rows/cols vs. pixelwise.

Why?  'frame-inhibit-implied-resize' is about _not_ resizing a frame's
window when one removes/adds one of the items it mentions.  It should
work with pixelwise and normal resizing.

 > frame-resize-pixelwise=t frame-inhibit-implied-resize=t text-width=1685
 > (Δ-15) text-height=1000 (Δ0) native-width=1720 (Δ-15) native-height 1004
 > (Δ0)
 >
 > Result: I think this case remains broken needing the adjustment from your
 > first patch that you wanted to also account for fringes?

15 is an odd number so it can't be the default fringes.  IIUC it's your
scroll bar which gets set up after the "frame was made" and while
resizing is inhibited.  Note that the fringes are purely Emacs internal
- we can set them up any way we like.  The scroll bar is more difficult
since the toolkit usually determines its default width.  You could try
to debug this with a breakpoint in 'gui_set_scroll_bar_width' and after
that one in 'frame_inhibit_resize'.  Here on xfwm/GTK-3 the text width
remains unchanged.

 > My default GUI setup is frame-resize-pixelwise t
 > frame-inhibit-implied-resize t as I expect many people have adopted these
 > days.

Few people have AFAICT.  Note that all these experiments are borderline.
Cloning a frame should respect the settings that were active at the time
the original was made.  We can try to make it behave reasonably when
these values change but I am not sure whether we will succeed.

martin

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

[Ship Mints]

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

On Fri, Dec 13, 2024 at 1:15 PM martin rudalics <rudalics@gmx.at> wrote:

>  > Thank you for the patch and continued focus on this. The following is
> the
>  > behavior I see on NS with the patch applied with target width 1700 and
>  > height 1000. These are results from clone-frame using text-pixels which
>  > I've included below without explicit pixelwise argument so it respects
>  > frame-resize-pixelwise.
>
> Thank you for conducting these experiments.
>

My pleasure, actually. I live inside Emacs so the better we make it for us
the better for all.

 > Note: frame-inhibit-implied-resize=(tab-bar-lines) is the default setting
>  > on NS:
>  >
>  > #if defined (USE_GTK) || defined (HAVE_NS)
>  >    frame_inhibit_implied_resize = list1 (Qtab_bar_lines);
>  >
>  > frame-resize-pixelwise=nil frame-inhibit-implied-resize=nil
> text-width=1692
>  > (Δ-8) text-height=984 (Δ-16) native-width=1727 (Δ-8) native-height 988
>  > (Δ-16)
>  >
>  > Result: respects lines/cols as expected.
>
> It's problematic to clone a frame made with 'frame-resize-pixelwise'
> non-nil in a setting with 'frame-resize-pixelwise' nil.  I always have
> 'frame-resize-pixelwise' t and never change it.
>

As you pointed out, these are experiments and reflect the desire to fully
understand (and control) Emacs behavior under various circumstances we
encounter.


>  > frame-resize-pixelwise=t frame-inhibit-implied-resize=(tab-bar-lines)
>  > text-width=1700 (Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0)
>  > native-height 1004 (Δ0)
>  >
>  > Result: Okay by accident, I think, only because tab-bar-lines parameter
> is
>  > nil during adjust_frame_height invocations?
>
> adjust_frame_size you mean, I suppose.  Does your frame have a tab bar?
>

Yes, typo. These results are all under -Q as we need to repro, so no
visible tab bar, just the default NS view which is the tool bar which under
master, now appears on the title bar. Something I didn't notice until today
but it's neither here nor there, I suppose. I disable tool-bar under all my
own real-world circumstances.

 > frame-resize-pixelwise=t frame-inhibit-implied-resize=nil text-width=1700
>  > (Δ0) text-height=1000 (Δ0) native-width=1735 (Δ0) native-height 1004
> (Δ0)
>  >
>  > Result: Okay but with frame-inhibit-implied-resize nil, I'd have
> expected
>  > rows/cols vs. pixelwise.
>
> Why?  'frame-inhibit-implied-resize' is about _not_ resizing a frame's
> window when one removes/adds one of the items it mentions.  It should
> work with pixelwise and normal resizing.
>

I said that because the latest patch respects frame-inhibit-implied-resize
not frame-resize-pixelwise.

 > frame-resize-pixelwise=t frame-inhibit-implied-resize=t text-width=1685
>  > (Δ-15) text-height=1000 (Δ0) native-width=1720 (Δ-15) native-height 1004
>  > (Δ0)
>  >
>  > Result: I think this case remains broken needing the adjustment from
> your
>  > first patch that you wanted to also account for fringes?
>
> 15 is an odd number so it can't be the default fringes.  IIUC it's your
> scroll bar which gets set up after the "frame was made" and while
> resizing is inhibited.  Note that the fringes are purely Emacs internal
> - we can set them up any way we like.  The scroll bar is more difficult
> since the toolkit usually determines its default width.  You could try
> to debug this with a breakpoint in 'gui_set_scroll_bar_width' and after
> that one in 'frame_inhibit_resize'.  Here on xfwm/GTK-3 the text width
> remains unchanged.
>

Indeed 15 is the vertical scroll bar width. This was what I reported in the
original bug submission. You suggested a patch that would accommodate
fringes, et.al. If you'd like me to make adjustments; e.g., resizing
fringes or whatever, happy to do it and rerun.

These are all ostensively calls to clone-frame. I'd expect, as I guess most
people would, that cloning produces the precise geometry of the originating
frame, scroll bar or not.


>  > My default GUI setup is frame-resize-pixelwise t
>  > frame-inhibit-implied-resize t as I expect many people have adopted
> these
>  > days.
>
> Few people have AFAICT.  Note that all these experiments are borderline.
> Cloning a frame should respect the settings that were active at the time
> the original was made.  We can try to make it behave reasonably when
> these values change but I am not sure whether we will succeed.
>

Let's try.

-Stephane

[-- Attachment #2: Type: text/html, Size: 6892 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: 1149 bytes --]

 > Indeed 15 is the vertical scroll bar width. This was what I reported in the
 > original bug submission. You suggested a patch that would accommodate
 > fringes, et.al. If you'd like me to make adjustments; e.g., resizing
 > fringes or whatever, happy to do it and rerun.

I've been throwing out the child with the bathwater.  Please try the
attached patch which retains an important conjunct.

 > These are all ostensively calls to clone-frame. I'd expect, as I guess most
 > people would, that cloning produces the precise geometry of the originating
 > frame, scroll bar or not.

The major purpose of 'frame-inhibit-implied-resize' is to avoid resizes
when a frame has been tailored to fit into some arrangement of windows
on the display as, for example, with a tiling window manager.  Here I
hardly ever use it.  By design, it should have no effect when making a
new frame which is what the corrected patch should support.  Still, it
might not work for elements like the external tool bar.

 >> We can try to make it behave reasonably when
 >> these values change but I am not sure whether we will succeed.
 >>
 >
 > Let's try.

Let's.

martin

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

diff --git a/src/frame.c b/src/frame.c
index f6053fca3ef..f22bd501a8d 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -158,20 +158,16 @@ get_frame_param (struct frame *frame, Lisp_Object prop)
 frame_inhibit_resize (struct frame *f, bool horizontal, Lisp_Object parameter)
 {
   Lisp_Object fullscreen = get_frame_param (f, Qfullscreen);
-  bool inhibit
-    = (f->after_make_frame
-       ? (EQ (frame_inhibit_implied_resize, Qt)
-	  || (CONSP (frame_inhibit_implied_resize)
-	      && !NILP (Fmemq (parameter, frame_inhibit_implied_resize)))
-	  || (horizontal
-	      && !NILP (fullscreen) && !EQ (fullscreen, Qfullheight))
-	  || (!horizontal
-	      && !NILP (fullscreen) && !EQ (fullscreen, Qfullwidth))
-	  || FRAME_TERMCAP_P (f) || FRAME_MSDOS_P (f))
-       : ((horizontal && f->inhibit_horizontal_resize)
-	  || (!horizontal && f->inhibit_vertical_resize)));
 
-  return inhibit;
+  return (f->after_make_frame
+	  && (EQ (frame_inhibit_implied_resize, Qt)
+	      || (CONSP (frame_inhibit_implied_resize)
+		  && !NILP (Fmemq (parameter, frame_inhibit_implied_resize)))
+	      || (horizontal
+		  && !NILP (fullscreen) && !EQ (fullscreen, Qfullheight))
+	      || (!horizontal
+		  && !NILP (fullscreen) && !EQ (fullscreen, Qfullwidth))
+	      || FRAME_TERMCAP_P (f) || FRAME_MSDOS_P (f)));
 }
 
 
@@ -957,8 +953,6 @@ make_frame (bool mini_p)
   f->garbaged = true;
   f->can_set_window_size = false;
   f->after_make_frame = false;
-  f->inhibit_horizontal_resize = false;
-  f->inhibit_vertical_resize = false;
   f->tab_bar_redisplayed = false;
   f->tab_bar_resized = false;
   f->tool_bar_redisplayed = false;
@@ -3128,8 +3122,6 @@ DEFUN ("frame-after-make-frame",
 {
   struct frame *f = decode_live_frame (frame);
   f->after_make_frame = !NILP (made);
-  f->inhibit_horizontal_resize = false;
-  f->inhibit_vertical_resize = false;
   return made;
 }
 
@@ -5918,7 +5910,6 @@ gui_figure_window_size (struct frame *f, Lisp_Object parms, bool tabbar_p,
 		xsignal1 (Qargs_out_of_range, XCDR (width));
 
 	      text_width = XFIXNUM (XCDR (width));
-	      f->inhibit_horizontal_resize = true;
 	    }
 	  else if (FLOATP (width))
 	    {
@@ -5954,7 +5945,6 @@ gui_figure_window_size (struct frame *f, Lisp_Object parms, bool tabbar_p,
 		xsignal1 (Qargs_out_of_range, XCDR (height));
 
 	      text_height = XFIXNUM (XCDR (height));
-	      f->inhibit_vertical_resize = true;
 	    }
 	  else if (FLOATP (height))
 	    {
diff --git a/src/frame.h b/src/frame.h
index 1d920d1a6bc..172eb5eca99 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -519,10 +519,6 @@ #define EMACS_FRAME_H
   bool_bf tool_bar_redisplayed : 1;
   bool_bf tool_bar_resized : 1;
 
-  /* Inhibit implied resize before after_make_frame is set.  */
-  bool_bf inhibit_horizontal_resize : 1;
-  bool_bf inhibit_vertical_resize : 1;
-
   /* Non-zero if this frame's faces need to be recomputed.  */
   bool_bf face_change : 1;
 

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

[Ship Mints]

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

That patch seems to work. Thank you, Martin. I tested on NS with and
without vertical scroll bars and with and without fringes. A basic test of
frameset-save and frameset-restore now seem to correctly respect its
embedded text-pixel geometry.

Now, about clone-frame. Are there any objections to the below
implementation that uses text-pixels?

(defun my/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)))
         (frame-resize-pixelwise frame-resize-pixelwise)
         (new-frame))
    (when (and (display-graphic-p frame)
               (or pixelwise frame-resize-pixelwise))
      (setq frame-resize-pixelwise t)
      (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))

I may be able to test on GTK early this week, but I think you have GNU
Linux/GTK on your end?

-Stephane

On Sat, Dec 14, 2024 at 3:27 AM martin rudalics <rudalics@gmx.at> wrote:

>  > Indeed 15 is the vertical scroll bar width. This was what I reported in
> the
>  > original bug submission. You suggested a patch that would accommodate
>  > fringes, et.al. If you'd like me to make adjustments; e.g., resizing
>  > fringes or whatever, happy to do it and rerun.
>
> I've been throwing out the child with the bathwater.  Please try the
> attached patch which retains an important conjunct.
>
>  > These are all ostensively calls to clone-frame. I'd expect, as I guess
> most
>  > people would, that cloning produces the precise geometry of the
> originating
>  > frame, scroll bar or not.
>
> The major purpose of 'frame-inhibit-implied-resize' is to avoid resizes
> when a frame has been tailored to fit into some arrangement of windows
> on the display as, for example, with a tiling window manager.  Here I
> hardly ever use it.  By design, it should have no effect when making a
> new frame which is what the corrected patch should support.  Still, it
> might not work for elements like the external tool bar.
>
>  >> We can try to make it behave reasonably when
>  >> these values change but I am not sure whether we will succeed.
>  >>
>  >
>  > Let's try.
>
> Let's.
>
> martin

[-- Attachment #2: Type: text/html, Size: 4652 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]

 > Now, about clone-frame. Are there any objections to the below
 > implementation that uses text-pixels?
...
 > 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.

I'd write that as

   If PIXELWISE or `frame-resize-pixelwise' is non-nil and FRAME's terminal
   is not text-only, use the pixel size of FRAME for the cloned frame.
   Otherwise, use the number of columns and lines of FRAME for the cloned
   frame.

The behavior of the 'fullscreen' parameter might be queer if
'frame-resize-pixelwise' is nil and PIXELWISE is non-nil but that's to
be expected.

 > I may be able to test on GTK early this week, but I think you have GNU
 > Linux/GTK on your end?

I've tried here with a GTK-3 and a Motif build and have seen no
problems.

What I've seen is a slight misbehavior in setting up the 'fullscreen'
parameter on the GTK build (so it's not related to your function).  With

(setq frame-resize-pixelwise t)
(setq frame-inhibit-implied-resize t)

setting it to 'maximized' works as expected but setting it to
'fullheight' leaves a gap at the bottom.  Surprisingly, cloning a
'fullheight' frame with your function removes the gap.  The Motif frames
do not have the problem so it might be tool bar related but that should
affect the maximized frame as well.  I'll look into this later but would
be interested if you see the same with a GTK build:

To test:

(setq frame-resize-pixelwise t)
(setq frame-inhibit-implied-resize t)
(set-frame-parameter nil 'fullscreen 'fullheight)

martin

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

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

 > I'd write that as
 >
 >    If PIXELWISE or `frame-resize-pixelwise' is non-nil and FRAME's terminal
 >    is not text-only, use the pixel size of FRAME for the cloned frame.
 >    Otherwise, use the number of columns and lines of FRAME for the cloned
 >    frame.

But may be we should write

                (and pixelwise frame-resize-pixelwise))

and say

   If PIXELWISE and `frame-resize-pixelwise' are both non-nil and FRAME's
   terminal is not text-only, use the pixel size of FRAME for the cloned
   frame.  Otherwise, use the number of columns and lines of FRAME for
   the cloned frame.

Setting PIXELWISE to t in a setting where 'frame-resize-pixelwise' is
nil means asking for trouble since the window manager might not honor
it.  WDYT?

martin

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

[Ship Mints]

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

My expectation was that clone-frame, when frame-resize-pixelwise t, would
implicitly DTRT. I expect most people would have the same expectation. Same
as frameset-restore (now fixed with the patch).

When packages we use invoke clone-frame, we can't control their
parameterization of those invocations without advice, so
implied pixelwise seems wise. I added the explicit parameter more as
awareness than anything else, but I imagine that carefully-crafted
pixelwise child frames in packages may want explicit pixelwise clones if
the authors aren't going to let-bind frame-resize-pixelwise around
clone-frame calls. I'd prefer that we recommend that, actually.

Let's just do away with the pixelwise parameter and rely on the user
setting? That would be in keeping with the rest of the implementation, I
think.

On Mon, Dec 16, 2024 at 4:32 AM martin rudalics <rudalics@gmx.at> wrote:

>  > I'd write that as
>  >
>  >    If PIXELWISE or `frame-resize-pixelwise' is non-nil and FRAME's
> terminal
>  >    is not text-only, use the pixel size of FRAME for the cloned frame.
>  >    Otherwise, use the number of columns and lines of FRAME for the
> cloned
>  >    frame.
>
> But may be we should write
>
>                 (and pixelwise frame-resize-pixelwise))
>
> and say
>
>    If PIXELWISE and `frame-resize-pixelwise' are both non-nil and FRAME's
>    terminal is not text-only, use the pixel size of FRAME for the cloned
>    frame.  Otherwise, use the number of columns and lines of FRAME for
>    the cloned frame.
>
> Setting PIXELWISE to t in a setting where 'frame-resize-pixelwise' is
> nil means asking for trouble since the window manager might not honor
> it.  WDYT?
>
> martin
>

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

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

[Ship Mints]

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

With your suggested wording changes but without the pixelwise argument:

(defun clone-frame (&optional frame no-windows)
  "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
the user option `frame-resize-pixelwise' is non-nil, and FRAME is not
text-only, clone the originating frame's pixel size. Otherwise, use the
number of FRAME's columns and lines in the clone.

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)
               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))

On Mon, Dec 16, 2024 at 5:40 AM Ship Mints <shipmints@gmail.com> wrote:

> My expectation was that clone-frame, when frame-resize-pixelwise t, would
> implicitly DTRT. I expect most people would have the same expectation. Same
> as frameset-restore (now fixed with the patch).
>
> When packages we use invoke clone-frame, we can't control their
> parameterization of those invocations without advice, so
> implied pixelwise seems wise. I added the explicit parameter more as
> awareness than anything else, but I imagine that carefully-crafted
> pixelwise child frames in packages may want explicit pixelwise clones if
> the authors aren't going to let-bind frame-resize-pixelwise around
> clone-frame calls. I'd prefer that we recommend that, actually.
>
> Let's just do away with the pixelwise parameter and rely on the user
> setting? That would be in keeping with the rest of the implementation, I
> think.
>
> On Mon, Dec 16, 2024 at 4:32 AM martin rudalics <rudalics@gmx.at> wrote:
>
>>  > I'd write that as
>>  >
>>  >    If PIXELWISE or `frame-resize-pixelwise' is non-nil and FRAME's
>> terminal
>>  >    is not text-only, use the pixel size of FRAME for the cloned frame.
>>  >    Otherwise, use the number of columns and lines of FRAME for the
>> cloned
>>  >    frame.
>>
>> But may be we should write
>>
>>                 (and pixelwise frame-resize-pixelwise))
>>
>> and say
>>
>>    If PIXELWISE and `frame-resize-pixelwise' are both non-nil and FRAME's
>>    terminal is not text-only, use the pixel size of FRAME for the cloned
>>    frame.  Otherwise, use the number of columns and lines of FRAME for
>>    the cloned frame.
>>
>> Setting PIXELWISE to t in a setting where 'frame-resize-pixelwise' is
>> nil means asking for trouble since the window manager might not honor
>> it.  WDYT?
>>
>> martin
>>
>

[-- Attachment #2: Type: text/html, Size: 4816 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]

 > With your suggested wording changes but without the pixelwise argument:
...
 >           (new-frame))

Please omit the parentheses here.
...
 >      (unless (display-graphic-p frame)

This is a fix you should mention in the commit message.
...

Please provide a diff to the current frame.el of master and a commit
message.  Have you done your Emacs paperwork already?  If not, I can
install this as a tiny change because it's short enough.  Still you
should start the paperwork since otherwise we cannot install further
changes from you.

Thanks, martin

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

[Ship Mints]

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

Yes, paperwork on file. I will submit a patch to the latest master now that
we're agreed. Thank you for your help with these changes.

On Mon, Dec 16, 2024 at 10:49 AM martin rudalics <rudalics@gmx.at> wrote:

>  > With your suggested wording changes but without the pixelwise argument:
> ...
>  >           (new-frame))
>
> Please omit the parentheses here.
> ...
>  >      (unless (display-graphic-p frame)
>
> This is a fix you should mention in the commit message.
> ...
>
> Please provide a diff to the current frame.el of master and a commit
> message.  Have you done your Emacs paperwork already?  If not, I can
> install this as a tiny change because it's short enough.  Still you
> should start the paperwork since otherwise we cannot install further
> changes from you.
>
> Thanks, martin
>

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

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

[Ship Mints]

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

Martin, I'm assuming you'll do the patch against frame.h and frame.c?

On Mon, Dec 16, 2024 at 10:55 AM Ship Mints <shipmints@gmail.com> wrote:

> Yes, paperwork on file. I will submit a patch to the latest master now
> that we're agreed. Thank you for your help with these changes.
>
> On Mon, Dec 16, 2024 at 10:49 AM martin rudalics <rudalics@gmx.at> wrote:
>
>>  > With your suggested wording changes but without the pixelwise argument:
>> ...
>>  >           (new-frame))
>>
>> Please omit the parentheses here.
>> ...
>>  >      (unless (display-graphic-p frame)
>>
>> This is a fix you should mention in the commit message.
>> ...
>>
>> Please provide a diff to the current frame.el of master and a commit
>> message.  Have you done your Emacs paperwork already?  If not, I can
>> install this as a tiny change because it's short enough.  Still you
>> should start the paperwork since otherwise we cannot install further
>> changes from you.
>>
>> Thanks, martin
>>
>

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

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

[Ship Mints]

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

Aha, you beat me to it.

On Mon, Dec 16, 2024 at 11:01 AM Ship Mints <shipmints@gmail.com> wrote:

> Martin, I'm assuming you'll do the patch against frame.h and frame.c?
>
> On Mon, Dec 16, 2024 at 10:55 AM Ship Mints <shipmints@gmail.com> wrote:
>
>> Yes, paperwork on file. I will submit a patch to the latest master now
>> that we're agreed. Thank you for your help with these changes.
>>
>> On Mon, Dec 16, 2024 at 10:49 AM martin rudalics <rudalics@gmx.at> wrote:
>>
>>>  > With your suggested wording changes but without the pixelwise
>>> argument:
>>> ...
>>>  >           (new-frame))
>>>
>>> Please omit the parentheses here.
>>> ...
>>>  >      (unless (display-graphic-p frame)
>>>
>>> This is a fix you should mention in the commit message.
>>> ...
>>>
>>> Please provide a diff to the current frame.el of master and a commit
>>> message.  Have you done your Emacs paperwork already?  If not, I can
>>> install this as a tiny change because it's short enough.  Still you
>>> should start the paperwork since otherwise we cannot install further
>>> changes from you.
>>>
>>> Thanks, martin
>>>
>>

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

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

[Ship Mints]

[-- Attachment #1.1: Type: text/plain, Size: 708 bytes --]

Patch attached.

On Mon, Dec 16, 2024 at 10:49 AM martin rudalics <rudalics@gmx.at> wrote:

>  > With your suggested wording changes but without the pixelwise argument:
> ...
>  >           (new-frame))
>
> Please omit the parentheses here.
> ...
>  >      (unless (display-graphic-p frame)
>
> This is a fix you should mention in the commit message.
> ...
>
> Please provide a diff to the current frame.el of master and a commit
> message.  Have you done your Emacs paperwork already?  If not, I can
> install this as a tiny change because it's short enough.  Still you
> should start the paperwork since otherwise we cannot install further
> changes from you.
>
> Thanks, martin
>

[-- Attachment #1.2: Type: text/html, Size: 1129 bytes --]

[-- Attachment #2: 0001-Support-pixelwise-frame-cloning-bug-74750.patch --]
[-- Type: application/octet-stream, Size: 2003 bytes --]

From ae7de06ce1a285960c5c77955f66fb30a88a828c Mon Sep 17 00:00:00 2001
From: shipmints <shipmints@gmail.com>
Date: Mon, 16 Dec 2024 11:36:27 -0500
Subject: [PATCH] Support pixelwise frame cloning (bug#74750)

* lisp/frame.el (clone-frame):
Honor frame-resize-pixelwise when cloning a frame's geometry
via make-frame text-pixel feature.
Correctly specify source frame in display-graphic-p test for
tty selection behavior.
---
 lisp/frame.el | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index 1b5aa8cff08..02f3f9f9dae 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -863,6 +863,9 @@ make-frame-command
 (defun clone-frame (&optional frame no-windows)
   "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 the user option `frame-resize-pixelwise' is non-nil, and FRAME is
+not text-only, clone the originating frame's pixel size.  Otherwise, use
+the number of FRAME's columns and lines for the clone.

 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
@@ -875,10 +878,17 @@ clone-frame
           (seq-remove (lambda (elem)
                         (memq (car elem) frame-internal-parameters))
                       (frame-parameters frame)))
-         (new-frame (make-frame)))
+         new-frame)
+    (when (and frame-resize-pixelwise
+               (display-graphic-p frame))
+      (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)
+    (unless (display-graphic-p frame)
       (select-frame new-frame))
     new-frame))

--
2.47.1

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

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

 > Patch attached.

Installed on master.

Thanks, martin

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

[Eli Zaretskii]

> From: Ship Mints <shipmints@gmail.com>
> Date: Mon, 16 Dec 2024 11:39:02 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, 74750@debbugs.gnu.org
> 
> Patch attached.

Thanks.

>  (defun clone-frame (&optional frame no-windows)
>    "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 the user option `frame-resize-pixelwise' is non-nil, and FRAME is

Please avoid using "when" as a conditional; use "if" instead.  "When"
can be interpreted as time-related condition, which is not what you
want.

> +not text-only, clone the originating frame's pixel size.  Otherwise, use
> +the number of FRAME's columns and lines for the clone.

This uses double negation, which makes the documentation harder to
understand.  Suggest to rephrase:

  By default, clone the pixel-size of the original frame, but if
  `frame-resize-pixelwise' is nil or FRAME is a text-only frame, use
  the FRAME's number of lines and columns for the clone.

> -    (unless (display-graphic-p)
> +    (unless (display-graphic-p frame)
>        (select-frame new-frame))

Why do you need this condition?

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

[Ship Mints]

[-- Attachment #1.1: Type: text/plain, Size: 1827 bytes --]

Updated patch with docstring amendments.

The tty test you're asking about was already in place, I merely added the
missing frame argument that ensures the test is against the source frame
rather than the ambient selected-frame. As to its utility, I can go only by
the docstring sentence that reads "If the terminal is a text-only terminal
then also select the new frame." Seems to have originated with this
commit 2c662e6d66165db8ead2f4d19a61af521807b8ba in 2021 when clone-frame
was introduced.

On Mon, Dec 16, 2024 at 12:32 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Ship Mints <shipmints@gmail.com>
> > Date: Mon, 16 Dec 2024 11:39:02 -0500
> > Cc: Eli Zaretskii <eliz@gnu.org>, 74750@debbugs.gnu.org
> >
> > Patch attached.
>
> Thanks.
>
> >  (defun clone-frame (&optional frame no-windows)
> >    "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 the user option `frame-resize-pixelwise' is non-nil, and FRAME is
>
> Please avoid using "when" as a conditional; use "if" instead.  "When"
> can be interpreted as time-related condition, which is not what you
> want.
>
> > +not text-only, clone the originating frame's pixel size.  Otherwise, use
> > +the number of FRAME's columns and lines for the clone.
>
> This uses double negation, which makes the documentation harder to
> understand.  Suggest to rephrase:
>
>   By default, clone the pixel-size of the original frame, but if
>   `frame-resize-pixelwise' is nil or FRAME is a text-only frame, use
>   the FRAME's number of lines and columns for the clone.
>
> > -    (unless (display-graphic-p)
> > +    (unless (display-graphic-p frame)
> >        (select-frame new-frame))
>
> Why do you need this condition?
>

[-- Attachment #1.2: Type: text/html, Size: 2804 bytes --]

[-- Attachment #2: 0001-Support-pixelwise-frame-cloning-bug-74750.patch --]
[-- Type: application/octet-stream, Size: 1997 bytes --]

From ae7de06ce1a285960c5c77955f66fb30a88a828c Mon Sep 17 00:00:00 2001
From: shipmints <shipmints@gmail.com>
Date: Mon, 16 Dec 2024 11:36:27 -0500
Subject: [PATCH] Support pixelwise frame cloning (bug#74750)

* lisp/frame.el (clone-frame):
Honor frame-resize-pixelwise when cloning a frame's geometry
via make-frame text-pixel feature.
Correctly specify source frame in display-graphic-p test for
tty selection behavior.
---
 lisp/frame.el | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index 1b5aa8cff08..02f3f9f9dae 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -863,3 +863,6 @@ make-frame-command
 (defun clone-frame (&optional frame no-windows)
   "Make a new frame with the same parameters and windows as FRAME.
 With a prefix arg NO-WINDOWS, don't clone the window configuration.
+If the user option `frame-resize-pixelwise' is non-nil, and FRAME is
+graphical, clone the originating frame's pixel size.  Otherwise, use
+the number of FRAME's columns and lines for the clone.

 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
@@ -875,10 +878,17 @@ clone-frame
           (seq-remove (lambda (elem)
                         (memq (car elem) frame-internal-parameters))
                       (frame-parameters frame)))
-         (new-frame (make-frame)))
+         new-frame)
+    (when (and frame-resize-pixelwise
+               (display-graphic-p frame))
+      (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)
+    (unless (display-graphic-p frame)
       (select-frame new-frame))
     new-frame))

--
2.47.1

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

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

 > Let's just do away with the pixelwise parameter and rely on the user
 > setting? That would be in keeping with the rest of the implementation, I
 > think.

I'll add a 'resize-pixelwise' frame parameter soon.  Then 'clone-frame'
would simply have to inspect the parameter of the frame to clone and we
are done.

Other packages might have to adapt too since with the new parameter a
user could never have set 'frame-resize-pixelwise' and still have frames
that resize pixelwise.

martin

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

[Ship Mints]

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

The notion here would be that any non-tty frame created under
frame-resize-pixelwise is t has its resize-pixelwise parameter set by
default? Should that affect frameset-save/restore also?

On Mon, Dec 16, 2024 at 11:07 AM martin rudalics <rudalics@gmx.at> wrote:

>  > Let's just do away with the pixelwise parameter and rely on the user
>  > setting? That would be in keeping with the rest of the implementation, I
>  > think.
>
> I'll add a 'resize-pixelwise' frame parameter soon.  Then 'clone-frame'
> would simply have to inspect the parameter of the frame to clone and we
> are done.
>
> Other packages might have to adapt too since with the new parameter a
> user could never have set 'frame-resize-pixelwise' and still have frames
> that resize pixelwise.
>
> martin
>

[-- Attachment #2: Type: text/html, Size: 1224 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]

 > The notion here would be that any non-tty frame created under
 > frame-resize-pixelwise is t has its resize-pixelwise parameter set by
 > default?

Yes.

 > Should that affect frameset-save/restore also?

Yes, for restoring on a non-tty terminal.

martin

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

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

 > That patch seems to work. Thank you, Martin. I tested on NS with and
 > without vertical scroll bars and with and without fringes. A basic test of
 > frameset-save and frameset-restore now seem to correctly respect its
 > embedded text-pixel geometry.

Installed on master.  The purpose of the slots I removed was likely that
I originally planned to implement something like

(width . (outer-pixels . ...))
(height . (outer-pixels . ...))

in which case any implied resizing should have been completely inhibited
during the entire initial phase.  But I never did that.

'make-frame' might still need some tweaking for the tool bar when
'frame-inhibit-implied-resize' is non-nil.  I'll have to look into that.

martin