all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Yuuki Harano <masm+emacs@masm11.me>
To: eliz@gnu.org
Cc: emacs-devel@gnu.org
Subject: Re: Merging the pgtk branch
Date: Wed, 11 Aug 2021 00:15:15 +0900 (JST)	[thread overview]
Message-ID: <20210811.001515.1312870178782255892.masm@luna.pink.masm11.me> (raw)
In-Reply-To: <835ywpo8ar.fsf@gnu.org>


On Sun, 01 Aug 2021 11:53:16 +0300,
	Eli Zaretskii <eliz@gnu.org> wrote:
>> -#ifdef USE_GTK
>> +#if defined(USE_GTK)
>> +#ifndef HAVE_PGTK
> 
> This could be done in a single conditional:
> 
>   #if defined USE_GTK && !defined HAVE_PGTK

Thanks.


>> +#define EMACS_TYPE_FIXED        (emacs_fixed_get_type ())
>> +#define EMACS_IS_FIXED(obj)     (G_TYPE_CHECK_INSTANCE_TYPE ((obj), EMACS_TYPE_FIXED))
> 
> 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.


>> --- a/src/font.c
>> +++ b/src/font.c
>> @@ -5584,7 +5584,11 @@ syms_of_font (void)
>>    syms_of_xftfont ();
>>  #endif  /* HAVE_XFT */
>>  #endif  /* not USE_CAIRO */
>> -#endif	/* HAVE_X_WINDOWS */
>> +#else	/* not HAVE_X_WINDOWS */
>> +#ifdef USE_CAIRO
>> +  syms_of_ftcrfont ();
>> +#endif
>> +#endif	/* not HAVE_X_WINDOWS */
> 
> Why was this needed? how did the Cairo build do this initialization
> until now?

----
#ifdef HAVE_WINDOW_SYSTEM
#ifdef HAVE_FREETYPE
  syms_of_ftfont ();
#ifdef HAVE_X_WINDOWS
  syms_of_xfont ();
#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
#endif  /* not HAVE_X_WINDOWS */
...
----


>> -#if defined (HAVE_XFT) || defined (HAVE_FREETYPE) || defined (HAVE_NS)
>> +#if defined (HAVE_XFT) || defined (HAVE_FREETYPE) || defined (HAVE_NS) || defined(HAVE_PGTK)
>>  extern Lisp_Object font_build_object (int, Lisp_Object, Lisp_Object, double);
>>  #endif
> 
> Doesn't the PGTK build define HAVE_XFT and HAVE_FREETYPE?

PGTK build defines HAVE_FREETYPE.
I'll revert the change.


>> - `pc' for a direct-write MS-DOS frame.
>> + `pc' for a direct-write MS-DOS frame,
>> + `pgtk' for an Emacs frame running entirely in GTK.
> 
> Since you call this "pure GTK", let's say so here as well.

OK.


>> @@ -4775,10 +4779,17 @@ gui_set_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
>>    if (border_width == f->border_width)
>>      return;
>  
>> +#ifndef HAVE_PGTK
>>    if (FRAME_NATIVE_WINDOW (f) != 0)
>>      error ("Cannot change the border width of a frame");
>> +#endif
>  
>>    f->border_width = border_width;
>> +
>> +#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.


>> @@ -6367,7 +6386,12 @@ focus (where a frame immediately loses focus when it's left by the mouse
>>  to set the size of a frame in pixels, to maximize frames or to make them
>>  fullscreen.  To resize your initial frame pixelwise, set this option to
>>  a non-nil value in your init file.  */);
>> +#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.


>> @@ -132,7 +136,9 @@ ftcrfont_open (struct frame *f, Lisp_Object entity, int pixel_size)
>>    filename = XCAR (val);
>>    size = XFIXNUM (AREF (entity, FONT_SIZE_INDEX));
>>    if (size == 0)
>> +  {
>>      size = pixel_size;
>> +  }
> 
> These braces are redundant here.

Indeed.


>> +#ifndef PGTK_TRACE
>> +#define PGTK_TRACE(fmt, ...) ((void) 0)
>> +#endif
> 
> Do we still need PGTK_TRACE (and all its calls)?

Maybe, no need.


>> +#ifndef HAVE_PGTK
>>    if (FRAME_X_DISPLAY (f) != DEFAULT_GDK_DISPLAY ())
>>      {
>>        GdkDisplay *gdpy = gdk_x11_lookup_xdisplay (FRAME_X_DISPLAY (f));
>> @@ -137,6 +150,17 @@ xg_set_screen (GtkWidget *w, struct frame *f)
>>        else
>>          gtk_window_set_screen (GTK_WINDOW (w), gscreen);
>>      }
>> +#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.


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


>> +#ifdef PGTK_DEBUG
> 
> Do we need this PGTK_DEBUG stuff and its callers?

Maybe, no need.


>> +static struct redisplay_interface pgtk_redisplay_interface = {
>> +  pgtk_frame_parm_handlers,
>> +  gui_produce_glyphs,
>> +  gui_write_glyphs,
>> +  gui_insert_glyphs,
>> +  gui_clear_end_of_line,
>> +  pgtk_scroll_run,
>> +  pgtk_after_update_window_line,
>> +  NULL, // gui_update_window_begin,
>> +  NULL, // gui_update_window_end,
> 
> Please use C-style comments, not C++-style comments (here and
> elsewhere).

OK.


>> +#define XFillRectangle(d, win, gc, x, y, w, h) \
>> +    ( cairo_rectangle (cr, x, y, w, h), cairo_fill (cr) )
> 
> I wonder why you need this XFillRectangle macro in code that is pure
> PGTK?

To avoid enbugging, I wanted to use XFillRectangle calls as is.

But there are the callers only here.  I should replace them with cairo.


>> +static int draw_glyphs_debug(const char *file, int lineno,
>> +			     struct window *w, int x, struct glyph_row *row,
>> +			     enum glyph_row_area area, ptrdiff_t start, ptrdiff_t end,
>> +			     enum draw_glyphs_face hl, int overlaps)
>> +{
>> +  return draw_glyphs(w, x, row, area, start, end, hl, overlaps);
>> +}
>> +#define draw_glyphs(w, x, r, a, s, e, h, o) \
>> +  draw_glyphs_debug(__FILE__, __LINE__, w, x, r, a, s, e, h, o)
>> +
> 
> The above looks like some left-over from debugging?  Do we still need
> it?

No need!
I'll revert the change.


>> @@ -32748,7 +32763,7 @@ mouse_face_from_buffer_pos (Lisp_Object window,
>>    hlinfo->mouse_face_face_id
>>      = face_at_buffer_position (w, mouse_charpos, &ignore,
>>  			       mouse_charpos + 1,
>> -                               !hlinfo->mouse_face_hidden, -1, 0);
>> +			       !hlinfo->mouse_face_hidden, -1, 0);
> 
> This looks like whitespace change?

I'll revert it.

-- 
Yuuki Harano



  parent reply	other threads:[~2021-08-10 15:15 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-01  8:53 Merging the pgtk branch Eli Zaretskii
2021-08-01 16:38 ` Yuuki Harano
2021-08-01 16:57   ` Stefan Monnier
2021-08-02 14:17     ` Yuuki Harano
2021-08-02 11:49   ` Eli Zaretskii
2021-08-03 12:52     ` Yuuki Harano
2021-08-03 13:37       ` Stefan Monnier
2021-08-04 13:43         ` Yuuki Harano
2021-11-14 12:20   ` Yuuki Harano
2021-11-14 13:27     ` Eli Zaretskii
2021-11-14 14:55       ` Yuuki Harano
2021-08-04 12:53 ` Font size (was: Merging the pgtk branch) Kévin Le Gouguec
2021-08-04 13:45   ` Eli Zaretskii
2021-08-04 13:58     ` Font size Kévin Le Gouguec
2021-08-04 15:46       ` Yuri Khan
2021-08-04 16:11         ` Eli Zaretskii
2021-08-04 17:09           ` Yuri Khan
2021-08-04 18:23             ` Eli Zaretskii
2021-08-04 18:33               ` Yuri Khan
2021-08-05 14:51                 ` Robert Pluim
2021-08-05 16:19                   ` Eli Zaretskii
2021-08-04 13:55   ` Font size (was: Merging the pgtk branch) Eli Zaretskii
2021-08-04 14:15     ` Font size Kévin Le Gouguec
2021-08-10 15:15 ` Yuuki Harano [this message]
2021-08-10 16:19   ` Merging the pgtk branch Eli Zaretskii
2021-11-29 15:41   ` Yuuki Harano
2021-12-02  3:45     ` Po Lu
2021-12-02  9:47       ` Robert Pluim
2021-12-02  9:50         ` Po Lu
2021-12-02 10:06           ` Robert Pluim
2021-12-02 10:12             ` Po Lu
2021-12-02 10:04         ` Po Lu
2021-12-02 11:34           ` Robert Pluim
2021-12-05 14:08     ` Eli Zaretskii
2021-12-05 16:01       ` Yuuki Harano
2021-12-05 16:06         ` Eli Zaretskii
2021-12-06  0:56           ` Po Lu
2021-12-06  4:41             ` Po Lu
2021-12-06 13:12               ` Yuuki Harano
2021-12-06 13:24                 ` Po Lu
2021-12-06 16:01                   ` Yuuki Harano
2021-12-09  5:01                     ` Po Lu
2021-12-18  7:58                       ` Po Lu
2021-12-18 12:52                         ` Yuuki Harano
2021-12-18 12:53                           ` Eli Zaretskii
2021-12-18 12:56                             ` Po Lu
2021-12-18 13:00                               ` Eli Zaretskii
2021-12-18 13:02                                 ` Po Lu
2021-12-18 12:54                           ` Po Lu
2021-12-18 12:58                             ` Eli Zaretskii
2021-12-18 13:01                               ` Po Lu
2021-12-18 13:24                                 ` Po Lu
2021-12-18 13:26                                   ` Po Lu
2021-12-18 13:37                                     ` Lars Ingebrigtsen
2021-12-18 13:39                                       ` Po Lu via Emacs development discussions.
2021-12-18 14:18                                         ` Eli Zaretskii
2021-12-18 13:45                                       ` Lars Ingebrigtsen
2021-12-18 13:40                                     ` Eli Zaretskii
2021-12-18 13:56                                     ` Yuuki Harano
2021-12-18 14:15                                       ` Bozhidar Batsov
2021-12-18 14:21                                         ` Eli Zaretskii
2021-12-18 15:12                                           ` Bozhidar Batsov
2021-12-18 15:37                                             ` Philip Kaludercic
2021-12-18 15:48                                             ` Eli Zaretskii
2021-12-18 15:56                                               ` Ken Brown
2021-12-18 16:05                                               ` Stefan Kangas
2021-12-18 16:10                                                 ` Eli Zaretskii
2021-12-19  0:30                                                 ` Po Lu
2021-12-19  0:28                                               ` Po Lu
2021-12-18 16:15                                       ` Eli Zaretskii
2021-12-19  0:32                                         ` Po Lu
2021-12-19  9:39                                           ` Bozhidar Batsov
2021-12-18 14:15                                     ` Eli Zaretskii
2021-12-18 19:10                                     ` Stefan Monnier
2021-12-18 21:50                                     ` Sean Whitton
2021-12-18 23:15                                     ` David Koppelman
2021-12-18 13:26                                 ` Eli Zaretskii
2021-12-18 13:29                                   ` Po Lu
2021-12-18 13:32                                     ` Eli Zaretskii
2021-12-18 13:33                                       ` Po Lu
2021-12-18 19:08                                         ` Stefan Monnier
2021-09-01 15:06 ` Transitory GUI connections (was Re: Merging the pgtk branch) Alex Bennée
2021-09-02  6:22   ` Po Lu
2021-09-17 15:09   ` Robert Pluim
  -- strict thread matches above, loose matches on Subject: below --
2021-12-05 19:14 Merging the pgtk branch Vitaly Ankh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210811.001515.1312870178782255892.masm@luna.pink.masm11.me \
    --to=masm+emacs@masm11.me \
    --cc=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.