Re: master 30dbdecd4a: * src/xterm.c: Add a small writeup on input handling on X.

Re: master 30dbdecd4a: * src/xterm.c: Add a small writeup on input handling on X.

[Po Lu]

I'll be doing some writing up on the X port in the future, but some
other people should contribute as well.  Martin in particular should
write a paragraph or two about how frames are resized on X, since I
think he knows quite a bit more than I do about that part of the code.

Thanks in advance.

Re: master 30dbdecd4a: * src/xterm.c: Add a small writeup on input handling on X.

[martin rudalics]

 > Martin in particular should
 > write a paragraph or two about how frames are resized on X, since I
 > think he knows quite a bit more than I do about that part of the code.

I can do that but note that most of the frame resizing code is shared
among X and other backends - in particular, the necessity to call
change_frame_size, set delayed_size_change there, call adjust_frame_size
eventually ...


While we're here - two questions related to size hints.  The first
concerns the x_wm_set_size_hint version in xterm.c.  _My_ version of it
currently starts as


#ifndef USE_GTK
void
x_wm_set_size_hint (struct frame *f, long flags, bool user_position, bool base_size)
{
   XSizeHints size_hints;
   Window window = FRAME_OUTER_WINDOW (f);

   if (!window)
     return;

   /* 2021 REMIX: Don't call widget_update_wm_size_hints here since on
      GNOME shell get_wm_shell may fail to produce the wmshell widget.
      As a consequence, no size hints get set before we issue our resize
      request, mutter (presumably) refuses to resize the outer window as
      requested and we end up with a wrong initial frame size.

      It's not clear whether other calls of update_wm_hints are affected
      as well but not calling widget_update_wm_size_hints here seems
      sufficient to fix the bug.  */

/** #ifdef USE_X_TOOLKIT **/
/**   if (f->output_data.x->widget) **/
/**     { **/
/**       widget_update_wm_size_hints (f->output_data.x->widget); **/
/**       return; **/
/**     } **/
/** #endif **/

   /* Setting PMaxSize caused various problems.  */
   size_hints.flags = PResizeInc | PMinSize /* | PMaxSize */;


Can you see any necessity to call widget_update_wm_size_hints here?


The second question is with XSetWMSizeHints in emacsgtkfixed.c: _My_
version of its starts as

/* Override the X function so we can intercept Gtk+ 3 calls.
    Use our values for min_width/height so that KDE don't freak out
    (Bug#8919), and so users can resize our frames as they wish.

    2021 REMIX: But do override only if x_gtk_override_size_hints is true
    (the default).  If x_gtk_override_size_hints is false, proceed with
    the original specification so shrinking frames with a specified
    minimum size works as intended (Bug#46963) and Emacs is always
    informed of the real sizes the WM reserves for its windows.

    We really should default x_gtk_override_size_hints to nil by default,
    but this can be done only after sufficient experience has been
    gathered, in particular on KDE.  Ideally, the code below will be
    dropped, the sooner the better.  */

void
XSetWMSizeHints (Display *d, Window w, XSizeHints *hints, Atom prop)
{
   struct x_display_info *dpyinfo = x_display_info_for_display (d);
   struct frame *f = x_top_window_to_frame (dpyinfo, w);
   long data[18];

   data[0] = hints->flags;
   ...
   data[17] = hints->win_gravity;

   if (x_gtk_override_size_hints && hints->flags & PMinSize && f)
     {
#ifdef HAVE_PGTK
       int w = f->output_data.pgtk->size_hints.min_width;
       int h = f->output_data.pgtk->size_hints.min_height;
#else
       int w = f->output_data.x->size_hints.min_width;
       int h = f->output_data.x->size_hints.min_height;
#endif
       data[5] = w;
       data[6] = h;
     }

Do you think we could eventually drop that overriding code invented by
Jan back then?  BTW one of these

#ifndef HAVE_PGTK
#ifdef HAVE_PGTK
#else
#endif
#endif

in that code should be eliminated - but I don't know which.

Thanks for your efforts, martin

Re: master 30dbdecd4a: * src/xterm.c: Add a small writeup on input handling on X.

[Po Lu]

martin rudalics <rudalics@gmx.at> writes:

> I can do that but note that most of the frame resizing code is shared
> among X and other backends - in particular, the necessity to call
> change_frame_size, set delayed_size_change there, call adjust_frame_size
> eventually ...

I think xterm.c is still the right place to put that information, since
xterm is essentially the reference implementation for an Emacs window
system.

> Can you see any necessity to call widget_update_wm_size_hints here?

Yes.  Xt has its own idea about size hints, and bad things will happen
if Xt doesn't know about them.

Ideally, we would actually just set the size hints on the EmacsShell
widget (that is the WMShell, since EmacsShell extends ApplicationShell,
which subclasses WMShell), and rely on Xt to do the heavy lifting.  I
will give that a look soon enough.

> The second question is with XSetWMSizeHints in emacsgtkfixed.c: _My_
> version of its starts as
>
> /* Override the X function so we can intercept Gtk+ 3 calls.
>    Use our values for min_width/height so that KDE don't freak out
>    (Bug#8919), and so users can resize our frames as they wish.
>
>    2021 REMIX: But do override only if x_gtk_override_size_hints is true
>    (the default).  If x_gtk_override_size_hints is false, proceed with
>    the original specification so shrinking frames with a specified
>    minimum size works as intended (Bug#46963) and Emacs is always
>    informed of the real sizes the WM reserves for its windows.
>
>    We really should default x_gtk_override_size_hints to nil by default,
>    but this can be done only after sufficient experience has been
>    gathered, in particular on KDE.  Ideally, the code below will be
>    dropped, the sooner the better.  */
>
> void
> XSetWMSizeHints (Display *d, Window w, XSizeHints *hints, Atom prop)
> {
>   struct x_display_info *dpyinfo = x_display_info_for_display (d);
>   struct frame *f = x_top_window_to_frame (dpyinfo, w);
>   long data[18];
>
>   data[0] = hints->flags;
>   ...
>   data[17] = hints->win_gravity;
>
>   if (x_gtk_override_size_hints && hints->flags & PMinSize && f)
>     {
> #ifdef HAVE_PGTK
>       int w = f->output_data.pgtk->size_hints.min_width;
>       int h = f->output_data.pgtk->size_hints.min_height;
> #else
>       int w = f->output_data.x->size_hints.min_width;
>       int h = f->output_data.x->size_hints.min_height;
> #endif
>       data[5] = w;
>       data[6] = h;
>     }
>
> Do you think we could eventually drop that overriding code invented by
> Jan back then?

Yes, I think we could (and should).  But I would introduce that option
first, ask most of the the people tracking master to set it to nil, and
if it seems okay after a year or two to then delete the code entirely.

> BTW one of these
>
> #ifndef HAVE_PGTK
> #ifdef HAVE_PGTK
> #else
> #endif
> #endif
>
> in that code should be eliminated - but I don't know which.

#ifdef HAVE_PGTK
      int w = f->output_data.pgtk->size_hints.min_width;
      int h = f->output_data.pgtk->size_hints.min_height;
#else
      int w = f->output_data.x->size_hints.min_width;
      int h = f->output_data.x->size_hints.min_height;
#endif

Should be replaced by the contents of the !HAVE_PGTK branch, since no X
function is overridden on PGTK.

Thanks.

Re: master 30dbdecd4a: * src/xterm.c: Add a small writeup on input handling on X.

[martin rudalics]

 > I'll be doing some writing up on the X port in the future, but some
 > other people should contribute as well.  Martin in particular should
 > write a paragraph or two about how frames are resized on X, since I
 > think he knows quite a bit more than I do about that part of the code.

I've now done that.  Please have a look.

martin

Re: master 30dbdecd4a: * src/xterm.c: Add a small writeup on input handling on X.

[martin rudalics]

While we're there: If I'm not mistaken, xg_frame_set_char_size now has
three instances of

#ifndef HAVE_PGTK
       gtk_window_resize (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)),
			 outer_width, outer_height);
#else
       if (FRAME_GTK_OUTER_WIDGET (f))
	{
	  gtk_window_resize (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)),
			     outer_width, outer_height);
	}
       else
	{
	  gtk_widget_set_size_request (FRAME_GTK_WIDGET (f),
				       outer_width, outer_height);
	}
#endif

Could you please refactor these into a separate routine and/or at least
remove those extra braces.

TIA, martin

Re: master 30dbdecd4a: * src/xterm.c: Add a small writeup on input handling on X.

[Po Lu]

martin rudalics <rudalics@gmx.at> writes:

> While we're there: If I'm not mistaken, xg_frame_set_char_size now has
> three instances of
>
> #ifndef HAVE_PGTK
>       gtk_window_resize (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)),
> 			 outer_width, outer_height);
> #else
>       if (FRAME_GTK_OUTER_WIDGET (f))
> 	{
> 	  gtk_window_resize (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)),
> 			     outer_width, outer_height);
> 	}
>       else
> 	{
> 	  gtk_widget_set_size_request (FRAME_GTK_WIDGET (f),
> 				       outer_width, outer_height);
> 	}
> #endif
>
> Could you please refactor these into a separate routine and/or at least
> remove those extra braces.

I'm not sure what those are for, but I will look into that.

Thanks.

Re: master 30dbdecd4a: * src/xterm.c: Add a small writeup on input handling on X.

[Po Lu]

martin rudalics <rudalics@gmx.at> writes:

>> I'll be doing some writing up on the X port in the future, but some
>> other people should contribute as well.  Martin in particular should
>> write a paragraph or two about how frames are resized on X, since I
>> think he knows quite a bit more than I do about that part of the code.
>
> I've now done that.  Please have a look.

LGTM.  I'll also explain resize synchronization too, thanks.