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