From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Yuuki Harano Newsgroups: gmane.emacs.devel Subject: Re: Merging the pgtk branch Date: Tue, 30 Nov 2021 00:41:15 +0900 (JST) Organization: Ingage Inc. Message-ID: <20211130.004115.1186997531521134959.masm@luna.pink.masm11.me> References: <835ywpo8ar.fsf@gnu.org> <20210811.001515.1312870178782255892.masm@luna.pink.masm11.me> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="2532"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: eliz@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Nov 29 16:42:50 2021 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mrinm-0000Mb-Gi for ged-emacs-devel@m.gmane-mx.org; Mon, 29 Nov 2021 16:42:47 +0100 Original-Received: from localhost ([::1]:52546 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mrink-0003lZ-Vl for ged-emacs-devel@m.gmane-mx.org; Mon, 29 Nov 2021 10:42:45 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:58892) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mrime-00034W-11 for emacs-devel@gnu.org; Mon, 29 Nov 2021 10:41:36 -0500 Original-Received: from shiro.masm11.me ([150.95.182.25]:56408) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mrimR-0004p7-Fh; Mon, 29 Nov 2021 10:41:35 -0500 Original-Received: from luna.pink.masm11.me (i153-144-34-242.s41.a033.ap.plala.or.jp [153.144.34.242]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by shiro.masm11.me (Postfix) with ESMTPSA id 19B3EC012D; Tue, 30 Nov 2021 00:41:16 +0900 (JST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=masm11.me; s=202002; t=1638200476; bh=Pvuq+Cxx+tFaOk7cj8SqTL9mnLqZWixZ4f5tEEgGbPM=; h=Date:To:Cc:Subject:From:In-Reply-To:References; b=OxX/WXExYevYXVNtePThYXFYPdDIVoG8PeCu0LDfZgwyFE/ZD/B2J4/n8W2R6S9ZS 8PBaTrSLB2GbJrqma5Im47B/sS54ese4N05d1Lui6MLsLZSdPyRX00ieoHhYHAeIVL 4N+d1V6t0sLDleycAgQotZPn6meuAAtGCWpitb0c= In-Reply-To: <20210811.001515.1312870178782255892.masm@luna.pink.masm11.me> X-Mailer: Mew version 6.8 on Emacs 29.0 Received-SPF: pass client-ip=150.95.182.25; envelope-from=masm@masm11.me; helo=shiro.masm11.me X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:280497 Archived-At: On Wed, 11 Aug 2021 00:15:15 +0900 (JST), Yuuki Harano 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