[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Merging the pgtk branch
From: |
Yuuki Harano |
Subject: |
Re: Merging the pgtk branch |
Date: |
Tue, 30 Nov 2021 00:41:15 +0900 (JST) |
On Wed, 11 Aug 2021 00:15:15 +0900 (JST),
Yuuki Harano <masm+emacs@masm11.me> 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.
Done.
>>> +#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.
Done.
>>> -#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.
Done.
>>> - `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.
Done.
>>> @@ -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.
I thought that gui_set_border_width could change the border width of a frame.
But there is the code:
---
if (FRAME_NATIVE_WINDOW (f) != 0)
error ("Cannot change the border width of a frame");
---
I was confused, and added #ifndef around it.
I don't know about the function.
What is it for?
>>> @@ -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.
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.
Reverted.
>>> +#ifndef PGTK_TRACE
>>> +#define PGTK_TRACE(fmt, ...) ((void) 0)
>>> +#endif
>>
>> Do we still need PGTK_TRACE (and all its calls)?
>
> Maybe, no need.
All removed.
>>> +#ifdef PGTK_DEBUG
>>
>> Do we need this PGTK_DEBUG stuff and its callers?
>
> Maybe, no need.
All removed.
>>> +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.
Done.
>>> +#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.
Done.
>>> +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.
Done.
>>> @@ -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.
Done.
--
Yuuki Harano