Re: Merging the pgtk branch

[Eli Zaretskii <eliz@gnu.org>]

> Date: Wed, 11 Aug 2021 00:15:15 +0900 (JST)
> Cc: emacs-devel@gnu.org
> From: Yuuki Harano <masm+emacs@masm11.me>
> 
> > Why is this unconditional?
> > 
> >> +extern GType emacs_fixed_get_type (void);
> > 
> > And this.
> 
> Since emacsgtkfixed.[ch] are extending a GTK's class, there should be
> those lines.
> But OK, they should be enclosed in #ifdef HAVE_PGTK.

Yes, that's what I meant.

> #ifdef USE_CAIRO
>   syms_of_ftcrfont ();       /* <------------ There is this call since before */
> #else
> #ifdef HAVE_XFT
>   syms_of_xftfont ();
> #endif  /* HAVE_XFT */
> #endif  /* not USE_CAIRO */
> #else   /* not HAVE_X_WINDOWS */
> #ifdef USE_CAIRO
>   syms_of_ftcrfont ();      /* <------------- I added this call. */
> #endif

Got it, thanks.

> >> +#ifdef HAVE_PGTK
> >> +  if (FRAME_TERMINAL (f)->frame_rehighlight_hook)
> >> +    (*FRAME_TERMINAL (f)->frame_rehighlight_hook) (f);
> >> +#endif
> > 
> > Why is the above done only for PGTK?
> 
> I'm sorry.  I forgot the reason.

I hope you will recall it.  Or maybe just disable the change and see
what goes wrong?

I'm not really opposed to the change, I'm just surprised it is needed,
and if indeed it is needed, I would like to have a comment there
explaining why.

> >> +#ifndef HAVE_PGTK
> >>    frame_resize_pixelwise = 0;
> >> +#else
> >> +  /* https://gitlab.gnome.org/GNOME/mutter/-/issues/396 */
> >> +  frame_resize_pixelwise = true;
> >> +#endif
> > 
> > Why the PGTK-specific setting here?
> 
> To work around the weird behavior when resizing with top-left corner.
> It is GNOME mutter's bug, which seems to be already fixed.
> The workaround may be able to be reverted.

Ok, but if you do want to leave the workaround, please add a comment
with the above explanation of the reason.

> >> +#else
> >> +  if (FRAME_X_DISPLAY(f) != DEFAULT_GDK_DISPLAY ())
> > 
> > So FRAME_X_P returns zero for PGTK, but FRAME_X_DISPLAY is still
> > relevant for it?  Isn't that confusing?
> 
> ----
> pgtkterm.h:#define FRAME_X_DISPLAY(f)        (FRAME_DISPLAY_INFO(f)->gdpy)
> ----
> 
> It's GTK's display.

OK, but please add a comment near FRAME_X_P's definition to the effect
that FRAME_X_P will return false for PGTK although FRAME_X_DISPLAY is
still valid.  (But see below: perhaps a more significant change
described there will make this a moot point.)

> >> -  f->output_data.x->hint_flags = 0;
> >> +  f->output_data.xp->hint_flags = 0;
> > 
> > Why did you need this change (and others like it)?  The "x" part here
> > has an important mnemonic value.
> 
> ----
> #include "xterm.h"
> #define xp x
> typedef struct x_output xp_output;
> #else
> #define xp pgtk
> typedef struct pgtk_output xp_output;
> #endif
> ----
> 
> When X-GTK build, xp is x.
> When PGTK build, xp is pgtk.
> 
> Without xp, all of them need to be conditional.
> e.g.
> ----
> #ifndef HAVE_PGTK
>   f->output_data.x->hint_flags = 0;
> #else
>   f->output_data.pgtk->hint_flags = 0;
> #endif

OK, but we have the same problem with other display types as well.
For example, the MS-Windows display has

  f->output_data.w32->...

The code is still conditional, allright, but the conditions are hidden
in the definitions of FRAME_DISPLAY_INFO, FRAME_FONT, etc.  Can't we
do something similar for PGTK?  If necessary, we could add more
FRAME_xxx macros to hide the conditionals, where the X and the PGTK
members need to be accessed differently.

Thanks!